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

Allow #[new] to return existing objects #2384

Closed
davidhewitt opened this issue May 17, 2022 · 15 comments · Fixed by #3287
Closed

Allow #[new] to return existing objects #2384

davidhewitt opened this issue May 17, 2022 · 15 comments · Fixed by #3287
Milestone

Comments

@davidhewitt
Copy link
Member

I don't think it's possible for #[new] in PyO3 to return an existing object (at the moment), though it really should be.

Originally posted by @davidhewitt in #2382 (comment)

@kszlim
Copy link

kszlim commented Nov 22, 2022

This is an important feature for me, either supporting this or having full blown support for PEP 435 enums would be great. I specifically need full emulation/support for an IntFlag-like type.

@davidhewitt
Copy link
Member Author

@kszlim I agree with you. While I think this will not make it into the 0.18 release, I would very much like to address this soon.

@kszlim
Copy link

kszlim commented Nov 28, 2022

Do you know if fully supporting an IntFlag like type is possible from pyo3? It seems like it'd require allowing returning references from rust into python or multiple ownership of the same pointer, ideally IntFlags would be lazily created and be singletons and have the is operator working correctly with them

@adamreichold
Copy link
Member

I think the problem really is limited to #[new], other functions and methods can return existing objects, so something like

#[pyclass]
pub struct Color { .. }

#[pyfunction]
pub fn red(py: Python) -> Py<Color> {
   static RED: GILOnceCell<Py<Color>> = GILOnceCell:new();

   RED.get_or_init(py, || Color { .. }).clone()
}

should work. (I think that a #[staticmethod] should work as well, giving you at least Color.red() or Color.RED() if you silence the naming convention warnings. @davidhewitt Can one combine #[getter] and #[staticmethod]?)

(The problem with #[new] is that it is assumed to return something that can be converted into a PyClassInitializer instead of the "finished" Python object because what is returned usually needs to be wrapped in an additional PyCell layer.)

@kszlim
Copy link

kszlim commented Nov 28, 2022

@adamreichold does that Py<Color> always evaluate to True in the following in python code?

Color.red() is Color.red()

I'm under the impression that the clone will make that not true.

@davidhewitt
Copy link
Member Author

Cloning a Py<T> will give you the same Python object (after increasing its reference count).

Can one combine #[getter] and #[staticmethod]?

Sounds like #[classattr] ;)

@adamreichold
Copy link
Member

adamreichold commented Nov 28, 2022

Sounds like #[classattr] ;)

Indeed. Thank you!

@kszlim So you better end up using #[classattr] to get the more customary Color.RED syntax on the Python side, but yes cloning Py<T> will not produce a new Python object, just a new reference to the same object.

@kszlim
Copy link

kszlim commented Dec 11, 2022

Sounds like #[classattr] ;)

Indeed. Thank you!

@kszlim So you better end up using #[classattr] to get the more customary Color.RED syntax on the Python side, but yes cloning Py<T> will not produce a new Python object, just a new reference to the same object.

late reply, but it's my understanding that the id produced won't be identical, ie, so even if the underlying object is the same, since it's another pointer to that same object, the is operator won't hold true.

@birkenfeld
Copy link
Member

No - "another pointer to the same object" means the pointer must point to the same object, i.e. the address/id is the same.

@kszlim
Copy link

kszlim commented Dec 11, 2022

No - "another pointer to the same object" means the pointer must point to the same object, i.e. the address/id is the same.

ah, thanks you're right, i just tested it. Guess I just need this issue to close for my code to work, thanks!

@alex
Copy link
Contributor

alex commented Jun 21, 2023

Is a potential solution here:

  • Adding a From impl that converts from Py<PyCell<T>> into PyClassInitializer.
  • Having create_cell_from_subtype simply return the existing PyCell in that case.

@alex
Copy link
Contributor

alex commented Jul 1, 2023

If ^^ sounds plausible, I'm happy to take a pass at working up a PR for it.

@davidhewitt
Copy link
Member Author

Sorry for the slow reply. The above sounds sort of on the lines, though I was hoping it might be simpler to just allow any T: IntoPy<PyObject> be a valid return type from #[new]. I'm trying to dabble around with that idea occasionally though haven't quite proven it works.

@alex
Copy link
Contributor

alex commented Jul 2, 2023 via email

@davidhewitt
Copy link
Member Author

I don't think it'd be necessary to solve the return type syntactically, my current thinking is that it should be possible to use autoref specialization somewhere in the macro implementation to handle the different code paths. Just needs some playing around to see if it can work...

alex added a commit to alex/pyo3 that referenced this issue Jul 2, 2023
alex added a commit to alex/pyo3 that referenced this issue Jul 2, 2023
alex added a commit to alex/pyo3 that referenced this issue Jul 2, 2023
alex added a commit to alex/pyo3 that referenced this issue Jul 2, 2023
alex added a commit to alex/pyo3 that referenced this issue Jul 2, 2023
alex added a commit to alex/pyo3 that referenced this issue Jul 2, 2023
davidhewitt pushed a commit that referenced this issue Jul 3, 2023
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.

5 participants