MAINT: Fixing python 3.12 compatibility #2838
Conversation
astroquery/esa/jwst/core.py
Outdated
| @@ -39,6 +39,12 @@ | |||
| __all__ = ['Jwst', 'JwstClass'] | |||
|
|
|||
|
|
|||
| # We do trust the ESA tar files, this is to avoid the new to Python 3.12 deprecation warning | |||
| # https://docs.python.org/3.12/library/tarfile.html#tarfile-extraction-filter | |||
| if hasattr(tarfile, "fully_trusted_filter"): | |||
There was a problem hiding this comment.
What exactly was the warning that you saw before this patch? I cannot find it in the logs. (Though it does looks like you finally have to drop astropy-helpers.)
Also, I am not sure about this. Can we always trust any TAR files that are given?
There was a problem hiding this comment.
It's a new python 3.12 deprecation:
DeprecationWarning: Python 3.14 will, by default, filter extracted tar archives and reject files or modify their metadata. Use the filter argument to control this behavior.
There was a problem hiding this comment.
Hmm, would the archive people be able to provide a way to inspect and decide if the TAR is trusted or not?
Anyway, I think this is okay for now but should probably have a follow-up issue to revisit.
There was a problem hiding this comment.
also, weird, but locally I was all in the clear, not sure why it worked.
Anyway, if that's the case, then I would separate the actual fixes from the CI and put in the fixes, as we're in fact python3.12 compatible, and installing an from a wheel works just well on py3.12 without the helpers
There was a problem hiding this comment.
(I'll deal with the helpers after the 0.4.7 release, and when switching to a date based versioning scheme)
bsipocz
force-pushed
the
CI_add_py312
branch
from
939b849
to
50da649
Compare
bsipocz
force-pushed
the
CI_add_py312
branch
from
50da649
to
2d9b950
Compare
Codecov Report
@@ Coverage Diff @@
## main #2838 +/- ##
=======================================
Coverage 66.48% 66.49%
=======================================
Files 235 235
Lines 18077 18081 +4
=======================================
+ Hits 12019 12023 +4
Misses 6058 6058
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This will require astropy/pyvo#488
@pllim - the tarfile one feels a bit tricky, doing these workarounds basically accepts everything not just in the ESA modules but in general. Everything feels very hacky, would you hacking a bit around with the imports be worth it? E.g. importing tarfile as esatar, or similar? Or should I just add the default back at the end of the file?