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

Avoid returning PyObject and add wrapper types for arro3 export #269

Merged
merged 6 commits into from
Dec 6, 2024

Conversation

kylebarron
Copy link
Owner

@kylebarron kylebarron commented Dec 6, 2024

  • Avoid returning PyObject, and prefer Bound<'py, PyAny> instead.

    For a long time I didn't really understand the difference between PyObject and Bound<'py, PyAny>. But recently I've learned that the former is like a "bare" PyAny while the latter is PyAny with an associated py: Python token. That means you can do Python operations without needing to separately acquire the GIL.

    It's preferable to return Bound, because that's what calls into Python return. It works nicer with the IntoPyObject trait. And it's easy to call .unbind() to get a Py<PyAny> (a PyObject).

  • This also adds wrapper types to simplify export to arro3.

    Now that we're using pyo3 0.23, there's a new IntoPyObject trait. This is a lot nicer than the previous IntoPy trait, because it allows for fallible conversions. So we can call to_arro3() from within the IntoPyObject trait impl. And e.g. if arro3.core isn't available, it'll correctly error.

    These wrapper types also nudge people toward the right choice: to dynamically pass data to the user's runtime instance of arro3.core.

@kylebarron kylebarron changed the title wip: avoid pyobject Avoid returning PyObject and add wrapper types for arro3 export Dec 6, 2024
@kylebarron kylebarron merged commit 3709db9 into main Dec 6, 2024
5 checks passed
@kylebarron kylebarron deleted the kyle/avoid-pyobject branch December 6, 2024 22:18
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 this pull request may close these issues.

1 participant