Incompatibility between roman_datamodels and stpipe attribute assignment

[braingram]

If a step is skipped (by the skip attribute being set to True before the step is called) stpipe will assign 'SKIPPED' to the model metadata:
https://github.com/spacetelescope/stpipe/blob/d20b6429cafe0a4ca846fd43af7b6a61241e632c/src/stpipe/step.py#L484-L486
Including a call like the follow (for tweakreg for example):

model['meta.cal_step.tweakreg'] = 'SKIPPED'

However this assignment does not set model.meta.cal_step.tweakreg (the expected metadata field) for roman datamodels.

import roman_datamodels.maker_utils as maker_utils, roman_datamodels.datamodels as dm
im = dm.ImageModel(maker_utils.mk_level2_image())
assert im.meta.cal_step.tweakreg == 'INCOMPLETE'   # the expected value is 'INCOMPLETE' on creation

im['meta.cal_step.tweakreg'] = 'SKIPPED'  # assign as would be done in stpipe for a skipped step

assert im.meta.cal_step.tweakreg == 'INCOMPLETE'  # the expected value was not updated
assert im._instance['meta.cal_step.tweakreg'] == 'SKIPPED'  # the above assignment created a key with the "dotted" string

[braingram]

@schlafly Would you consider this a roman_datamodels bug?

Is it planned to allow assignments using the "dotted" string, or perhaps should this cause an error (instead of silently creating a new key value pair in the _instance) and stpipe updated to not used an assignment with a "dotted" string?

[schlafly]

Sorry, out of the office today, but yes, your example looks clear that stpipe expects us to support this notation, so I agree with your sense that we need to handle this case better.

…

On Thu, Jun 27, 2024, 10:43 Brett Graham ***@***.***> wrote: @schlafly <https://github.com/schlafly> Would you consider this a roman_datamodels bug? Is it planned to allow assignments using the "dotted" string, or perhaps should this cause an error (instead of silently creating a new key value pair in the _instance) and stpipe updated to not used an assignment with a "dotted" string? — Reply to this email directly, view it on GitHub <#343 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAK2E4KUOYWIA6JJRWSPBU3ZJQXINAVCNFSM6AAAAABGPP2QYSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJVGA2DSNZSGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[braingram]

There are additional factors that make the automatic skip setting in stpipe not work. Even if the attribute access is fixed most romancal steps do not define a class_alias. For example, refpix doesn't define one. When elp is run with refpix.skip=True the cal_step.refpix attribute of the model is not set because stpipe doesn't know to use refpix (it expects the class_alias to match the cal_step.<identifier>: https://github.com/spacetelescope/stpipe/blob/ecd5d162be425c24db2498b34bcbaeccec4ac557/src/stpipe/step.py#L511

@schlafly was it a design decision to not use class_alias for romancal steps? There are a few that have them (resample, tweakreg, etc) but many do not. I must admit that I don't have a full understanding of what class_alias does but I think it also allows strun resample to call romancal.steps.ResampleStep (which could be problematic if a user has both jwst and romancal installed).

I think the options for resolving this issue are either:

• define class_alias for all romancal steps and have them match the cal_step.<identifier>
• modify stpipe to not require a class_alias (and have some other API) for setting cal_step status

[schlafly]

@ddavis-stsci will know the design decisions better than I. I doubt that this was a conscious decision, and so we could go ahead and add them. But I don't have a solution for Webb & Roman sharing class_aliases.

[ddavis-stsci]

I think that we should define class_alias for all the steps and take advantage of the stpipe infrastructure.