support Spectrum1D reading from file with flux in counts

[kecnry]

this PR implements an exception to support reading a Spectrum1D from a file with fluxes in counts. This does not however address more general reading round-tripping such as the one reported in #997. Note also that this is just hardcoding an additional exception, Spectrum1D currently doesn't seem to do any unit checking on the inputs for flux, and so other units might result in the same error (including non-sensical units, like fluxes with units of meters).

Example (that used to raise an OSError: Could not identify column containing the flux):

import specutils
import numpy as np
import astropy.units as u

sp = specutils.Spectrum1D(flux=np.linspace(0,1,11)*u.Unit('count'), spectral_axis=np.random.random(11)*u.Unit('J'))
sp.write("spec.fits", overwrite=True)

specutils.Spectrum1D.read("spec.fits")

[codecov]

Codecov Report

Merging #1018 (fec6908) into main (d4b3d2f) will increase coverage by 0.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
+ Coverage   70.03%   70.13%   +0.09%     
==========================================
  Files          64       64              
  Lines        4339     4346       +7     
==========================================
+ Hits         3039     3048       +9     
+ Misses       1300     1298       -2     

Impacted Files	Coverage Δ
specutils/io/parsing_utils.py	88.96% <100.00%> (+1.29%)	⬆️
specutils/io/asdf/types.py	100.00% <0.00%> (ø)
specutils/io/asdf/extension.py	100.00% <0.00%> (ø)
specutils/manipulation/resample.py	94.94% <0.00%> (ø)
specutils/manipulation/model_replace.py	96.42% <0.00%> (ø)
...utils/io/default_loaders/tests/test_jwst_reader.py	78.02% <0.00%> (+0.32%)	⬆️

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

[pllim]

u.Unit('J')

You sure you didn't mean Jy?

[pllim]

Are we able to turn that example you provided into a test?

[kecnry]

You sure you didn't mean Jy?

No, this is for spectral axis, not flux, so I just chose a random compatible unit for the spectral axis. Swap out with Hz or um if you'd rather!

Are we able to turn that example you provided into a test?

Sure, we could provide a regression test for this same hardcoded case, but it really would only test that someone doesn't revert this commit... so I'm wondering if it makes more sense to write general tests with the longer-term solution? Happy to basically copy-paste this in to a test though just to make sure it doesn't raise errors.

[pllim]

No, this is for spectral axis, not flux

Doh! Ignore me... 😆

but it really would only test that someone doesn't revert this commit

Right, we wouldn't want regression, which could happen if we ever refactor the internals.

[pllim]

I think the test could be written using parametrize and there is no need for random number generator. Please see suggestions.

Not sure what's up with the change log, maybe @rosteen only updates it on release? https://github.com/astropy/specutils/blob/main/CHANGES.rst

[rosteen]

Not sure what's up with the change log, maybe @rosteen only updates it on release? https://github.com/astropy/specutils/blob/main/CHANGES.rst

Yeah, I've been updating it when I do a release. I'm certainly not opposed to people updating it when they make a PR though!

I'm going to approve this, but generally agree with your suggestions @pllim . I'm about to log off for vacation but feel free to merge this once those are addressed.

[dhomeier]

I agree with the requested changes, otherwise this looks sensible. Yes, Spectrum1D.__init__ does not make any checks on flux PhysicalType, but there you are directly passing your quantity. parsing_utils are searching for some compatible data fields in an input file, so it makes sense to be more conservative.

[pllim]

@dhomeier , does the PR look okay to you now, or is there something else? Thanks!

[dhomeier]

Everything you had requested as far as I can see; good to me too, thanks!

[pllim]

Thanks, all!