AL-850: adding tweakreg into stcal

[emolter]

Resolves AL-850
Partially resolves JP-3668

This PR moves the majority of the tweakreg functionality from JWST into stcal, such that it can be used by both Roman and JWST without needing to be maintained in both places. This relates to the alignment between images and to an absolute source catalog only, but not to finding sources within images - this is handled differently by the two observatories, and is split into its own step for Roman.

Several WCS-related utilities have also been moved into alignment or modified therein. These should also eventually be called by JWST and Roman from stcal and removed in those repositories.

Checklist

• added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
• updated relevant tests
• updated relevant documentation
• updated relevant milestone(s)
• added relevant label(s)

[codecov]

Codecov Report

Attention: Patch coverage is 85.60794% with 58 lines in your changes missing coverage. Please review.

Project coverage is 84.01%. Comparing base (5772579) to head (37ea975).
Report is 196 commits behind head on main.

Files with missing lines	Patch %	Lines
src/stcal/tweakreg/tweakreg.py	68.64%	37 Missing ⚠️
src/stcal/alignment/util.py	79.48%	8 Missing ⚠️
src/stcal/tweakreg/utils.py	63.15%	7 Missing ⚠️
src/stcal/tweakreg/astrometric_utils.py	90.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #267      +/-   ##
==========================================
+ Coverage   83.79%   84.01%   +0.22%     
==========================================
  Files          35       39       +4     
  Lines        6998     7345     +347     
==========================================
+ Hits         5864     6171     +307     
- Misses       1134     1174      +40     

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

[braingram]

Thanks for all your work on this.

[braingram]

One more change. @emolter would you change the "unreleased" CHANGES.rst version to 1.8.0 for this PR?

[emolter]

Do not merge at this time. somehow one of the last small changes broke s_region values in jwst. looking into it

[emolter]

Long story short, isinstance(wcsinfo, Wcsinfo) returns False for JWST wcsinfo objects, of type ObjectNode. However, if I do

for attr in ["ra_ref", "dec_ref", ...]:
    assert hasattr(wcsinfo, attr)

all of the assert statements pass. The documentation and other snooping I've done seem to indicate that a runtime_checkable protocol should be basically shorthand for all the hasattr statements, so I'm pretty confused about what's happening there. My only idea (based on PEP 544) is that these ObjectNode objects are not initialized until called explicitly, but I don't know enough about those objects to be sure.

I tried updating all the JWST code to pass in wcsinfo as a dict in all cases (using wcsinfo.instance), but this leads to the issue that updating keywords in the wcsinfo object via e.g. update_s_region_keyword becomes cumbersome and IMO unnecessarily verbose (it becomes necessary to reassign all the values from the dict to the wcsinfo object after the fact).

While perhaps not perfect, changing back to if not isinstance(wcsinfo, dict): achieves the desired behavior. While technically the not would make possible passing any other type through, the type hint (still using the custom Wcsinfo protocol but no longer runtime checkable) specifies what attributes a wcsinfo object needs to have for these routines to run properly. Let me know if that solution sounds ok to you @braingram.

edit: still please do not merge until fully tested with jwst regtests

[emolter]

Ok, all the regression tests for the JWST repo are now passing. It may be prudent to wait for more approvals of that PR, just in case changes are requested that require updating the stcal side for some reason. Other than that, this is ready to merge. @zacharyburnett what is the status of the romancal release - is it now safe to merge into stcal? @braingram are you happy with the changes now, and if so, can you remove the "changes requested" status?

[zacharyburnett]

nice addition!

[braingram]

This object (and DataModel) isn't added by this PR but I think could be removed with the changes in this PR. I'm not suggesting that we remove these as part of this PR. I opened an issue to track their removal: #273

[braingram]

Thanks!

[nden]

I think this function needs to replicate what the jwst version does, which is the most general way. The use case that this is missing is passing sliced_wcs.SlicedLowLevelWCS object. It's a JWST use case, that was requested by external users. There are tests for this in jwst:
https://github.com/spacetelescope/jwst/blob/master/jwst/resample/tests/test_utils.py#L191
Were they not moved to stcal or am I not seeing them?

[emolter]

@nden Thanks for the comment. I think the first link goes to something unrelated, unless I'm missing something. There are two reproject functions in jwst, one in assign_wcs.util and one in resample.resample_utils. The one in resample is not affected by this PR, and is the one that those tests cover.

In my opinion, consolidating these functions falls under the scope of this issue and does not need to be done here. But I can work on changing it if you like

[mairanteodoro]

Looks good to me. Thanks, @emolter!