spacetelescope / stdatamodels

https://stdatamodels.readthedocs.io
Other
5 stars 24 forks source link

JP-3482: Load units from FITS default header keywords directly #238

Closed tapastro closed 8 months ago

tapastro commented 9 months ago

Resolves JP-3482

Closes [spacetelescope/jwst#8109]

This PR addresses a lack of specified units in the spec/spectable jwst schemas, resulting in metadata not propagating through a save/load cycle. Additionally, loading a spectrum into a datamodel will not have any unit strings accessible. This feels like hacky workaround, but does generate unit strings in the spec_table. Looking forward to expert review 🙂

Checklist

codecov[bot] commented 9 months ago

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (12a4c3a) 64.71% compared to head (e5dfeb8) 64.62%.

Files Patch % Lines
src/stdatamodels/jwst/datamodels/multispec.py 35.29% 11 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #238 +/- ## ========================================== - Coverage 64.71% 64.62% -0.09% ========================================== Files 102 102 Lines 5668 5685 +17 ========================================== + Hits 3668 3674 +6 - Misses 2000 2011 +11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

hbushouse commented 8 months ago

Is this being superseded by #243 and hence can be closed?

braingram commented 8 months ago

I believe the open PR #242 supersedes this (I'm not sure if the test PR #243 fixes the issue here and is more an example of one way to define units to dicuss in issue #240). @tapastro does #242 look good to you?

braingram commented 8 months ago

Not enough coffee yet this morning... I clicked merge on #242 and then realized I hadn't yet heard back on my last comment. @tapastro does #242 solve the issue and if so are you ok with closing this PR?

tapastro commented 8 months ago

This has definitely been superseded and can be closed!