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

Create a soft_def shim to combine move_to and Getattr #473

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

anivegesana
Copy link
Contributor

New (hopefully more intuitive) construct to combine move_to and Getattr into one function call.

Resolves #447

@mmckerns mmckerns added this to the dill-0.3.6 milestone May 29, 2022
@mmckerns mmckerns self-requested a review June 5, 2022 09:46
Copy link
Member

@mmckerns mmckerns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, in general. I don't like the name soft_def. You aren't actually "defining" the shim, you kind of do that above when you write the function code... it's more like we register the shim in _dill. @anivegesana: What do you think of the name register, or something similar? Otherwise LGTM, and will pull when we agree on a name.

@anivegesana
Copy link
Contributor Author

register is already taken by the register function for the pickler. What about if_not_defined_in or ifndef_in?

@mmckerns
Copy link
Member

mmckerns commented Jun 7, 2022

How about: register_shim? Isn't that what you are doing? You are registering the shim in _dill (and setting a default). Also... one other thought... is it worthwhile to enable passing the module as a keyword arg (with _dill as the default)?

@mmckerns mmckerns merged commit a6bf9a2 into uqfoundation:master Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

single shims function calls move_to and Getattr
2 participants