JP-2113: Update NIRCam WFSS transforms for field dependence (v6) #124
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #124 +/- ##
==========================================
- Coverage 65.03% 63.88% -1.15%
==========================================
Files 99 99
Lines 5385 5486 +101
==========================================
+ Hits 3502 3505 +3
- Misses 1883 1981 +98
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I noticed a potential issue with the proposed schema changes.
Basically one needs to check that currently written jwst files can be opened without issue with the changes proposed in this PR. If there are issues, then my suggestions below may help fix those problems.
| type: array | ||
| items: | ||
| type: object |
There was a problem hiding this comment.
This will likely be incompatible with existing files as they will contain a single instance of the object rather than an array containing a single object. This may cause a validation error when reading such files.
If there is a validation error with existing files then
| type: array | |
| items: | |
| type: object | |
| oneOf: | |
| - type: object | |
| - type: array | |
| items: | |
| type: object |
Should resolve that error. This will apply to all the similar changes in this file.
There was a problem hiding this comment.
When written (a few months ago), there was no intent to preserve backwards compatibility as the prior versions were barely "science-valid". I don't know if this stance has changed, but other parts of the code are also implicitly backwards incompatible and would break if used with older references. Will be something to discuss once this is closer to review.
| @@ -20,19 +20,22 @@ class NIRCAMGrismDispersionConverter(TransformConverterBase): | |||
| def from_yaml_tree_transform(self, node, tag, ctx): | |||
| from stdatamodels.jwst.transforms import models | |||
|
|
|||
| _fname = getattr(models, node["class_name"]) | |||
| _fname = getattr(models, node["model_type"]) | |||
There was a problem hiding this comment.
Additional changes maybe necessary if my suggested changes to the schema are needed. Namely, it may need to branch on single instances vs lists of instances.
zacharyburnett
force-pushed
the
jp-2113-nircam-wfss-v6
branch
from
4ea88ea to
0c4f917
Compare
tapastro
force-pushed
the
jp-2113-nircam-wfss-v6
branch
from
0c4f917 to
11511f4
Compare
tapastro
force-pushed
the
jp-2113-nircam-wfss-v6
branch
from
e5acc82 to
731f972
Compare
|
I think this broke Jdaviz. See #159 |
Resolves JP-2113
Resolves RCAL-nnnn
Closes #
This PR provides higher-order polynomial transform models for the NIRCam WFSS grisms. It has a coupled PR in jwst (spacetelescope/jwst#7018).
Checklist
CHANGES.rst(either inBug FixesorChanges to API)