-
Notifications
You must be signed in to change notification settings - Fork 21
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
RCAL-701: Level 3 Mosaic Update #288
Conversation
for more information, see https://pre-commit.ci
…an_datamodels into RCAL-701_L3Metadata
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
==========================================
+ Coverage 97.16% 97.48% +0.32%
==========================================
Files 28 29 +1
Lines 2542 2709 +167
==========================================
+ Hits 2470 2641 +171
+ Misses 72 68 -4 ☔ View full report in Codecov by Sentry. |
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.
FYI, this all looks good to me, but let's not merge until seeing what needs to happen in romancal. Thanks!
33f798f
to
1c9181c
Compare
The old dependency failure is expected, due to the astropy Table tag version. |
Any reason to not bump |
5471575
to
58dbabc
Compare
0bc773f
to
4347968
Compare
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.
This looks good to me. I am leery of actually using the append_individual_image_meta stuff until we have actually tried running it on real images; I worry about weird things in the metadata like ndarrays from source detection, etc.. But it's fine for the moment since we haven't actually turned that on in romancal, I think? Can you and @stscieisenhamer coordinate to merge these and get the L3 bits working with the new schema? Thank you!
72bd5ec
to
853e477
Compare
a01410f
to
a547611
Compare
e91f1bb
to
94e5fba
Compare
edf17f9
to
d68795a
Compare
for more information, see https://pre-commit.ci
setup default for mosaic associations
1cbecb9
to
f57cc4d
Compare
for more information, see https://pre-commit.ci
initialize coordinates and ref_file metas
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.
LGTM, you don't need to change anything.
roman_datamodels.stnode.MosaicAssociations | ||
""" | ||
|
||
mosass = stnode.MosaicAssociations() |
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.
Just an observation: we usually we abbreviate association as asn so mosasn instead of mosass.
You did get a good laugh out of me.
…dels into RCAL-701_L3Metadata
Resolves RCAL-701
Closes #
This PR updates the contents of the WFI_Mosaic datamodel, maker utilities, and tests to a more streamlined metadata design for level 3 products.
This update is expected to require new RomanCAL regression test files, so there will be no jenkins link below. There is a RomanCAL ticket for the code dependency changes: spacetelescope/romancal#1057
Checklist
CHANGES.rst
under the corresponding subsection