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

Fix bug when setting meta.background in SkyMatchStep. #1233

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mairanteodoro
Copy link
Collaborator

@mairanteodoro mairanteodoro commented May 10, 2024

Resolves RCAL-837

This PR addresses a bug in SkyMatchStep that was causing meta.background.subtracted to always be set to None instead of True/False, which was preventing ResampleStep from properly using the sky level as determined by SkyMatchStep and set in meta.background.level.

The changes to the code also required some refactoring to the unit tests.

The code changes in this PR are also related to this old issue: spacetelescope/rad#247

Regression tests
The only failed test has nothing to do with the changes in this PR.
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/764/
https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/770/

Checklist

  • added entry in CHANGES.rst under the corresponding subsection
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)
  • ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR

@mairanteodoro mairanteodoro added bug Something isn't working and removed testing no-changelog-entry-needed labels May 10, 2024
@mairanteodoro mairanteodoro marked this pull request as ready for review May 10, 2024 14:14
@mairanteodoro mairanteodoro requested a review from a team as a code owner May 10, 2024 14:14
@mairanteodoro mairanteodoro requested review from bmorris3 and schlafly and removed request for a team May 10, 2024 14:15
@mairanteodoro mairanteodoro self-assigned this May 10, 2024
Copy link

codecov bot commented May 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.93%. Comparing base (1aec135) to head (ebc5596).
Report is 6 commits behind head on main.

Current head ebc5596 differs from pull request most recent head d01bf65

Please upload reports for the commit d01bf65 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1233   +/-   ##
=======================================
  Coverage   78.93%   78.93%           
=======================================
  Files         117      117           
  Lines        8086     8086           
=======================================
  Hits         6383     6383           
  Misses       1703     1703           
Flag Coverage Δ *Carryforward flag
nightly 62.78% <ø> (ø) Carriedforward from 1aec135

*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

@bmorris3 bmorris3 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, it was time for this update. Thanks @mairanteodoro!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants