Roundtripping spectra and to what extent we should warn users / expect metadata?

[gully]

@ericasaw raised the issue of "roundtripping" a spectrum:

the ability to create Spectrum objects from existing wavelength and flux quantities (eg: like lightkurve's LightCurve constructor, EchelleSpectrum.wavelength = arr; EchelleSpectrum.flux = arr)

This pattern is currently supported in muler:

roundtrip_spec = IGRINSSpectrum(spectral_axis=wavelength*u.Angstrom, 
                                flux=flux*u.dimensionless_unscaled, )

but with one huge caveat: not all methods will successfully work after roundtripping.

So the question arises: how much do we want to support spectra created in this slightly non-standard, but still possible manner?

A good example use case is barycentric correction: Imagine you have a "roundtripped" spectrum, and you try to run barycentric_correct( ). It will generally not have the meta nor meta['header'] attributes that it needs to determine the BERV. So should we :

1. Raise an informative error
2. Allow the user to input the needed properties
3. Status quo (it raises a KeyError)
4. Other?

@ericasaw and I agree that solution 1 is both expedient and useful to the users.

Open questions:

What other methods are expected to fail when using a spectrum that has been generated through round-tripping?

Should we recommend that folks provide a header= input when handing in the flux and wavelength?

[gully]

At first glance, it seems like one could simply put a warning in the __init__ if the file= kwarg is not passed in:

        if file is not None:
            # [...]
            super().__init__(
                spectral_axis=lamb.to(u.Angstrom),
                flux=flux,
                mask=mask,
                wcs=None,
                uncertainty=uncertainty,
                meta=meta_dict,
                **kwargs,
            )
        else:
            super().__init__(*args, **kwargs)
            if not ("header" in self.meta.keys()):
                log.warn(
                    "The spectrum metadata appears to be missing.  "
                    "The functionality of muler may be impaired without metadata.  "
                    "See discussion at https://github.com/OttoStruve/muler/issues/79."
                )

I tried this and it works when I attempt the roundtripping:

spec = IGRINSSpectrum(file=full_path)

wavelength = spec.wavelength
flux = spec.flux

roundtrip_spec = IGRINSSpectrum(spectral_axis=wavelength, flux=flux)

The spectrum metadata appears to be missing. The functionality of muler may be impaired without metadata. See discussion at #79.

The warning message goes away if you put meta = spec.meta ✅

However, it over-produces the warning, for reasons I did not anticipate and that may reveal an overzealous-albeit-harmless copy strategy going on behind the scenes:

spec.normalize() # erroneously produces the warning

IGRINSSpectrumList operations also produce the warnings (one for each order, ouch).

Two takeaways:

1. For now I think it may be expedient to target warnings on the individual operations, say .barycentric_correct() that hinge on the accurate metadata being in place.
2. There appear to be unnecessary copies going on. Someone could drill down on these to understand the antipattern that led to them in the first place.

[ericasaw]

Hi @gully,
Was just testing reading in the new retel files with muler for the new RRISA release and I noticed that each order came up with this error. Is this something we need to fix internally or am I missing something in our fits files that needs to be added with the next release?

[gully]

Recapping what @ericasaw and I just talked about in the hallway, you should have a FITS header in the zeroth index in the rtell files.

muler/src/muler/igrins.py

Line 107 in 3b6f7cd

hdr = hdus[0].header

Can you verify the rtell files contain a plp-like header in the zeroth index?

The other conceivable hiccup is that some specutils math operations do not pass through the header by default (bummer!). You have to manually tell the math to handle the metadata:
spec.add(other_spec, handle_meta='first_found')

Can you inspect the sequence of operations that led to this warning to see if any math operations may suffer from this metadata amnesia?