-
Notifications
You must be signed in to change notification settings - Fork 22
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
RAD-151: L2 and L3 catalog and segmentation map schemas #393
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #393 +/- ##
=======================================
Coverage 95.38% 95.38%
=======================================
Files 4 4
Lines 195 195
=======================================
Hits 186 186
Misses 9 9 ☔ View full report in Codecov by Sentry. |
I need to think of the best way to include the two time attributed to the level 2 catalogs and then I'll add them (one option is to add the entire exposure schema, though an overkill at the moment, it may end up being simpler after the L1/L2 metadata review). But meanwhile, since all other comments were addressed, I'll open it for a review, particularly to the SDP team, if anything needs to be added to the |
@nden regarding the archive destinations, please follow this design: For all keywords in basic, program, and mosaic_basic, add 2 new tables “SourceCatalog” and “SegmentationMap” to the destination (with the same field name as the others). And if L2 files will also generate source catalogs and segmentation maps: For all keywords in visit, add 2 new tables “SourceCatalog” and “SegmentationMap” to the destination. |
@jbrookens Sounds good. Thanks for the fast decision! |
Co-authored-by: William Jamieson <wjamieson@stsci.edu>
Co-authored-by: Nadia Dencheva <nadia.astropy@gmail.com>
Co-authored-by: Nadia Dencheva <nadia.astropy@gmail.com>
$ref: wfi_optical_element-1.0.0 | ||
exposure: | ||
title: Exposure Information | ||
tag: asdf://stsci.edu/datamodels/roman/tags/exposure-1.0.0 |
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.
I noticed you didn't make any updates to exposure-1.0.0.yaml Does that mean it should be removed from this file?
tag: asdf://stsci.edu/datamodels/roman/tags/program-1.0.0 | ||
visit: | ||
title: Visit Information | ||
tag: asdf://stsci.edu/datamodels/roman/tags/visit-1.0.0 |
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.
I noticed you didn't make any updates to visit-1.0.0.yaml Does that mean it should be removed from this file?
@kdupriestsci @jbrookens Sorry, my previous commit was incomplete. I now got confirmation we will need L2 catalogs and I'll make all changes shortly. |
@PaulHuwe @kdupriestsci @jbrookens This is ready now for a final review. |
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.
I believe exposure should just be SourceCatalog, not SegmentationMap. My suggestion for L2 was:
And if L2 files will also generate source catalogs and segmentation maps:
For all keywords in visit, add 2 new tables “SourceCatalog” and “SegmentationMap” to the destination.
For all keywords in exposure, add a new table “SourceCatalog” to the destination.
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.
Aargh, my bad. Fixed now. Thanks.
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
Resolves RAD-151
Closes #377
This PR includes and expands Brett's code in #374.
It adds schemas for Level 2 and Level 3 segmentation maps and source catalog.
This may change as metadata gets finalized by RTB.
Checklist
CHANGES.rst
under the corresponding subsection