Revert "[Python/Test] Work around pint TypeError - pint #1969 resolved" #1912
Conversation
This reverts commit dd03a35.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1912 +/- ##
==========================================
- Coverage 74.11% 74.11% -0.01%
==========================================
Files 445 445
Lines 55454 55454
Branches 9121 9121
==========================================
- Hits 41101 41098 -3
- Misses 11262 11264 +2
- Partials 3091 3092 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| from contextlib import nullcontext | ||
| from dataclasses import dataclass | ||
| from typing import Optional, Tuple, Dict | ||
| import sys | ||
|
|
||
| import pytest | ||
| try: | ||
| pint = pytest.importorskip("pint", "0.17.0") | ||
| except TypeError as e: | ||
| # The extra exception handling is here because pint is incompatible with | ||
| # Python 3.13. Once https://github.com/hgrecco/pint/issues/1969 is resolved the | ||
| # try/except here can be restored to just the importorskip. | ||
| pytest.skip(f"pint import failed due to {e}", allow_module_level=True) | ||
| pytest.importorskip("pint", "0.17.0") |
There was a problem hiding this comment.
I think this should not be a pure "revert" -- the removal of unused imports that was done in the original commit should be preserved. I also think the sense of "revert" is a bit wrong here; at least to me it implies that the original commit was erroneous, which isn't the case.
There was a problem hiding this comment.
In addition to that change, I think we need to set the minimum version here as the patch that fixed this bug, otherwise there's a (admittedly small) chance that an incompatible version could be installed.
There was a problem hiding this comment.
If so, that minimum version should be dependent on the Python version. For Python 3.10-3.12, whatever this new version is isn't required.
There was a problem hiding this comment.
Hmm, that's a good point and complicates this somewhat, probably more than it's worth. Especially since this is just in the test suite. The place where it might make sense to add Python-version-specific Pint version ranges would be in conda and/or the wheels, since the old versions of Pint can't be updated to say they're incompatible with 3.13. Probably still not worth it, though, for this low-probability event.
This reverts commit dd03a35.
Changes proposed in this pull request
Revert commit made as workaround until pint#1969 resolved (it is resolved)
If applicable, fill in the issue number this pull request is fixing
closes #1525 (already closed, but superior)
If applicable, provide an example illustrating new features this pull request is introducing
Checklist
scons build&scons test) and unit tests address code coverage