Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step and Pipeline do not respect get_crds_parameters override for JWST ModelContainer #196

Open
emolter opened this issue Oct 10, 2024 · 0 comments

Comments

@emolter
Copy link
Collaborator

emolter commented Oct 10, 2024

The ModelContainer class in the jwst repository attempts to provide its own get_crds_parameters() method (code here), but this is not respected in stpipe. Instead, get_crds_parameters() is read from the first datamodel in the sequence.

This override failure doesn't seem to be breaking anything currently in jwst, and it may not be necessary at all. However, the method itself cannot easily be removed because ModelContainer relies on isinstance(container, AbstractDataModel) returning True to make its way through Step.run, and get_crds_parameters() and crds_observatory are required methods to pass that check.

I see two ways to fix this that are not mutually exclusive, and both could (and perhaps should) be implemented:

  1. Change the logic for finding get_crds_parameters() such that if a Sequence-type input has its own get_crds_parameters() method, that method is used.
  2. Make it so that ModelContainer does not need to satisfy the requirements of an AbstractDataModel in order to pass through Step.run. This change will make sense particularly once this pull request is merged, as it removes the inheritance of ModelContainer from JWSTDataModel. This withdrawn PR provided a (partial?) implementation of this change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant