Skip to content
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

Merged
merged 22 commits into from
Apr 3, 2024

Conversation

nden
Copy link
Collaborator

@nden nden commented Mar 18, 2024

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

@nden nden requested review from kdupriestsci, jbrookens and a team as code owners March 18, 2024 16:41
@nden nden marked this pull request as draft March 18, 2024 16:41
Copy link

codecov bot commented Mar 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.38%. Comparing base (0fb1239) to head (a5384b6).
Report is 13 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

@nden nden changed the title L2 and L3 catalog and segmentation map schemas RAD-151: L2 and L3 catalog and segmentation map schemas Mar 18, 2024
@nden nden added this to the Build 14 milestone Mar 18, 2024
@nden nden added the Level 4 label Mar 18, 2024
@nden nden mentioned this pull request Mar 18, 2024
4 tasks
@nden
Copy link
Collaborator Author

nden commented Mar 25, 2024

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 archive_catalog field in those schemas.

@nden nden marked this pull request as ready for review March 25, 2024 17:12
@nden nden mentioned this pull request Mar 29, 2024
4 tasks
@jbrookens
Copy link
Collaborator

@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).
For all keywords in photometry, add a new table “SourceCatalog” to the destination.

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.

@nden
Copy link
Collaborator Author

nden commented Apr 1, 2024

@jbrookens Sounds good. Thanks for the fast decision!

$ref: wfi_optical_element-1.0.0
exposure:
title: Exposure Information
tag: asdf://stsci.edu/datamodels/roman/tags/exposure-1.0.0
Copy link
Collaborator

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
Copy link
Collaborator

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?

@nden
Copy link
Collaborator Author

nden commented Apr 3, 2024

@kdupriestsci @jbrookens Sorry, my previous commit was incomplete. I now got confirmation we will need L2 catalogs and I'll make all changes shortly.

@nden
Copy link
Collaborator Author

nden commented Apr 3, 2024

@PaulHuwe @kdupriestsci @jbrookens This is ready now for a final review.

Copy link
Collaborator

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator

@PaulHuwe PaulHuwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nden nden merged commit 6271ba5 into spacetelescope:main Apr 3, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create schemas for source_catalog and segmentation_map
6 participants