astropy
Decide if not-in-order spectra should be supported
In spacetelescope/specviz#471 an issue came up that at it's core is about a spectrum being "out of order" - that is, some of the other specutils machinery assumes the spectral_axis is strictly ordered.
While it's of course possible to fix the associated specutils machinery, it's almost certain that user-written analysis tools might make this assumption. So what is to be done? Here are the options I see:
- Have a warning be issued when a
Spectrum1Dis created and the inputs are out-of-order (both the array and the wcs case, probably by just checkingspectral_axis). This warning would basically just say "this might work for some things... but be aware here there be dragons because it's tricky to get this right". - Raise an exception if the input is a
Quantityspectral_axis, saying "you should sort your spectral_axis" (and a warning for wcs, which is impossible to fix-up) - Forcibly re-order the inputs
- Do nothing in code, but document clearly what the expectation is - i.e., should user code assume in-order or not?
cc @brechmos-stsci @nluetzge @crawfordsm @nmearl
Given that other strong checks are done on the input to Spectrum1D (i.e., it must have units), then I think it might make the most sense to be 2) (or at least 1) ). Forcibly re-order does not seem right, to me. Just my 2 cents.
I don't know if a warning in Spectrum1D is enough... its hard to connect this to the failure later in the extract region. Maybe the checks should be in the wcs routines? Which would make it easier to debug because right now it does not give any error when it cannot extract regions. (Oh maybe a related issue, add a warning when the extracted region is None).
I think I mildly tend towards 2 as well, although it hinges on whether we know of any use cases where spectra need to be out-of-order...
Oh maybe a related issue, add a warning when the extracted region is None
(see the discussion in #367)
I agree with the general swing in favor of (2).
I'm not sure I understand the reasoning behind just issuing a warning for the WCS case though. Seems like if it's an exception for the spectral_axis case it should be an exception for the WCS case.
The failure of the WCS parsing is at the astropy FITSWCS-level -- that is to say, the astropy fits machinery expects that the dispersion axis is strictly ordered. I think this means that we, at the specutils-level, shouldn't bother with supporting the use case of a non-ordered spectral axis and should make the user very aware of it.
I'm for 2, and I think we should do something touching on 3. E.g. a keyword force_order='ascending' or similar for users that don't want to have to edit their FITS file to get specutils to parse it. (I'm imaging the case where some hapless scientist has to deal with the COS spectra and just wants specutils to parse the darn file).
Let's think about the use cases for this. If I convert my spectrum from wavelength to frequency, I'll turn an ascending sorting into a decending sorting. Is that OK?
The other case I've come across this (before specutils even existed) are echelle spectra where the orders overlap, i.e. some wavelength range is seen in more than one order. I'm not sure how common it is to stick all this into a single spectrum (instead of having one spectrum per order), but at least I;ve seen that at some point.
WCS handling has been updated to support both strictly ascending and descending data, but non-ordered data is still not supported (and probably can't be at least in the wcs).
@eteq is this issue now about how to let the user know, or is the internal handling enough? (Currently, the wcs will have fail hard with an associated error about the ordering).
I've often run into a problem where a single wavelength is repeated with two values (e.g. in model spectra at spectral breaks). I'm fine with not supporting this, but notifying the user. If the WCS warning is intelligible enough, that's fine.