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

Rcal 803 Initial resampling to skycell in the hlp #1214

Merged
merged 9 commits into from
May 3, 2024

Conversation

ddavis-stsci
Copy link
Collaborator

@ddavis-stsci ddavis-stsci commented May 2, 2024

Resolves RCAL-803

Closes #1162

This PR adds the ability to use an association to resample the members to the output product based on a skycell definition.

Checklist

@ddavis-stsci ddavis-stsci added this to the 24Q3_B14 milestone May 2, 2024
@ddavis-stsci ddavis-stsci self-assigned this May 2, 2024
@ddavis-stsci ddavis-stsci requested a review from a team as a code owner May 2, 2024 19:51
Copy link

codecov bot commented May 2, 2024

Codecov Report

Attention: Patch coverage is 17.24138% with 48 lines in your changes are missing coverage. Please review.

Project coverage is 79.16%. Comparing base (b61e70b) to head (12825be).
Report is 3 commits behind head on main.

Files Patch % Lines
romancal/pipeline/highlevel_pipeline.py 17.24% 48 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1214      +/-   ##
==========================================
+ Coverage   79.05%   79.16%   +0.10%     
==========================================
  Files         116      116              
  Lines        7994     8045      +51     
==========================================
+ Hits         6320     6369      +49     
- Misses       1674     1676       +2     
Flag Coverage Δ *Carryforward flag
nightly 62.78% <ø> (ø) Carriedforward from 72342b4

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@schlafly schlafly left a 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.

Some notes:

  • I hadn't thought about this before, but it's a little ~interesting that we are doing outlier detection & sky matching using the full SCAs, but the projection using only the skycell area. For the outlier detection that's wasteful, but probably not horribly so. For the sky matching it produces different results, but probably different results in a mildly beneficial way. I think we should just document that behavior for now.
  • I agree with the code comments that we'll probably want to refactor this to get resample to take a WCS directly rather than via a file.
  • I think I had the impression that you wanted to add some associated regtest demonstrating the L3 pipeline on skycells?

@ddavis-stsci ddavis-stsci merged commit 48aa506 into spacetelescope:main May 3, 2024
29 of 30 checks passed
@ddavis-stsci ddavis-stsci deleted the rcal-803_dsd branch May 3, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HLP add ability to resample to projection cells
2 participants