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-139: Add Members Keyword to Resample Schema #396

Merged
merged 3 commits into from
Mar 29, 2024

Conversation

PaulHuwe
Copy link
Collaborator

@PaulHuwe PaulHuwe commented Mar 26, 2024

Resolves RAD-139

Closes #340

This PR adds the members keyword to the resample schema.

Checklist

Copy link

codecov bot commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.38%. Comparing base (0fb1239) to head (4e5ebe5).
Report is 11 commits behind head on main.

❗ Current head 4e5ebe5 differs from pull request most recent head 3f397d8. Consider uploading reports for the commit 3f397d8 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #396   +/-   ##
=======================================
  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.

@PaulHuwe PaulHuwe marked this pull request as ready for review March 26, 2024 13:59
@PaulHuwe PaulHuwe requested review from kdupriestsci, jbrookens and a team as code owners March 26, 2024 13:59
@schlafly
Copy link
Collaborator

Do we expect the members to already be included as individual_image_meta.basic.filename? It is good to include this somewhere, and we aren't currently populating individual_image_meta.basic.filename, but once is probably enough?

Copy link
Collaborator

@ddavis-stsci ddavis-stsci left a comment

Choose a reason for hiding this comment

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

Looks good

@PaulHuwe
Copy link
Collaborator Author

Do we expect the members to already be included as individual_image_meta.basic.filename? It is good to include this somewhere, and we aren't currently populating individual_image_meta.basic.filename, but once is probably enough?

Yes, I would expect this information to be in individual_image_meta.basic['filename'] (basic is a table). The reason for this ticket is that romancal does set this keyword value (e.g. https://github.com/spacetelescope/romancal/blob/main/romancal/resample/resample.py#L311), so it should be added to the schema. I have not gone through romancal to see if removing that entirely would affect other steps or not.

@schlafly
Copy link
Collaborator

Okay, fine to duplicate this.

@PaulHuwe PaulHuwe merged commit 34b997a into spacetelescope:main Mar 29, 2024
11 checks passed
@nden nden added this to the Build 14 milestone Mar 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Members Keyword to Resample Schema
5 participants