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

Remove unnecessary global variable and unit tests. #1314

Merged

Conversation

mairanteodoro
Copy link
Collaborator

@mairanteodoro mairanteodoro commented Jul 18, 2024

Closes #1306

This PR eliminates the global variable ALIGN_TO_ABS_REFCAT from TweakRegStep and the two unit tests that used it. Since TweakRegStep is always used for absolute astrometry, there is no need to have that global variable with fixed value.

Regression Tests
All tests are passing:

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

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

Attention: Patch coverage is 93.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.28%. Comparing base (af07aa2) to head (344a1c2).
Report is 248 commits behind head on main.

Files with missing lines Patch % Lines
romancal/tweakreg/tweakreg_step.py 93.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1314      +/-   ##
==========================================
- Coverage   79.37%   79.28%   -0.10%     
==========================================
  Files         117      117              
  Lines        8086     8065      -21     
==========================================
- Hits         6418     6394      -24     
- Misses       1668     1671       +3     
Flag Coverage Δ *Carryforward flag
nightly 62.98% <ø> (+0.11%) ⬆️ Carriedforward from af07aa2

*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.

@mairanteodoro mairanteodoro marked this pull request as ready for review July 18, 2024 01:04
@mairanteodoro mairanteodoro requested a review from a team as a code owner July 18, 2024 01:04
@mairanteodoro mairanteodoro force-pushed the mteodoro-fix-issue-1306 branch from 6ae25f6 to 344a1c2 Compare July 18, 2024 01:57
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.

LGTM

@mairanteodoro mairanteodoro merged commit 2b19fc4 into spacetelescope:main Jul 18, 2024
30 checks passed
braingram added a commit to braingram/romancal that referenced this pull request Jul 18, 2024
@nden nden added this to the 24Q4_B15 milestone Jul 19, 2024
zacharyburnett pushed a commit to zacharyburnett/romancal that referenced this pull request Jul 22, 2024
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.

Investigate removing ALIGN_TO_ABS_REFCAT from tweakreg
3 participants