search

edelooff / sqlalchemy-json

Full-featured JSON type with mutation tracking for SQLAlchemy
http://variable-scope.com/posts/mutation-tracking-in-nested-json-structures-using-sqlalchemy
BSD 2-Clause "Simplified" License
189 stars 34 forks source link

Fix list.sort() argument passing #27

Closed mvdbeek closed 3 years ago

mvdbeek commented 3 years ago

The cmp keyword doesn't exist anymore, and so fails with

...
  File "/home/runner/work/galaxy/galaxy/galaxy root/lib/galaxy/datatypes/mothur.py", line 78, in set_meta
    dataset.metadata.labels.sort()
  File "/home/runner/work/galaxy/galaxy/galaxy root/.venv/lib/python3.7/site-packages/sqlalchemy_json/track.py", line 174, in sort
    super(TrackedList, self).sort(cmp=cmp, key=key, reverse=reverse)
TypeError: sort() takes at most 2 keyword arguments (3 given)
edelooff commented 3 years ago

Thanks for the PR, this solves some pretty glaring Python 3 compatibility problems. The positional-only marker is redundant though, and breaks compatibility with any version prior to 3.8. Having it match the signature to the current list.sort would be preferred (*, key=None, reverse=False), that way it works in any version of Python 3.

If you could tweak the PR, I'd be happy to merge it.

mvdbeek commented 3 years ago

Sure, thanks for the quick response!