Skip to content

AL-1042: Add a distinct corrector for Roman#243

Merged
mcara merged 7 commits intospacetelescope:mainfrom
mcara:roman-gwcs
May 8, 2026
Merged

AL-1042: Add a distinct corrector for Roman#243
mcara merged 7 commits intospacetelescope:mainfrom
mcara:roman-gwcs

Conversation

@mcara
Copy link
Copy Markdown
Member

@mcara mcara commented Mar 6, 2026

Add a roman-specific corrector class with unique romantics string.

@mcara mcara added this to the Better Awsomeness milestone Mar 6, 2026
@mcara mcara requested a review from PaulHuwe March 6, 2026 07:22
@mcara mcara self-assigned this Mar 6, 2026
@mcara mcara requested a review from a team as a code owner March 6, 2026 07:22
@mcara mcara added the enhancement New feature or request label Mar 6, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.51%. Comparing base (b3a32ea) to head (f58bb46).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #243      +/-   ##
==========================================
- Coverage   93.53%   93.51%   -0.02%     
==========================================
  Files          11       11              
  Lines        2011     2021      +10     
==========================================
+ Hits         1881     1890       +9     
- Misses        130      131       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@braingram braingram left a comment

Choose a reason for hiding this comment

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

LGTM. 2 minor comments that don't need to hold up approval.

Comment thread tweakwcs/tests/test_multichip_jwst.py Outdated
Comment thread CHANGELOG.rst
@mcara mcara changed the title Add a distinct corrector for Roman AL-1042: Add a distinct corrector for Roman Mar 6, 2026
Copy link
Copy Markdown

@PaulHuwe PaulHuwe left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@mcara
Copy link
Copy Markdown
Member Author

mcara commented Mar 9, 2026

I actually do wonder if it would be better to move the new class to romancal - it actually seems the most logical place.

@mairanteodoro
Copy link
Copy Markdown

I actually do wonder if it would be better to move the new class to romancal - it actually seems the most logical place.

If the new class is supposed to have the same purpose and functionality as that for JWST, I don't think it should be moved to romancal.

@mcara mcara requested review from PaulHuwe and braingram March 18, 2026 14:34
Comment thread tweakwcs/tests/conftest.py
Comment thread tweakwcs/__init__.py Outdated
@mcara mcara requested a review from braingram March 19, 2026 06:13
@braingram
Copy link
Copy Markdown
Contributor

Thanks for making the updates. Code LGTM. Would you run and link regression tests for jwst and roman that use this PR?

Copy link
Copy Markdown

@PaulHuwe PaulHuwe left a comment

Choose a reason for hiding this comment

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

LGTM

@mcara
Copy link
Copy Markdown
Member Author

mcara commented Mar 24, 2026

@mcara
Copy link
Copy Markdown
Member Author

mcara commented Mar 24, 2026

Thanks for making the updates. Code LGTM. Would you run and link regression tests for jwst and roman that use this PR?

Done

@mcara
Copy link
Copy Markdown
Member Author

mcara commented May 5, 2026

Re-running regression tests for pipelines using this branch as dependency. It does not test that new correcto for romancal works in the pipeline (those tests will be run in pipeline PRs). It just shows that merging this PR will not affect pipelines:

@mcara
Copy link
Copy Markdown
Member Author

mcara commented May 5, 2026

Regression tests pass but CI style check is failing with numerous warnings in unrelated files possibly caused by #247. Should I revert that PR?

@mcara mcara merged commit d1726d5 into spacetelescope:main May 8, 2026
28 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants