JP-3668: Move tweakreg into stcal #8624

emolter · 2024-07-03T18:25:51Z

The portions of the tweakreg step that are shared between jwst and romancal are being moved into the stcal repository; see this stcal PR. This PR updates the jwst pipeline accordingly.

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

added entry in CHANGES.rst within the relevant release section
updated or added relevant tests
updated relevant documentation
added relevant milestone
added relevant label(s)
ran regression tests, post a link to the Jenkins job below.
How to run regression tests on a PR
All comments are resolved
Make sure the JIRA ticket is resolved properly

codecov · 2024-07-03T19:00:20Z

Codecov Report

Attention: Patch coverage is 79.62963% with 11 lines in your changes missing coverage. Please review.

Project coverage is 60.05%. Comparing base (d9a59d2) to head (94ce74e).

Files with missing lines	Patch %	Lines
jwst/resample/resample_utils.py	33.33%	6 Missing ⚠️
jwst/tweakreg/tweakreg_step.py	84.84%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8624      +/-   ##
==========================================
- Coverage   60.21%   60.05%   -0.17%     
==========================================
  Files         370      369       -1     
  Lines       38635    38409     -226     
==========================================
- Hits        23265    23065     -200     
+ Misses      15370    15344      -26

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

emolter · 2024-07-10T16:39:28Z

regression tests started here. Still can't figure out why the singular unit test is failing, but if regtests turn up more failures that might give more information.

emolter · 2024-07-11T14:18:50Z

one-liner change to make the version of update_s_region_imaging in stcal identical to what used to be in the jwst repo did indeed fix the regtest failures. Unfortunately, these seem unrelated to the single unit test failure tweakreg/tests/test_multichip_jwst.py:test_multichip_alignment_step. Still tracking that one down.

emolter · 2024-07-11T15:20:57Z

@mcara RE the one unit test that is failing: I still don't quite understand what it is trying to do, exactly, but I put the results of some initial exploration of what could be going wrong into a docstring on the test itself. I'm not asking you to diagnose this in detail for me, necessarily, but based on those notes perhaps you can suggest where things might be going wrong or what else I should check.

mcara

Failing tests are due to monkeypatch not patching align_wcs.

jwst/tweakreg/tests/test_multichip_jwst.py

mcara

I will withhold my final review pending review of the sister PR in stcal. Both PRs look good but I would like to look into the issue of data model interface. At this time this PR is "Approved" but I do not want it merged yet.

mcara

LGTM

emolter · 2024-07-18T12:53:02Z

started new regression test run here

emolter · 2024-07-22T17:16:05Z

new regression tests started here, after minor changes to both stcal and jwst sides.

edit: lots of failing regression tests due to very small differences in the S_REGION. looking into the reason

emolter · 2024-07-23T23:46:37Z

Another new set of regtests here

emolter · 2024-07-24T22:11:03Z

regression test run covering fix of the final few failing S_REGION issues - all passing

melanieclarke

I'll defer to all the expert reviews already done for this change. From a cursory review, this looks ready to go to me.

It looks like pyproject.toml needs an update after the stcal change is merged, so I'll hold off on approval, pending that coordination.

nden

LGTM

emolter · 2024-07-25T18:53:27Z

I'll defer to all the expert reviews already done for this change. From a cursory review, this looks ready to go to me.

It looks like pyproject.toml needs an update after the stcal change is merged, so I'll hold off on approval, pending that coordination.

I just merged the stcal side since Nadia gave the approval. The correct thing to pin is stcal@main, right? And wait for an stcal release to change it to 1.8.0 later? @melanieclarke

melanieclarke · 2024-07-25T18:57:44Z

I just merged the stcal side since Nadia gave the approval. The correct thing to pin is stcal@main, right? And wait for an stcal release to change it to 1.8.0 later? @melanieclarke

Sounds right to me. @braingram?

pyproject.toml

Co-authored-by: Brett Graham <[email protected]>

melanieclarke

Okay, all looks good now!

github-actions bot added assign_wcs resample tweakreg testing installation labels Jul 3, 2024

emolter force-pushed the JP-3668 branch from fc20b8c to 88d9357 Compare July 3, 2024 18:35

emolter force-pushed the JP-3668 branch from d73ef30 to 2c80121 Compare July 9, 2024 22:05

github-actions bot added cube_build documentation labels Jul 9, 2024

emolter force-pushed the JP-3668 branch from 2c80121 to 0c0bd31 Compare July 10, 2024 16:31

emolter force-pushed the JP-3668 branch from 0c0bd31 to 4ee4d0a Compare July 11, 2024 13:58

emolter requested review from braingram and mcara July 11, 2024 15:18

emolter added this to the Build 11.1 milestone Jul 11, 2024

mcara requested changes Jul 12, 2024

View reviewed changes

