Db Field instances cannot be compared to other objects
|Reported by:||charettes||Owned by:||charettes|
|Component:||Database layer (models, ORM)||Version:||master|
|Has patch:||yes||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
`django.db.models.fields.Field.__cmp__` assumes that it's always compared to a another Field instance thus raising an AttributeError when the following code is executed:
from django.db.models.fields import Field Field() == None
It should make sure that other is a Field instance before accessing the creation_counter property and if not return NotImplemented.
I stumbled on this issue when trying to use a PickledObjectField to store Field instance. When calling full_clean on the field it raised an AttributeError because it was trying to compare it EMPTY_VALUES: value in EMPTY_VALUES.
I agree that this is NOT a common use case of the Field class however I think that Field instances should be able to compare themselves to other objects without raising a confusing AttributeError.
I'm attaching a patch that sticks with simple comparison (although we should REALLY move to advanced comparison) with tests.
Here's a chat log I had on #python concerning simple comparison:
charettes 13:31:23 What should an object that implements __cmp__ method should return if it can't compare to other. papna 13:31:49 charettes: 1) Don't write __cmp__ methods unless you're targetting mesozoic Python 2) NotImplemented charettes 13:32:41 papna: But the doc doesn't say that... it says only to use it for advanced comparaison charettes 13:32:58 papna: http://docs.python.org/reference/datamodel.html#object.__cmp__ papna 13:33:42 charettes: Why are you using __cmp__? charettes 13:34:42 papna: actually I'm writing a patch for a project that used this method... should I suggest to switch to adavanced comparaison? papna 13:36:43 charettes: IIRC __cmp__ still uses the paradigm that __eq__ and friends and __add__ and friends and plenty of other stuff does, but I might be wrong. It's not flexible, which is why it has been deprecated well over a decade. charettes 13:37:27 papna: aight ty for your time
I can adapt the current patch to use advanced comparison instead if you prefer.
Change History (15)
comment:1 Changed 3 years ago by charettes
- Needs documentation unset
- Needs tests unset
- Patch needs improvement unset
- Summary changed from Db Field instances cannot be to other objects to Db Field instances cannot be compared to other objects
comment:6 Changed 3 years ago by akaariai
- Patch needs improvement set
- Triage Stage changed from Ready for checkin to Accepted
comment:7 Changed 3 years ago by charettes
- Version changed from 1.4 to master