Skip to content

Comments

swev-id: astropy__astropy-7671 Fix minversion failures with LooseVersion dev comparisons#98

Open
casey-brooks wants to merge 2 commits intoastropy__astropy-7671from
noa/issue-91
Open

swev-id: astropy__astropy-7671 Fix minversion failures with LooseVersion dev comparisons#98
casey-brooks wants to merge 2 commits intoastropy__astropy-7671from
noa/issue-91

Conversation

@casey-brooks
Copy link

Summary

  • add _normalize_for_loose_version to strip epochs/local tags and encode prerelease/dev/post tags into dotted numeric segments
  • ensure minversion ignores distutils deprecation warnings and falls back to normalized comparisons whenever tagged versions are involved or LooseVersion raises TypeError
  • extend astropy/utils/tests/test_introspection.py with coverage for dev, prerelease, post-release, epoch, and local-version cases

Testing

  • .venv/bin/flake8 astropy/utils/introspection.py astropy/utils/tests/test_introspection.py
  • LD_LIBRARY_PATH=/nix/store/wffgswxkp55xi14jy63rjsnfvl2qvmxy-gcc-14.3.0-lib/lib:/nix/store/qipd93x9gjyiygqk673rd2ssnf8y7jj0-gcc-14.3.0-lib/lib:/nix/store/zmjqwzhgl9hwr8xnk8raj8d50lzkql66-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib PYTHONPATH=/tmp/pycompat:/workspace/astropy/.venv/lib/python3.10/site-packages:. .venv/bin/python -m pytest astropy/utils/tests/test_introspection.py -k minversion -p no:cacheprovider

Observed failure

>>> from astropy.utils import minversion
>>> minversion('numpy', '1.14dev')
TypeError                                 Traceback (most recent call last)
<ipython-input-1-760e6b1c375e> in <module>()
      1 from astropy.utils import minversion
----> 2 minversion('numpy', '1.14dev')

~/dev/astropy/astropy/utils/introspection.py in minversion(module, version, inclusive, version_path)
    144
    145     if inclusive:
--> 146         return LooseVersion(have_version) >= LooseVersion(version)
    147     else:
    148         return LooseVersion(have_version) > LooseVersion(version)

~/local/conda/envs/photutils-dev/lib/python3.6/distutils/version.py in __ge__(self, other)
     68
     69     def __ge__(self, other):
---> 70         c = self._cmp(other)
     71         if c is NotImplemented:
     72             return c

~/local/conda/envs/photutils-dev/lib/python3.6/distutils/version.py in _cmp(self, other)
    335         if self.version == other.version:
    336             return 0
--> 337         if self.version < other.version:
    338             return -1
    339         if self.version > other.version:

TypeError: '<' not supported between instances of 'int' and 'str'

Reproduction

>>> from distutils.version import LooseVersion
>>> LooseVersion('1.14.3')  >= LooseVersion('1.14dev')
...
TypeError: '<' not supported between instances of 'int' and 'str'

Fixes #91

@casey-brooks casey-brooks requested a review from a team December 18, 2025 16:00
@casey-brooks
Copy link
Author

Test & Lint Summary

  • .venv/bin/flake8 astropy/utils/introspection.py astropy/utils/tests/test_introspection.py (pass)
  • LD_LIBRARY_PATH=/nix/store/wffgswxkp55xi14jy63rjsnfvl2qvmxy-gcc-14.3.0-lib/lib:/nix/store/qipd93x9gjyiygqk673rd2ssnf8y7jj0-gcc-14.3.0-lib/lib:/nix/store/zmjqwzhgl9hwr8xnk8raj8d50lzkql66-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib PYTHONPATH=/tmp/pycompat:/workspace/astropy/.venv/lib/python3.10/site-packages:. .venv/bin/python -m pytest astropy/utils/tests/test_introspection.py -k minversion -p no:cacheprovider (7 passed, 3 deselected)

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for improving the LooseVersion fallback and adding coverage. I did spot one functional regression: normalization currently slices the release components down to three entries, so any 4+ segment version collapses when the fallback path runs. For example, minversion(_module_with_version('1.2.3.4.dev1'), '1.2.3.5.dev1') now returns True even though the required version is newer. Please keep all release segments (while still padding missing ones) and add a regression test for a four-component release. Happy to take another look once that’s in place.

@casey-brooks
Copy link
Author

Test & Lint Summary

  • .venv/bin/flake8 astropy/utils/introspection.py astropy/utils/tests/test_introspection.py (pass)
  • LD_LIBRARY_PATH=/nix/store/wffgswxkp55xi14jy63rjsnfvl2qvmxy-gcc-14.3.0-lib/lib:/nix/store/qipd93x9gjyiygqk673rd2ssnf8y7jj0-gcc-14.3.0-lib/lib:/nix/store/zmjqwzhgl9hwr8xnk8raj8d50lzkql66-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib PYTHONPATH=/tmp/pycompat:/workspace/astropy/.venv/lib/python3.10/site-packages:. .venv/bin/python -m pytest astropy/utils/tests/test_introspection.py -k minversion -p no:cacheprovider (8 passed, 3 deselected)

Copy link

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the normalization regression so quickly. The updated helper preserves every release component, the sentinel keeps tag metadata distinct, and the new multi-component dev test exercises the edge case I flagged. Everything else looks solid and the expanded test matrix covers the tricky tag ordering. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants