Minor changes to DC loaders for future surveys

[aragilar]

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]

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 ⚠️
specutils/io/default_loaders/sami.py	96.82%	2 Missing ⚠️

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     

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

[rosteen]

Why the change here? It looks like this function acts on an existing spectra_map object and thus, as the comment says, shouldn't return anything.

[aragilar]

The original idea (based on what spectra I had access to at the time) was that files were either a single extension (with data represented as a 2d array, which is what most of the GAMA spectra were) or were across multiple extensions. We've got (legacy, so cannot change the format the files were written in) files that instead require more manipulation (e.g. the WCS was only in the first extension), so we want to be able to keep a reference around to the 1d spectrum independent of the mapping.

There are at least 3 surveys where we're going to use this (MGC, SAMI, S7), so rather than hacking this into each of their loaders, I figured I'd do the change in the place where it needs to be.

[rosteen]

Gotcha, seems reasonable enough. Any chance you could add a test for the new fallback header code?

[aragilar]

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.

[rosteen]

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.

[aragilar]

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).

[aragilar]

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.

[aragilar]

I've rebased this on top of #1132 to see if this actually shows the right coverage now.

[aragilar]

The SAMI file coverage is as expected now.

[rosteen]

LGTM now that coverage is reporting properly.