JP-3784: Fix off-nominal pixel_replace and cube_build output names

[melanieclarke]

Resolves JP-3784

Fixes output filenames for pixel_replace and cube_build when the steps are called outside the pipelines. For pixel_replace, only the case where the input is a ModelContainer is affected.

Based on #8979, with a few additional fixes and test updates.

This PR also includes a minor refactor to move the invariant_filename function from spec3 to stpipe.utilities. In the end, that function wasn't needed to fix this problem, but it still seems like a more useful place for that function to be stored.

Tasks

• request a review from someone specific, to avoid making the maintainers review every PR
• add a build milestone, i.e. Build 11.3 (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#>.datamodels.rst
• changes/<PR#>.scripts.rst
• changes/<PR#>.fits_generator.rst
• changes/<PR#>.set_telescope_pointing.rst
• changes/<PR#>.pipeline.rst

stage 1

• changes/<PR#>.group_scale.rst
• changes/<PR#>.dq_init.rst
• changes/<PR#>.emicorr.rst
• changes/<PR#>.saturation.rst
• changes/<PR#>.ipc.rst
• changes/<PR#>.firstframe.rst
• changes/<PR#>.lastframe.rst
• changes/<PR#>.reset.rst
• changes/<PR#>.superbias.rst
• changes/<PR#>.refpix.rst
• changes/<PR#>.linearity.rst
• changes/<PR#>.rscd.rst
• changes/<PR#>.persistence.rst
• changes/<PR#>.dark_current.rst
• changes/<PR#>.charge_migration.rst
• changes/<PR#>.jump.rst
• changes/<PR#>.clean_flicker_noise.rst
• changes/<PR#>.ramp_fitting.rst
• changes/<PR#>.gain_scale.rst

stage 2

• changes/<PR#>.assign_wcs.rst
• changes/<PR#>.badpix_selfcal.rst
• changes/<PR#>.msaflagopen.rst
• changes/<PR#>.nsclean.rst
• changes/<PR#>.imprint.rst
• changes/<PR#>.background.rst
• changes/<PR#>.extract_2d.rst
• changes/<PR#>.master_background.rst
• changes/<PR#>.wavecorr.rst
• changes/<PR#>.srctype.rst
• changes/<PR#>.straylight.rst
• changes/<PR#>.wfss_contam.rst
• changes/<PR#>.flatfield.rst
• changes/<PR#>.fringe.rst
• changes/<PR#>.pathloss.rst
• changes/<PR#>.barshadow.rst
• changes/<PR#>.photom.rst
• changes/<PR#>.pixel_replace.rst
• changes/<PR#>.resample_spec.rst
• changes/<PR#>.residual_fringe.rst
• changes/<PR#>.cube_build.rst
• changes/<PR#>.extract_1d.rst
• changes/<PR#>.resample.rst

stage 3

• changes/<PR#>.assign_mtwcs.rst
• changes/<PR#>.mrs_imatch.rst
• changes/<PR#>.tweakreg.rst
• changes/<PR#>.skymatch.rst
• changes/<PR#>.exp_to_source.rst
• changes/<PR#>.outlier_detection.rst
• changes/<PR#>.tso_photometry.rst
• changes/<PR#>.stack_refs.rst
• changes/<PR#>.align_refs.rst
• changes/<PR#>.klip.rst
• changes/<PR#>.spectral_leak.rst
• changes/<PR#>.source_catalog.rst
• changes/<PR#>.combine_1d.rst
• changes/<PR#>.ami.rst

other

• changes/<PR#>.wfs_combine.rst
• changes/<PR#>.white_light.rst
• changes/<PR#>.cube_skymatch.rst
• changes/<PR#>.engdb_tools.rst
• changes/<PR#>.guider_cds.rst

[melanieclarke]

Regression tests running here:
https://github.com/spacetelescope/RegressionTests/actions/runs/12401830665

Test shows no failures.

[emolter]

LGTM, I like the improvements to test_invariant_filename. Admittedly I only skimmed this because I assume it is similar to Jane's PR that I already looked at

[melanieclarke]

@emolter - this is the updated fix for the pixel_replace output names. I will leave it at draft so it doesn't get merged for 11.2, but it is ready for review.

It turns out the only fix necessary was to default the pixel replace step to output_use_model=True. This is done for several other similar steps where multiple input are used, but output files should be based on the input name (tweakreg, master_background, cube_build), and should probably be added anywhere we might reasonably expect the input and output to be a list of files.

I kept the change to put invariant_filename into stpipe.utilities, because that seems like the right place for it anyway, but it is no longer needed to fix the filenames.

[emolter]

It turns out the only fix necessary was to default the pixel replace step to output_use_model=True.

So to be clear, this is an stpipe option available to all steps, and it's added to the spec just because we want to change the default value?

It seems to me we might reasonably expect a list of files in any step that has a container as output. Are you thinking of any other steps in particular?

[melanieclarke]

So to be clear, this is an stpipe option available to all steps, and it's added to the spec just because we want to change the default value?

Yes, the default value is False. We might want to consider whether the default value should be True, because all stpipe steps will generate similar nonsense names if the model to be saved is a Sequence.

It seems to me we might reasonably expect a list of files in any step that has a container as output. Are you thinking of any other steps in particular?

I don't know for sure if there are any other steps that need this. The only specific step I was wondering about was outlier_detection, which does not modify this parameter. Looking at it now, though, it looks like it always returns a ModelLibrary, so it probably isn't affected - ModelLibraries are saved by iterating over the models.

[codecov]

Codecov Report

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

Project coverage is 76.90%. Comparing base (095d364) to head (d65a12d).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
jwst/stpipe/utilities.py	85.71%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9019      +/-   ##
==========================================
+ Coverage   76.89%   76.90%   +0.01%     
==========================================
  Files         497      498       +1     
  Lines       45656    45761     +105     
==========================================
+ Hits        35108    35194      +86     
- Misses      10548    10567      +19     

Flag	Coverage Δ		*Carryforward flag
nightly	77.39% <ø> (-0.01%)	⬇️	Carriedforward from 095d364

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

[melanieclarke]

Now that 1.17.0 is released, I will merge this when CI completes.

[jemorrison]

@melanieclarke Thank you for getting this merged.