-
Notifications
You must be signed in to change notification settings - Fork 28
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
Populate segmentation model metadata for exposures and mosaics. #1391
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1391 +/- ##
==========================================
- Coverage 78.48% 78.48% -0.01%
==========================================
Files 117 117
Lines 7861 7868 +7
==========================================
+ Hits 6170 6175 +5
- Misses 1691 1693 +2
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…sociation name for output meta.filename by default in resample.
Regtests are now passing: https://github.com/spacetelescope/RegressionTests/actions/runs/10600468693/job/29383387213 |
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 see one issue with this so far. If I run the code with a json file I get the expected three files,
r0099101001001001001_r274dp63x31y81_prompt_F158_cat.asdf
r0099101001001001001_r274dp63x31y81_prompt_F158_i2d.asdf
r0099101001001001001_r274dp63x31y81_prompt_F158_segm.asdf
If I change the product name and rerun the pipeline I also get the expected three files,
r0099101001001001001_user_choice_cat.asdf
r0099101001001001001_user_choice_i2d.asdf
r0099101001001001001_user_choice_segm.asdf
the issue is that this seems to remove the i2d file from the run before and I now see only
ls -1 r0099101001001001001_r274dp63x31y81_prompt_F158*.asdf
r0099101001001001001_r274dp63x31y81_prompt_F158_cat.asdf
r0099101001001001001_r274dp63x31y81_prompt_F158_segm.asdf
I don't think we should be deleting files from a previous run.
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.
The latest changes fixed the file deletion issue so LGTM
Great. regtests also passed, so I'm merging. |
Closes #1389
We were previously populating segmentation image metadata with filler values that were not related to the input L2 or L3 image. This PR adds code that copies the metadata from the catalogs into the segmentation images.
This now also fixes some file naming issues; the mosaic pipeline was saving the catalogs, as well as the step saving the catalogs, leading to two copies of the catalogs with different file names. Now only the step saves the catalogs. I have also changed some of the logic around the construction of the file names.
Regtests running here: https://github.com/spacetelescope/RegressionTests/actions/runs/10600468693/job/29383387213
Checklist
CHANGES.rst
under the corresponding subsection