Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eJWST - Instrument search updated and bugfixes #2691

Merged

Conversation

jespinosaar
Copy link
Contributor

Dear astroquery team,

I am creating this pull request to include some small changes in how searching for instrument is being done in ESA JWST module and to fix some bugs we have found executing the tests (mostly remote testing and code in the documentation). Thanks in advance for your support!

cc @esdc-esac-esa-int

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Merging #2691 (5601454) into main (2facaed) will increase coverage by 0.03%.
The diff coverage is 70.00%.

@@            Coverage Diff             @@
##             main    #2691      +/-   ##
==========================================
+ Coverage   69.40%   69.44%   +0.03%     
==========================================
  Files         304      304              
  Lines       22445    22457      +12     
==========================================
+ Hits        15579    15595      +16     
+ Misses       6866     6862       -4     
Impacted Files Coverage Δ
astroquery/esa/xmm_newton/__init__.py 100.00% <ø> (ø)
astroquery/esa/xmm_newton/core.py 64.35% <ø> (ø)
astroquery/esa/xmm_newton/tests/__init__.py 100.00% <ø> (ø)
astroquery/esa/xmm_newton/tests/dummy_handler.py 80.00% <ø> (ø)
...troquery/esa/xmm_newton/tests/dummy_tap_handler.py 86.27% <ø> (ø)
astroquery/esa/xmm_newton/tests/setup_package.py 60.00% <ø> (ø)
astroquery/esa/xmm_newton/tests/test_xmm_newton.py 92.03% <ø> (ø)
...ery/esa/xmm_newton/tests/test_xmm_newton_remote.py 27.08% <16.66%> (+0.20%) ⬆️
astroquery/esa/jwst/core.py 75.87% <66.66%> (+0.21%) ⬆️
astroquery/esa/iso/__init__.py 100.00% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jespinosaar
Copy link
Contributor Author

Dear all,

I think all the tests are ok, please let me know if you have further comments.

Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

Borderline whether this needs a changelog for the instrument query changes. As I see we haven't had a use case in the test suite that has actually run into the case sensitivity/insensitivity question, and we received no bug reports about it, yet you may want to mention it in the changelog.

The rest of the fixes are greatly appreciated.

I go ahead and merge this now, as the fixes are good, and let me know about the changelog, we can always add even after the merg.

@bsipocz bsipocz merged commit 770949e into astropy:main Mar 28, 2023
@jespinosaar
Copy link
Contributor Author

Dear @bsipocz ,

Many thanks for merging this! I think no bugs are being reported as we discovered it right after a reprocessing was executed but it was going to affect how the instruments were filtered. I forgot to include anything in the changelog, but if you want to I can add it in other PR.

Regards,
Javi

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

Successfully merging this pull request may close these issues.

None yet

3 participants