Skip to content

Comments

Fix: UnrecognizedUnit __eq__ with None returns False (no TypeError) [swev-id: astropy__astropy-7606]#99

Open
casey-brooks wants to merge 1 commit intoastropy__astropy-7606from
fix/unrecognizedunit-eq-none
Open

Fix: UnrecognizedUnit __eq__ with None returns False (no TypeError) [swev-id: astropy__astropy-7606]#99
casey-brooks wants to merge 1 commit intoastropy__astropy-7606from
fix/unrecognizedunit-eq-none

Conversation

@casey-brooks
Copy link

Observed failure

In [12]: x = u.Unit('asdf', parse_strict='silent')

In [13]: x == None  # Should be False
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-13-2486f2ccf928> in <module>()
----> 1 x == None  # Should be False

/Users/aldcroft/anaconda3/lib/python3.5/site-packages/astropy/units/core.py in __eq__(self, other)
   1699 
   1700     def __eq__(self, other):
-> 1701         other = Unit(other, parse_strict='silent')
   1702         return isinstance(other, UnrecognizedUnit) and self.name == other.name
   1703 

/Users/aldcroft/anaconda3/lib/python3.5/site-packages/astropy/units/core.py in __call__(self, s, represents, format, namespace, doc, parse_strict)
   1808 
   1809         elif s is None:
-> 1810             raise TypeError("None is not a valid Unit")
   1811 
   1812         else:

TypeError: None is not a valid Unit

Fix summary

  • guard the Unit(...) conversion inside UnrecognizedUnit.__eq__ with a try/except (ValueError, UnitsError, TypeError) and return False on failure
  • keep existing equality semantics otherwise and add regression tests for comparisons against None, strings, and arbitrary objects

Testing

  • PYTHONPATH=. 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/y26slwav2ygi7gls9dzabygpaipwwwj4-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib .venv/bin/python -m pytest astropy/units/tests/test_units.py -k unrecognized --cache-dir=/workspace/.pytest_cache
  • flake8 astropy/units/core.py astropy/units/tests/test_units.py --per-file-ignores='astropy/units/tests/test_units.py:F841,E501,E222,E711' (existing style violations in legacy sections; new assertions pass)

Comparing an unrecognized unit to None raised a TypeError. Wrap Unit parsing in __eq__ with a try/except and return False on ValueError/UnitsError/TypeError. Add regression tests covering None, string, and non-unit comparisons.
@casey-brooks casey-brooks requested a review from a team December 18, 2025 16:02
@casey-brooks
Copy link
Author

Local validation

  • PYTHONPATH=. 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/y26slwav2ygi7gls9dzabygpaipwwwj4-gcc-14.3.0-lib/lib:/nix/store/n5lymg0y5x6i9wipkjrsi8aczv1nr4qc-zlib-1.3.1/lib .venv/bin/python -m pytest astropy/units/tests/test_units.py -k unrecognized --cache-dir=/workspace/.pytest_cache → 5 passed, 240 deselected
  • flake8 astropy/units/core.py astropy/units/tests/test_units.py --per-file-ignores='astropy/units/tests/test_units.py:F841,E501,E222,E711' → no new violations (legacy sections still require broader cleanup)

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.

Looks good to me.

Copy link
Collaborator

@rowan-stein rowan-stein left a comment

Choose a reason for hiding this comment

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

LGTM. Reviewed: changes fix UnrecognizedUnit.eq for None; regression tests added; local units tests pass.

@rowan-stein rowan-stein changed the base branch from astropy__astropy-7606 to main January 11, 2026 18:42
@rowan-stein rowan-stein changed the base branch from main to astropy__astropy-7606 January 12, 2026 20:31
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.

3 participants