Code assumes non-required `cal_step` exists

[braingram]

Prior to spacetelescope/romanisim#337 romanisim generated valid L2 files that did not contain a meta.cal_step. These files cause issues for romancal which assumes all L2 files have meta.cal_step. One example is in the flux step:

romancal/romancal/flux/flux_step.py

Line 99 in a859946

if model.meta.cal_step["flux"] == "COMPLETE":

fails on L2 files that do not have a meta.cal_step.

This could be addressed by updating the pipeline to avoid accessing cal_step directly and instead using some to-be-determined API for accessing the cal_step attribute. There is unused and unfinished code in Step for recording the status:

romancal/romancal/stpipe/core.py

Line 127 in a859946

def record_step_status(self, model, step_name, success=True):

jwst took a different approach implementing 2 utility functions:

• record_step_status
• query_step_status

which could be borrowed here.

[braingram]

@stscijgbot-rstdms

[stscijgbot-rstdms]

This issue is tracked on JIRA as RCAL-1347.

[schlafly]

I think this would be a good enhancement and clean up a lot of test code. The other code updates romanisim to create this metadata, though, so I see this as a good enhancement rather than a bug. For Webb was the motivation similar, not wanting to require that these step statuses exist?

[braingram]

Pinging @emolter for some input here on the motivation (and success, advice) on record/query_step_status in jwst.

The jwst datamodels are a bit different with respect to what's "required" (and that's an ongoing discussion in some sense).

In brief, every model uses the same "core" schema which has meta.cal_step defined (but not required).

At the moment stdatamodels will automatically create meta.cal_step when accessed (since it's in the schema) so any_model.meta.cal_step.some_step = "COMPLETE" should never raise an exception since meta.cal_step will be created if it doesn't exist.

At the risk of adding some "scope creep" if we define a common API that the pipelines implement for setting/getting step status we could likely upstream some of this to stpipe and remove some of the logic that steps have to manually implement to set step status.

[emolter]

The point of these two functions in jwst is to handle getting/setting the status keyword for either a single datamodel or a collection (ModelContainer or ModelLibrary) using a single function that accepts any of these as input. I'd say this has been useful for the stage 3 pipelines and elsewhere in reducing repeated code.

record_step_status was moved out of the Step class for JWST by spacetelescope/jwst#8600.
I think the original motivation was that directly setting self.skip led to buggy behavior; see the discussion on spacetelescope/jwst#8498. Removing that line wouldn't have forced us to move the method out of the class; however I find it very useful that this is NOT a method of the step class. There are lots of instances in the pipeline where we want to e.g. set the status within a helper function that's not part of the Step class, or set the status of one step from a different step entirely. I can provide examples if it would help.

These functions have been somewhat inconsistently applied, and I'm sure you could still find pipeline instances where step status getting/setting on lists of models happens without using them.