effigies / looseversion

A backwards/forwards-compatible fork of distutils.version.LooseVersion
Other
13 stars 1 forks source link

Ordering bug "TypeError: '<' not supported between instances of 'str' and 'int'" #11

Closed maaaaz closed 1 year ago

maaaaz commented 1 year ago

Hello there, Another bug encountered:

>>> m = ['15.2', '15.0.1', '14.7', '14.6.2', '14.5.1', '14.4.1', '14.3.1', '14.2.2', '14.2.1', '14.1.1', '14.0.1', '13.10', '13.9.3', '13.9.1', '13.8.1', '13.7.1', '13.6.2', '13.6.1', '13.5.1', '13.4.2', '13.4.1', '13.3.2', '13.3.1', '13.2.2', '13.2.1', '13.1.1', '13.0.1', '13.0-beta3']
>>> sorted(m, key=LooseVersion)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python3.11/dist-packages/looseversion.py", line 135, in __lt__
    c = self._cmp(other)
        ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/dist-packages/looseversion.py", line 185, in _cmp
    if self.version < other.version:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'str' and 'int'

Cheers !

effigies commented 1 year ago

See issue #6. The issue will be that you have 13.0-beta3 instead of 13.0.0-beta3.

In [43]: lv.LooseVersion('13.0-beta3').version
Out[43]: [13, 0, '-', 'beta', 3]

In [44]: lv.LooseVersion('13.0.1').version
Out[44]: [13, 0, 1]

LooseVersion does an okay job of comparing semi-consistent version strings, but it can't handle slots that could be either numeric or string.

My main goal with this package is to keep known semantics working. I do not intend to change the semantics to support things that were never supported, as that risks breaking expectations of users. You are free, however, to subclass if you want to define what it means to compare a str and an int:

class LooserVersion(lv.LooseVersion):
    def _cmp(self, other: object) -> int:
        ...
maaaaz commented 1 year ago

Very clear, thank you, I understand.

FYI, this broken example comes from PostgreSQL : https://community.chocolatey.org/packages/postgresql/13.0-beta3

maaaaz commented 1 year ago

And by the way, it seems that packaging version.parse now handles this case:

>>> from packaging import version
>>> sorted(m, key=version.parse)
['13.0-beta3', '13.0.1', '13.1.1', '13.2.1', '13.2.2', '13.3.1', '13.3.2', '13.4.1', '13.4.2', '13.5.1', '13.6.1', '13.6.2', '13.7.1', '13.8.1', '13.9.1', '13.9.3', '13.10', '14.0.1', '14.1.1', '14.2.1', '14.2.2', '14.3.1', '14.4.1', '14.5.1', '14.6.2', '14.7', '15.0.1', '15.2']
intgr commented 1 year ago

I do not intend to change the semantics to support things that were never supported, as that risks breaking expectations of users.

FWIW, this didn't actually crash in distutils LooseVersion with Python 2.x: because str and int are comparable/orderable in Python 2.

It was never fixed in distutils for Python 3.x because by that time LooseVersion was already deprecated.

effigies commented 1 year ago

Good to know. It seems cmp(a: int, b: str) == -1. If that's the expected behavior, I'm okay reviving it and making the package available for Python 2 as well.

Would you be up for making a PR?

effigies commented 1 year ago

Added an issue https://github.com/effigies/looseversion/issues/16.