Skip to content

Commit

Permalink
fix: SDSS-V SpectrumList format ambiguity, mwmVisit BOSS load fail (#…
Browse files Browse the repository at this point in the history 
…1185)

* fix: mwmVisit BOSS HDU default loader fail

- no longer checks for "date_obs"; calculate that yourself
- also adds "sdss_id" to metadata now

* fix: SDSS-V SpectrumList loader ambiguity + add: BOSS-only mwm test cases

- added new test cases for BOSS-only mwmVisit and mwmStar files
- added new checks to SpectrumList mwmVisit/mwmStar test to check verified filetype is correct
- forced override on default SpectrumList loaders -- now SpectrumList is no longer ambiguous and doesn't require a format specification
  - relevant areas in tests are updated accordingly
- added print warnings to when HDU is not specified on Spectrum1D loaders for files with multiple spectra.
- ensured tests now remove tempfiles with os.remove
  - arguably, this could work better with tmpfile, but i don't know how tests are deployed on the server-side

* add: changelog -> CHANGES.rst

- three points outlining changes listed in PR as per #1185

* fix: HDU unspecified print message

Co-authored-by: Brian Cherinka <havok2063@hotmail.com>

* feat: condense loaders into only SpectrumList of 1D flux

- all loaders now only load for a single datatype, avoiding prior knowledge of SDSS datatypes
- updated to only load as SpectrumList
- updated to load all visits in mwmVisit files as individual Spectrum1D objects in the SpectrumList
- relevant tests removed
- relevant import __all__ adjusted

* chore: changelog update

- describes changes shown previously

* revert: readd all Spectrum1D loaders and tests

revert back to 4bee136

* feat: visit specification on mwmVisit load

- readded print -> warnings conversion
- can specify the visit to load on mwmVisit load.
- added relevant tests for the new mwmVisit case

* revert: completely rollback to initial case

- no longer unpacks nD spectrum in all cases for mwm
- will still unpack flux from 2D to 1D in the event its single visit or coadd

* fix: test cases

- fixed flux array length check so that the 2D mwm visits are checked properly

* fix: split test cases for user warning + remove useless warning

- removed HDU is empty warning on mwm SpecList, not really worth a warning
- moved test cases which are designed to throw exceptions into their own test function

---------

Co-authored-by: Brian Cherinka <havok2063@hotmail.com>
  • Loading branch information
