Minor changes to DC loaders for future surveys
This makes it easier to use a fallback header and access the last spectra from a wrapper loader. These changes are to support loading some of the IFS surveys (such as SAMI) that Data Central hosts.
Codecov Report
Attention: Patch coverage is 91.42857% with 6 lines in your changes are missing coverage. Please review.
Project coverage is 86.77%. Comparing base (
5ec4931) to head (ef70cdb). Report is 3 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| specutils/io/default_loaders/dc_common.py | 42.85% | 4 Missing :warning: |
| specutils/io/default_loaders/sami.py | 96.82% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #1122 +/- ##
===========================================
+ Coverage 72.22% 86.77% +14.55%
===========================================
Files 62 63 +1
Lines 4432 4500 +68
===========================================
+ Hits 3201 3905 +704
+ Misses 1231 595 -636
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Sorry, this dropped down my todo list, as a test of the changes, I've added the first pass of the SAMI loaders we're working on to the PR, which use the changed functionality.
Sorry, this dropped down my todo list, as a test of the changes, I've added the first pass of the SAMI loaders we're working on to the PR, which use the changed functionality.
Looking at the code coverage, it seems like the SAMI loaders are never getting called during testing, so I suspect that the identifier for the new loader isn't positively identifying the files you uploaded.
I've modified the tests slightly, that should directly flag whether the SAMI loaders are being used or not (which is weird because it works locally).
Oh, it looks like the tests are being skipped, if you look at test_loaders in the coverage run, there's a whole bunch of tests marked as skip. I guess they should be not skipped, which will likely improve coverage all round.
I've rebased this on top of #1132 to see if this actually shows the right coverage now.
The SAMI file coverage is as expected now.
LGTM now that coverage is reporting properly.