Skip to content

Conversation

cosmoJFH
Copy link
Contributor

@cosmoJFH cosmoJFH commented Mar 7, 2025

Dear Astroquery team,

the Euclid datalink service has been updated, and the new retrieval types 'SPECTRA_BGS', 'SPECTRA_RGS' are now available. We would like to include them in Euclid module, so the method get_spectrum could use them.

cc @esdc-esac-esa-int

jira: EUCLIDPCR-1914

@cosmoJFH cosmoJFH changed the title EUCLID : include new datalink retrieval type 'SPECTRA_BGS', 'SPECTRA_RGS' EUCLID : include new datalink retrieval type 'SPECTRA_BGS' and 'SPECTRA_RGS' Mar 7, 2025
Copy link

codecov bot commented Mar 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.10%. Comparing base (2dbfa3f) to head (c5b7a81).
Report is 236 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3249      +/-   ##
==========================================
+ Coverage   69.07%   69.10%   +0.02%     
==========================================
  Files         232      232              
  Lines       19617    19632      +15     
==========================================
+ Hits        13551    13566      +15     
  Misses       6066     6066              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cosmoJFH cosmoJFH changed the title EUCLID : include new datalink retrieval type 'SPECTRA_BGS' and 'SPECTRA_RGS' EUCLID : include new datalink retrieval types 'SPECTRA_BGS' and 'SPECTRA_RGS' Mar 7, 2025
@bsipocz bsipocz added this to the v0.4.10 milestone Mar 7, 2025
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.

Some minor comments, otherwise looks good. I'll just go ahead and commit the suggestions and merge the PR.

Thanks!

bsipocz and others added 2 commits March 7, 2025 08:44
The test will need a bit more tweaking, I'll do that in a follow-up.
@bsipocz
Copy link
Member

bsipocz commented Mar 7, 2025

Apparently, I had a very quick review of the tests in the original PR and didn't notice that a lot of them are catching very generic Exceptions rather than the more specific exception type the code is expected to raise. Ideally, those need to be fixed and be made more specific.

I've also added an example of how to match the error message without a new assert line, etc.

The former would be nice to be addressed sometime in the future, while the latter is just a nice pytest feature but it doesn't matter much if the code is refactored or not.

@bsipocz bsipocz merged commit c761ff5 into astropy:main Mar 7, 2025
10 checks passed
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.

2 participants