jpwatts / django-positions

A Django field for custom model ordering.
BSD 3-Clause "New" or "Revised" License
284 stars 71 forks source link

TypeError raises when collection field changes (python3 only) #51

Closed akszydelko closed 7 years ago

akszydelko commented 7 years ago

It is because of some recent changes #43, in some cases the updated value remains as None. E.g. when a field which serves as collection indicator is changed.

Then it reaches the following line:

if max_position >= updated >= min_position:

Source

Where python3 rises TypeError unorderable types: int() >= NoneType(), because non exact comparison are prohibited in python3.

The ordering comparison operators (<, <=, >=, >) raise a TypeError exception when the operands don’t have a meaningful natural ordering. Thus, expressions like 1 < '', 0 > None or len <= len are no longer valid, and e.g. None < None raises TypeError instead of returning False. A corollary is that sorting a heterogeneous list no longer makes sense – all the elements must be comparable to each other. Note that this does not apply to the == and != operators: objects of different incomparable types always compare unequal to each other.

Source

But I guess the recent change #43 may also influence python2 and lead to unwanted behaviour, but I haven't analysed that.

akszydelko commented 7 years ago

I have created a fix for the issue and pull requested the changes.

In my fix I have also considered #28, so the problem will not came back. And it will not because the following condition will be satisfied:

# existing instance, position not modified; no cleanup required
if current is not None and updated is None:
    return current

Bigger picture

So function will be left earlier as expected, after that we can safely replace the None value with -1, which will put the object at as the last one in the collection.