jwst/tweakreg/tests/test_multichip_jwst.py Outdated Show resolved Hide resolved

jwst/tweakreg/tests/test_multichip_jwst.py Outdated Show resolved Hide resolved

jwst/tweakreg/tests/test_multichip_jwst.py Outdated Show resolved Hide resolved

emolter marked this pull request as ready for review July 12, 2024 13:12

emolter requested a review from a team as a code owner July 12, 2024 13:12

mcara reviewed Jul 12, 2024

View reviewed changes

emolter force-pushed the JP-3668 branch from 615ee6a to 1706240 Compare July 15, 2024 17:55

emolter mentioned this pull request Jul 15, 2024

AL-850: adding tweakreg into stcal spacetelescope/stcal#267

Merged

5 tasks

emolter force-pushed the JP-3668 branch from 65e80b4 to 5e5fd44 Compare July 17, 2024 14:20

mcara approved these changes Jul 18, 2024

View reviewed changes

emolter force-pushed the JP-3668 branch from f483bfc to fae7dd0 Compare July 22, 2024 16:04

emolter added 16 commits July 24, 2024 14:53

added changelog entry, small style fixes to resample utils

e56d172

removing some methods that were already moved to stcal

8432443

attempted fix failed unit tests

0aa683f

reverted unwanted changes to update_fits_wcsinfo imports

467e8b1

fixed failing unit test per @mcara suggestion

5521088

small cleanup to test

ca0ae89

accommodate refactor of stcal to remove dependence on datamodels

19cae8a

fixed failing unit test

5713d83

fixes based on requested variable name changes in stcal

05f0b7f

passing wcsinfo.instance to stcal

5c96fe5

missed one

c1135cf

missed another one

c9237a3

revert .instance passing of wcsinfo into stcal

89db226

replacing update_s_region functions with compute_s_region

9de2702

fixing test failures for ifu modes due to typo

260352f

fixed handling of NaNs in footprint case for s_region

14bdd92

emolter force-pushed the JP-3668 branch from 1c87861 to 14bdd92 Compare July 24, 2024 22:07

streamlining update_s_region again

d93def6

melanieclarke reviewed Jul 25, 2024

View reviewed changes

nden approved these changes Jul 25, 2024

View reviewed changes

bumped stcal dependency

3c77205

braingram reviewed Jul 25, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

Update pyproject.toml

94ce74e

Co-authored-by: Brett Graham <[email protected]>

melanieclarke approved these changes Jul 25, 2024

View reviewed changes

emolter merged commit 859ac8e into spacetelescope:master Jul 25, 2024
24 checks passed

emolter deleted the JP-3668 branch July 25, 2024 20:02

stscijgbot-jp mentioned this pull request Jul 25, 2024

tweakreg to stcal #8594

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JP-3668: Move tweakreg into stcal #8624

JP-3668: Move tweakreg into stcal #8624

emolter commented Jul 3, 2024 •

edited by melanieclarke

Loading

codecov bot commented Jul 3, 2024 •

edited

Loading

emolter commented Jul 10, 2024

emolter commented Jul 11, 2024

emolter commented Jul 11, 2024

mcara left a comment

mcara left a comment

mcara left a comment

emolter commented Jul 18, 2024

emolter commented Jul 22, 2024 •

edited

Loading

emolter commented Jul 23, 2024

emolter commented Jul 24, 2024 •

edited

Loading

melanieclarke left a comment

nden left a comment

emolter commented Jul 25, 2024

melanieclarke commented Jul 25, 2024

melanieclarke left a comment

JP-3668: Move tweakreg into stcal #8624

JP-3668: Move tweakreg into stcal #8624

Conversation

emolter commented Jul 3, 2024 • edited by melanieclarke Loading

codecov bot commented Jul 3, 2024 • edited Loading

Codecov Report

emolter commented Jul 10, 2024

emolter commented Jul 11, 2024

emolter commented Jul 11, 2024

mcara left a comment

Choose a reason for hiding this comment

mcara left a comment

Choose a reason for hiding this comment

mcara left a comment

Choose a reason for hiding this comment

emolter commented Jul 18, 2024

emolter commented Jul 22, 2024 • edited Loading

emolter commented Jul 23, 2024

emolter commented Jul 24, 2024 • edited Loading

melanieclarke left a comment

Choose a reason for hiding this comment

nden left a comment

Choose a reason for hiding this comment

emolter commented Jul 25, 2024

melanieclarke commented Jul 25, 2024

melanieclarke left a comment

Choose a reason for hiding this comment

emolter commented Jul 3, 2024 •

edited by melanieclarke

Loading

codecov bot commented Jul 3, 2024 •

edited

Loading

emolter commented Jul 22, 2024 •

edited

Loading

emolter commented Jul 24, 2024 •

edited

Loading