@rileythai @havok2063
rileythai and havok2063 authored Nov 1, 2024
1 parent a34ebb8 commit c4b90ed
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 112 deletions.
8 changes: 8 additions & 0 deletions CHANGES.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,23 @@ Bug Fixes
- Fixed ``Spectrum1D.with_flux_unit()`` not converting uncertainty along
with flux unit. [#1181]

- Fixed ``mwmVisit`` SDSS-V ``Spectrum1D`` and ``SpectrumList`` default loader
being unable to load files containing only BOSS instrument spectra. [#1185]

- Fixed automatic format detection for SDSS-V ``SpectrumList`` default loaders. [#1185]

- Fixed extracting a spectral region when one of spectrum/region is in wavelength
and the other is in frequency units. [#1187]


Other Changes and Additions
^^^^^^^^^^^^^^^^^^^^^^^^^^^

- Replaced ``LevMarLSQFitter`` with ``TRFLSQFitter`` as the former is no longer
recommended by ``astropy``. [#1180]

- "Multi" loaders have been removed from SDSS-V ``SpectrumList`` default loaders. [#1185]

1.17.0 (2024-10-04)
-------------------

Expand Down
71 changes: 40 additions & 31 deletions specutils/io/default_loaders/sdss_v.py
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
"""Register reader functions for various spectral formats."""
import warnings
from typing import Optional

import numpy as np
from astropy.units import Unit, Quantity, Angstrom
from astropy.nddata import StdDevUncertainty, InverseVariance
from astropy.io.fits import HDUList, BinTableHDU, ImageHDU
from astropy.utils.exceptions import AstropyUserWarning

from ...spectra import Spectrum1D, SpectrumList
from ..registers import data_loader
Expand Down Expand Up @@ -183,9 +185,10 @@ def load_sdss_apStar_1D(file_obj, idx: int = 0, **kwargs):


@data_loader(
"SDSS-V apStar multi",
"SDSS-V apStar",
identifier=apStar_identify,
dtype=SpectrumList,
force=True,
priority=10,
extensions=["fits"],
)
Expand Down Expand Up @@ -259,9 +262,10 @@ def load_sdss_apVisit_1D(file_obj, **kwargs):


@data_loader(
"SDSS-V apVisit multi",
"SDSS-V apVisit",
identifier=apVisit_identify,
dtype=SpectrumList,
force=True,
priority=10,
extensions=["fits"],
)
Expand Down Expand Up @@ -311,7 +315,6 @@ def load_sdss_apVisit_list(file_obj, **kwargs):


# BOSS REDUX products (specLite, specFull, custom coadd files, etc)

@data_loader(
"SDSS-V spec",
identifier=spec_sdss5_identify,
Expand All @@ -337,6 +340,8 @@ def load_sdss_spec_1D(file_obj, *args, hdu: Optional[int] = None, **kwargs):
"""
if hdu is None:
# TODO: how should we handle this -- multiple things in file, but the user cannot choose.
warnings.warn('HDU not specified. Loading coadd spectrum (HDU1)',
AstropyUserWarning)
hdu = 1 # defaulting to coadd
# raise ValueError("HDU not specified! Please specify a HDU to load.")
elif hdu in [2, 3, 4]:
Expand All @@ -347,9 +352,10 @@ def load_sdss_spec_1D(file_obj, *args, hdu: Optional[int] = None, **kwargs):


@data_loader(
"SDSS-V spec multi",
"SDSS-V spec",
identifier=spec_sdss5_identify,
dtype=SpectrumList,
force=True,
priority=5,
extensions=["fits"],
)
Expand Down Expand Up @@ -433,16 +439,21 @@ def _load_BOSS_HDU(hdulist: HDUList, hdu: int, **kwargs):
priority=20,
extensions=["fits"],
)
def load_sdss_mwm_1d(file_obj, hdu: Optional[int] = None, **kwargs):
def load_sdss_mwm_1d(file_obj,
hdu: Optional[int] = None,
visit: Optional[int] = None,
**kwargs):
"""
Load an unspecified spec file as a Spectrum1D.
Parameters
----------
file_obj : str, file-like, or HDUList
FITS file name, file object, or HDUList..
hdu : int
hdu : Optional[int]
Specified HDU to load.
visit : Optional[int]
Specified visit index to load.
Returns
-------
Expand All @@ -459,17 +470,22 @@ def load_sdss_mwm_1d(file_obj, hdu: Optional[int] = None, **kwargs):

# TODO: how should we handle this -- multiple things in file, but the user cannot choose.
if hdu is None:
for i in range(len(hdulist)):
for i in range(1, len(hdulist)):
if hdulist[i].header.get("DATASUM") != "0":
hdu = i
warnings.warn(
'HDU not specified. Loading spectrum at (HDU{})'.
format(i), AstropyUserWarning)
break

return _load_mwmVisit_or_mwmStar_hdu(hdulist, hdu, **kwargs)
# load spectra and return
return _load_mwmVisit_or_mwmStar_hdu(hdulist, hdu)


@data_loader(
"SDSS-V mwm multi",
"SDSS-V mwm",
identifier=mwm_identify,
force=True,
dtype=SpectrumList,
priority=20,
extensions=["fits"],
Expand All @@ -485,12 +501,8 @@ def load_sdss_mwm_list(file_obj, **kwargs):
Returns
-------
SpectrumList
The spectra contained in the file, where:
Spectrum1D
A given spectra of nD flux
None
If there are no spectra for that spectrograph/observatory
SpectrumList[Spectrum1D]
A list spectra from each visit with each instrument at each observatory (mwmVisit), or the coadd from each instrument/observatory (mwmStar).
"""
spectra = SpectrumList()
with read_fileobj_or_hdulist(file_obj, memmap=False, **kwargs) as hdulist:
Expand All @@ -505,10 +517,6 @@ def load_sdss_mwm_list(file_obj, **kwargs):
for hdu in range(1, len(hdulist)):
if hdulist[hdu].header.get("DATASUM") == "0":
# Skip zero data HDU's
# TODO: validate if we want this printed warning or not.
# it might get annoying & fill logs with useless alerts.
print("WARNING: HDU{} ({}) is empty.".format(
hdu, hdulist[hdu].name))
continue
spectra.append(_load_mwmVisit_or_mwmStar_hdu(hdulist, hdu))
return spectra
Expand All @@ -527,8 +535,8 @@ def _load_mwmVisit_or_mwmStar_hdu(hdulist: HDUList, hdu: int, **kwargs):
Returns
-------
Spectrum1D
The spectrum with nD flux contained in the HDU.
list[Spectrum1D]
List of spectrum with 1D flux contained in the HDU.
"""
if hdulist[hdu].header.get("DATASUM") == "0":
Expand Down Expand Up @@ -563,12 +571,11 @@ def _load_mwmVisit_or_mwmStar_hdu(hdulist: HDUList, hdu: int, **kwargs):
mask = mask != 0

# collapse shape if 1D spectra in 2D array
# NOTE: this fixes a jdaviz handling bug for 2D of shape 1,
# it could be that it's expected to be parsed this way.
if flux.shape[0] == 1:
flux = flux[0]
e_flux = e_flux[0]
mask = mask[0]
flux = np.ravel(flux)
e_flux = e_flux[0] # different class
mask = np.ravel(mask)

# Create metadata
meta = dict()
Expand All @@ -578,24 +585,26 @@ def _load_mwmVisit_or_mwmStar_hdu(hdulist: HDUList, hdu: int, **kwargs):
meta["snr"] = np.array(hdulist[hdu].data["snr"])

# Add identifiers (obj, telescope, mjd, datatype)
# TODO: need to see what metadata we're interested in for the MWM files.
meta["telescope"] = hdulist[hdu].data["telescope"]
meta["instrument"] = hdulist[hdu].header.get("INSTRMNT")
try:
try: # get obj if exists
meta["obj"] = hdulist[hdu].data["obj"]
except KeyError:
pass

# choose between mwmVisit/Star via KeyError except
try:
meta["date"] = hdulist[hdu].data["date_obs"]
meta["mjd"] = hdulist[hdu].data["mjd"]
meta['mjd'] = hdulist[hdu].data['mjd']
meta["datatype"] = "mwmVisit"
except KeyError:
meta["mjd"] = (str(hdulist[hdu].data["min_mjd"][0]) + "->" +
str(hdulist[hdu].data["max_mjd"][0]))
meta["min_mjd"] = str(hdulist[hdu].data["min_mjd"][0])
meta["max_mjd"] = str(hdulist[hdu].data["max_mjd"][0])
meta["datatype"] = "mwmStar"
finally:
meta["name"] = hdulist[hdu].name
meta["sdss_id"] = hdulist[hdu].data['sdss_id']

# drop back a list of Spectrum1Ds to unpack
return Spectrum1D(
spectral_axis=spectral_axis,
flux=flux,
Expand Down
Loading

0 comments on commit c4b90ed

Please sign in to comment.