-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update MultiSpec schema to provide keywords for recording extraction characteristics #264
Update MultiSpec schema to provide keywords for recording extraction characteristics #264
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ 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 ☔ View full report in Codecov by Sentry. |
@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 |
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 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a change log entry
f283136
to
378aa9e
Compare
Partially resolves JP-3169
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
CHANGES.rst
(either inBug Fixes
orChanges to API
)