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

call and run changes #165

Open
braingram opened this issue Jul 11, 2024 · 2 comments
Open

call and run changes #165

braingram opened this issue Jul 11, 2024 · 2 comments

Comments

@braingram
Copy link
Collaborator

xlinking spacetelescope/jwst#8130

Some discussion is underway to clean up the Step methods including call run and __call__ to clarify which methods create new instances, fetch reference files, overwrite parameters etc.

One currently discussed plan would be to:

  • deprecate (with a UserWarning or some other warning visible in notebook runs) __call__ for eventual removal, requiring the explicit use of run or call (or the renamed versions)
  • deprecate (again with a notebook visible warning) both run and call in favor of a less ambiguous name

I'd make an argument that part of the confusion is that call is a classmethod yet a user can call it on an instance. Replacing the classmethod with a separate function would help clean this up maybe create_and_run(step_class, ...) where the method can check that step_class is not an instance.

run could be renamed to maybe run_without_crds_parameters to make it explicit that the crds parameter files will be ignored.

Pinging @jdavies-st for comments.

@jdavies-st
Copy link
Contributor

I think deprecating __call__ is probably not a good idea, as that's generally what one does with class instances - calls them. I think what __call__ points to, i.e. self.run is problematic, but only because the class constructor doesn't take (generally) input data as a required input.

Agree completely that Step.call is a class factory and should be a function, not a class method. It is confusing to users to allow it on an instance. I would not be for adding more non-pythonic methods of creating steps and running them. Keep it as one would expect it to be in Python.

Those are just some inital thoughts. Not new by any means, but things we knew were bad 6 years ago.

@braingram
Copy link
Collaborator Author

xref: #9

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

2 participants