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

ToPyObject and IntoPy<PyObject> boilerplates #1595

Closed
pickfire opened this issue May 3, 2021 · 2 comments
Closed

ToPyObject and IntoPy<PyObject> boilerplates #1595

pickfire opened this issue May 3, 2021 · 2 comments

Comments

@pickfire
Copy link
Contributor

pickfire commented May 3, 2021

While working on chrono pyo3 integration, I noticed I am typing out IntoPy referencing ToPyObject quite often.

    impl ToPyObject for NaiveDate {
        fn to_object(&self, py: pyo3::Python) -> pyo3::PyObject {
            let mdf = self.mdf();
            let date = PyDate::new(py, self.year(), mdf.month() as u8, mdf.day() as u8)
                .expect("Failed to construct date");
            date.into()
        }
    }

    impl IntoPy<pyo3::PyObject> for NaiveDate {
        fn into_py(self, py: pyo3::Python) -> pyo3::PyObject {
            ToPyObject::to_object(&self, py)
        }
    }

If we have implemented IntoPy<pyo3::PyObject> is there a reason why one still wants to implement ToPyObject when it could exactly be the same thing? So most of the time I just reference the same thing. I know in some cases where there could be like specialization mechanism when self in involved but at least from what I did, all of the are the same (since I couldn't think it could be better done when it is using self).

Is there any reason why we don't do something like trait default function (like Iterator) so without typing the ToPyObject it will automatically gets created unless user wants a specialized version? Or is there some cases where we can only have like IntoPy<PyObject> but not ToPyObject so we can't automatically create the function for it?

@davidhewitt
Copy link
Member

Yes, I agree that there is some overlap here. I've wondered about this myself. It could be possible to provide a blanket implementation, e.g. I've wondered about one of the following in the past:

  • IntoPy<PyObject> for &T where T: ToPyObject
  • ToPyObject for T where T: IntoPy<PyObject> + Clone

However I keep backing down from trying to add either of these blanket impls, because actually these traits do have different semantics. IntoPy<PyObject> is semantically about transferring ownership of data to Python, whereas ToPyObject is about copying data.

I'd welcome new ideas for how to present these APIs. Maybe there's a single trait which can describe both semantics. I would also quite like to remove the generic parameter from IntoPy. In the past I convinced myself that it's probably not possible until min_specialization is stable. (Related to #1200 as far as I recall.)

The "default function" you suggest is actually what rust-cpython did for the original ToPyObject trait. (https://docs.rs/cpython/0.6.0/cpython/trait.ToPyObject.html) Somewhere along the history of PyO3's fork, this got split out into the different traits. I'd need to refresh my memory as to all the design decisions (it might have been related to specialization).

@davidhewitt
Copy link
Member

Closing now we have #4495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants