standardize setattr use in skip

[braingram]

Closes spacetelescope/roman_datamodels#343

jwst regtests all pass: https://github.com/spacetelescope/RegressionTests/actions/runs/11804014366
romancal regtests all pass: https://github.com/spacetelescope/RegressionTests/actions/runs/11803229018
(it may be worthwhile to add a regression or unit test in romancal for the feature fixed in this PR in a follow-up romancal PR)

This PR standardizes how stpipe sets meta.cal_step.<class_alias> when a step is skipped. On main the behavior for a single roman datamodel (not a ModelLibrary) does not log the skip. Running the following on main:

from romancal.pipeline import ExposurePipeline
elp = ExposurePipeline()
elp.saturation.skip = True
elp.refpix.skip = True
elp.linearity.skip = True
elp.dark_current.skip = True
elp.rampfit.skip = True
elp.assign_wcs.skip = True
elp.flatfield.skip = True
elp.photom.skip = True
elp.source_catalog.skip = True
elp.tweakreg.skip = True
result = elp.run("some_uncal.asdf")
print(result[0].meta.cal_step.saturation)

results in printing "INCOMPLETE" indicating that even though the saturation step was skipped stpipe failed to log this to the resulting datamodel.

With this PR the above example results in printing "SKIPPED".

Tasks

• update or add relevant tests
• update relevant docstrings and / or docs/ page
• Does this PR change any API used downstream? (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)
  • run regression tests with this branch installed ("git+https://github.com/<fork>/stpipe@<branch>")
    • jwst regression test
    • romancal regression test

news fragment change types...

• changes/<PR#>.feature.rst: new feature
• changes/<PR#>.bugfix.rst: fixes an issue
• changes/<PR#>.doc.rst: documentation change
• changes/<PR#>.removal.rst: deprecation or removal of public API
• changes/<PR#>.misc.rst: infrastructure or miscellaneous change

[codecov]

Codecov Report

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

Project coverage is 95.22%. Comparing base (45d4cde) to head (a2a8d46).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
src/stpipe/step.py	81.81%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #195       +/-   ##
===========================================
+ Coverage   80.80%   95.22%   +14.42%     
===========================================
  Files          37       37               
  Lines        2849     3165      +316     
===========================================
+ Hits         2302     3014      +712     
+ Misses        547      151      -396     

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

[braingram]

@schlafly I can't request you as a reviewer but when available would you give this a look (or suggest an alternate roman reviewer)? This PR should fix steps not being marked as "SKIPPED" when skipped view setting the skip attribute or by passing <step>.skip==True to strun.

[schlafly]

Thanks Brett, looks good. Remind me---I thought we did have some regtests that skipped tests (e.g., for the all saturated test), and I thought you had stumbled on this issue in the process of testing some unrelated code in romancal? So I had expected some steps to be newly marked SKIPPED. But perhaps these have all been coded around.

[braingram]

Thanks Brett, looks good. Remind me---I thought we did have some regtests that skipped tests (e.g., for the all saturated test), and I thought you had stumbled on this issue in the process of testing some unrelated code in romancal? So I had expected some steps to be newly marked SKIPPED. But perhaps these have all been coded around.

So far I've only been able to find one test in romancal that passes skip to (an equivalent to) strun:
https://github.com/spacetelescope/romancal/blob/9031ae12b14b22ba565581670e74b826d40ed86a/romancal/regtest/test_wfi_image_pipeline.py#L283
This test sets tweakreg.skip=True however the test checks that cal_step.tweakreg==INCOMPLETE. Interestingly this PR doesn't switch the result to cal_step.tweakreg==SKIPPED because there's.... yet another bug? The issue is that tweakreg is passed a list of models. stpipe on main (and with this PR, to match main) won't peek into that list to see if it's a list of models (and set the cal_step to skipped). I'll give this some more thought but I think this is a separate issue (since tweakreg is passed a list of models, not a model and not a library, both of which are handled in this PR). Part of me thinks stpipe should handle a list of models in a way where the cal_step status is set to skipped if the step is skipped. However this doesn't appear to be the case on main even for jwst as seen in the following

import jwst.datamodels
from jwst.step import RampFitStep

ims = [jwst.datamodels.ImageModel()] * 2
step = RampFitStep()
step.skip = True
result = step.run(ims[0])
print(result.meta.cal_step.ramp_fit)

ims = [jwst.datamodels.ImageModel()] * 2
result = step.run(ims)
print(result[0].meta.cal_step.ramp_fit)

This prints out (in addition to all the pipeline logging stuff):

SKIPPED
None

When passed a model stpipe marks it as skipped when the step is skipped. When passed a list of models, stpipe doesn't mark any models as skipped. This is the same when run with main or with this PR.

For the fully saturated tests (which trigger this part of the exposure pipeline) the skipping of steps is done within the pipeline (not via stpipe) which also sets the step status. Those uses aren't impacted by this PR.

[schlafly]

Ugh. Thanks. I agree with you that this PR looks good and should proceed.

[nden]

lgtm