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

Generated classes are part of the builtin module #474

Closed
rth opened this issue May 6, 2019 · 3 comments · Fixed by #477
Closed

Generated classes are part of the builtin module #474

rth opened this issue May 6, 2019 · 3 comments · Fixed by #477

Comments

@rth
Copy link

rth commented May 6, 2019

Related to the pickle support #100 , as mentioned by @althonos ,

PyO3 exposes all classes as part of the builtins module, because the import mechanism has not been properly implemented, so pickle tries to use builtins.MyClass and fails with the error you reported.

This is also discussed in the following test,

// Rationale: The class can be added to many modules, but will only be initialized once.
// We currently have no way of determining a canonical module, so builtins is better
// than using whatever calls init first.
assert_eq!(module, "builtins");

Once __module__ it is set to builtin, it cannot be changed in an obvious way (at least I have not found one). Using the example class,

       cls = MyClass()
       cls.__module__ = "correct_name"
E   TypeError: can't set attributes of built-in/extension type 'MyClass'

it would be nice if there way some way to manually set the module name.

Also setuptools-rust does have the module name provided by the user for each RustExtension , but doesn't seem to be assigning it?

@rth rth mentioned this issue May 6, 2019
@Alexander-N
Copy link
Member

This was changed in c71c116#diff-d3dec314b1956210565bd5849a91e770. I don't see a problem in setting the module to the first one which initializes the class. This seems analogous to when I import a class in Python, then its __module__ attribute does not change to the module it was imported in.

@konstin How about we revert to the old behavior?

@rth It might be possible to subclass in Python and set __module__ there.

@rth
Copy link
Author

rth commented May 6, 2019

It might be possible to subclass in Python and set module there

Thanks @Alexander-N ! You are right that works, but I'm hoping for some alternative solution that wouldn't require subclassing every single pyo3 generated class.

@konstin
Copy link
Member

konstin commented May 6, 2019

I'm fine with reverting to the old behaviour if that unbreaks pickle, however we should document in the guide that module name depends on where the class was first used.

Ideally we should also add an option to set the module manually, but that's independent of the default.

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

Successfully merging a pull request may close this issue.

3 participants