Add support for setting output_wcs of the resample step to an object

[mcara]

Resolves RCAL-909

Closes #1404

This PR adds support for setting output_wcs of the ResampleStep to a gwcs.WCS object.

Tasks

• request a review from someone specific, to avoid making the maintainers review every PR
• add a build milestone, i.e. 24Q4_B15 (use the latest build if not sure)
• Does this PR change user-facing code / API? (if not, label with no-changelog-entry-needed)
  • write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
  • update or add relevant tests
  • update relevant docstrings and / or docs/ page
  • start a regression test and include a link to the running job (click here for instructions)
    • Do truth files need to be updated ("okified")?
      • after the reviewer has approved these changes, run okify_regtests to update the truth files
• if a JIRA ticket exists, make sure it is resolved properly

news fragment change types...

• changes/<PR#>.general.rst: infrastructure or miscellaneous change
• changes/<PR#>.docs.rst
• changes/<PR#>.stpipe.rst
• changes/<PR#>.associations.rst
• changes/<PR#>.scripts.rst
• changes/<PR#>.mosaic_pipeline.rst
• changes/<PR#>.patch_match.rst

steps

• changes/<PR#>.dq_init.rst
• changes/<PR#>.saturation.rst
• changes/<PR#>.refpix.rst
• changes/<PR#>.linearity.rst
• changes/<PR#>.dark_current.rst
• changes/<PR#>.jump_detection.rst
• changes/<PR#>.ramp_fitting.rst
• changes/<PR#>.assign_wcs.rst
• changes/<PR#>.flatfield.rst
• changes/<PR#>.photom.rst
• changes/<PR#>.flux.rst
• changes/<PR#>.source_detection.rst
• changes/<PR#>.tweakreg.rst
• changes/<PR#>.skymatch.rst
• changes/<PR#>.outlier_detection.rst
• changes/<PR#>.resample.rst
• changes/<PR#>.source_catalog.rst

[mcara]

Roman regression tests: https://github.com/spacetelescope/RegressionTests/actions/workflows/romancal.yml

[codecov]

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 80.22%. Comparing base (d2ff8de) to head (453d4f3).

Files with missing lines	Patch %	Lines
romancal/pipeline/mosaic_pipeline.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1631      +/-   ##
==========================================
+ Coverage   80.02%   80.22%   +0.19%     
==========================================
  Files         112      112              
  Lines        6844     6842       -2     
==========================================
+ Hits         5477     5489      +12     
+ Misses       1367     1353      -14     

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

[mcara]

Regression and unit tests pass although I do not know if they cover the code from this PR, in particular, the code from romancal/pipeline/mosaic_pipeline.py.

[ddavis-stsci]

LGTM

[schlafly]

This looks good, thanks. Do you understand the docs build failures? I don't obviously see what in your changes is relevant there, unless it's somehow choking on something in a docstring that I'm missing.

[braingram]

I don't think this works for the mosaic pipeline.

[braingram]

Suggested change

	result = step.process(ModelLibrary(exposure_1))
	result = step.run(ModelLibrary(exposure_1))

The mosaic pipeline uses run (as recommended). If I change this to run the test fails with: stpipe.config_parser.ValidationError: Config parameter 'output_wcs'

[mcara]

This PR addresses specific use case indicated in #1404 (comment).

[braingram]

The comment mentions lines 133-135 in the mosaic pipeline:

romancal/romancal/pipeline/mosaic_pipeline.py

Lines 133 to 135 in d2ff8de

	wcs_file.write_to("skycell_wcs.asdf")
	self.resample.output_wcs = "skycell_wcs.asdf"

A bit further down where resample is called the pipeline uses resample.run:

romancal/romancal/pipeline/mosaic_pipeline.py

Line 148 in d2ff8de

result = self.resample.run(result)

The suggestion here is to update the test you've added to use run instead of process. When I tested this change locally it failed with the error mentioned above.

[braingram]

The issue appears to be this line is stpipe:
https://github.com/spacetelescope/stpipe/blob/47c3867cdab9d8c9fb13150047df44cfbbd97f9e/src/stpipe/step.py#L443
which prints out the step parameters when the step is called in this test. Since output_wcs is defined as a string in the spec:

romancal/romancal/resample/resample_step.py

Line 68 in d2ff8de

output_wcs = string(default='') # Custom output WCS.

this fails since the get_pars triggers validation against the spec.

When a step is part of a pipeline the parameters are printed at the start of Pipeline.run (which occurs before Pipeline.process where resample.run is called). This means that updating the test to use run fails (due to the parameters being printed after the output_wcs is assigned) but the mosaic pipeline usage prints the step specs prior to output_wcs being assigned and doesn't trigger the validation (running with an invalid spec).

Based on our previous conversation I was under the impression that this PR was going to take a different approach where the skycell wcs would be computed in resample (since all the relevant information is passed to resample via the association). That approach would not rely on being able to run an invalid step as long as it's made invalid during a pipeline run. Is that no longer the approach you're in favor of?

[braingram]

#1642 implements the approach discussed above where the skycell wcs is determined from the ModelLibrary association information within resample. This has a few benefits:

• allows the skycell wcs to be used in outlier detection with no code changes (addressing Use skycell WCS in outlier detection #1615)
• simplifies the mosaic pipeline
• doesn't require providing invalid input

I also did some more testing with the approach in this PR (passing a gwcs.WCS instance to output_wcs). It would be possible to update the test to use run if the step spec was changed to:

output_wcs = pass(default='')  # Custom output WCS. 

which disables validation of output_wcs.