Move JPLSpec to linelists/ #3455
Conversation
| "Please use 'from astroquery.linelists.jplspec import JPLSpec' instead. " | ||
| "The old import path will be removed in a future version.", | ||
| DeprecationWarning, | ||
| stacklevel=2 |
There was a problem hiding this comment.
I don't know if this is necessary or right. I have no experience playing with stacklevels yet.
There was a problem hiding this comment.
I can't remember any more why I went with stacklevel 2 for astroquery.irsa, but it worked.
keflavich
force-pushed
the
move_jplspec
branch
from
c10fb2a to
8424b04
Compare
bsipocz
left a comment
bsipocz
left a comment
There was a problem hiding this comment.
The concept is OK, but you need to fix the tests
CHANGES.rst
Outdated
| jplspec | ||
| ^^^^^^^ | ||
|
|
||
| - Moved to linelists/ [#3455] |
There was a problem hiding this comment.
We will need a more detailed changelog, as well as to add a new linelists.jplspec entry to mark its creation
There was a problem hiding this comment.
OK, might need to iterate on this once more
| "Please use 'from astroquery.linelists.jplspec import JPLSpec' instead. " | ||
| "The old import path will be removed in a future version.", | ||
| DeprecationWarning, | ||
| stacklevel=2 |
There was a problem hiding this comment.
I can't remember any more why I went with stacklevel 2 for astroquery.irsa, but it worked.
astroquery/linelists/jplspec/core.py
Outdated
| from ...query import BaseQuery | ||
| from ...utils import async_to_sync |
There was a problem hiding this comment.
as you already touch this, use normal imports, we don't need absolute ones, especially not for anything we don't directly test in this particular file
There was a problem hiding this comment.
Do you mean switch to from astroquery...? (that's an absolute import, right? the from ... is relative?)
There was a problem hiding this comment.
yeap. I think we needed this in python2 era (but even then, it could have just stayed relative from the module in the test files and not for everything astroquery), but it's not really an issue any more.
|
non-remote CI is fixed (well, we'll see, but at least locally...). Remote CI can't be so easily repaired as it requires the workaround in #3456, so docs are expected to fail. |
|
ah, oops, I need to not fail for warnings... |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3455 +/- ##
=======================================
Coverage 70.68% 70.69%
=======================================
Files 232 233 +1
Lines 20088 20091 +3
=======================================
+ Hits 14200 14203 +3
Misses 5888 5888 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bsipocz
left a comment
bsipocz
left a comment
There was a problem hiding this comment.
All looks good, but needs a rebase for the changelog.
fix bad file add fix ****s
bsipocz
force-pushed
the
move_jplspec
branch
from
eeb56f2 to
8140eb7
Compare
2 participants
CDMS was moved there, this is the right home for JPLSpec too.
This is the first in a series of PRs to clean up and regularize these modules.