spacetelescope / stdatamodels

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

Update MultiSpec schema to provide keywords for recording extraction characteristics #264

Closed jemorrison closed 4 months ago

jemorrison commented 5 months ago

Partially resolves JP-3169 <!Resolves RCAL-nnnn -->

Is needed to close spacetelescope/jwst#7773

This PR updates the MultiSpec model to include the xstart, xstop, ystart, ystop values (if they are defined) so they can be recorded in the FITS header

The extraction x center and y center were already defined with keyword values of EXTR_X, EXTR_Y. Since the extraction locations are related to these values I picked EXTRXSTR: x start of extraction region EXTRXSTP: x stop of extraction region EXTRYSTR: y start of extraction region EXTRYSTP: y stop of extraction region

But these value can be changed if a better set can be found

Checklist

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.87%. Comparing base (4d7c3a6) to head (bb886a7). Report is 2 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #264 +/- ## ========================================== + Coverage 64.84% 64.87% +0.03% ========================================== Files 103 104 +1 Lines 5694 5699 +5 ========================================== + Hits 3692 3697 +5 Misses 2002 2002 ```

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

jemorrison commented 5 months ago

@hbushouse @tapastro I could not add you to the list of reviewers. Are the names of the FITS Keywords for these parameters ok to you. This PR will need to be merged before the extract 1d code updates will be able to use them

hbushouse commented 4 months ago

We already have a couple of similar sets of keywords that give region limits:

SUBSTRT1, SUBSTRT2, SUBSIZE1, SUBSIZE2 - which record the location and size of subarrays relative to the full detector SLTSTRT1, SLTSTRT2, SLTSIZE1, SLTSIZE2 - which record the location and size of 2-D cutouts performed in extract_2d

These work a bit differently, in that instead of including explicit start/stop values, they only specify the start location and then record the size.

So if we want keywords that record start/stop values, we can't just duplicate these existing sets by simply changing the prefix (i.e. use "EXTR" instead of "SUB" and "SLT"). But I'm wondering if we should at least conform to the same use of axis numbers in the keywords, instead of using "x" and "y"? Unfortunately we already have a precedent set with the "EXTR_X" and "EXTR_Y" keywords that are used to record the center of the circular apertures used to make 1-D extractions from IFU cubes. So ... not sure what the best naming convention should be for these new keywords.

Not a big fan of the fact that "EXTRXSTR" and "EXTRXSTP" are so similar looking until you see the difference in the very last letter. But I honestly don't have many better suggestions. So I guess, after all of this, I'm OK with the proposed keyword names.