kylebarron commented on code in PR #10894:
URL: https://github.com/apache/arrow-rs/pull/10894#discussion_r4134886969
##########
arrow-pyarrow/src/lib.rs:
##########
@@ -132,6 +132,29 @@ const ARRAY_STREAM_INPUT_TYPE: PyStaticExpr =
type_hint_union!(
type_hint_identifier!("pyarrow", "Table")
);
+/// Trait for converting Python objects to arrow-rs types without validation.
+///
+/// Prefer [`FromPyArrow`] where possible, as it validates the result before
returning.
+pub trait FromPyArrowUnchecked: Sized {
+ /// Convert a Python object to an arrow-rs type without validating the
result.
+ ///
+ /// # Safety
+ ///
+ /// The data exported by the Python object must satisfy all Arrow spec
invariants
+ /// (valid offsets, buffer sizes, UTF-8, etc.) that
[`ArrayData::validate_full`] checks.
+ unsafe fn from_pyarrow_bound_unchecked(value: &Bound<PyAny>) ->
PyResult<Self>;
+}
+
+impl<T: FromPyArrowUnchecked> FromPyArrowUnchecked for Vec<T> {
+ unsafe fn from_pyarrow_bound_unchecked(value: &Bound<PyAny>) ->
PyResult<Self> {
+ let mut v = Vec::with_capacity(value.len().unwrap_or(0));
+ for item in value.try_iter()? {
+ v.push(unsafe { T::from_pyarrow_bound_unchecked(&item?)? });
+ }
+ Ok(v)
+ }
+}
Review Comment:
I don't love this:
- This implicitly assumes that you can iterate over some Python object
(presumably a chunked array in this case) and get Arrow-native types. It thus
over-indexes on pyarrow, specifically, to the detriment of other Python Arrow
libraries.
- A pyarrow schema is also iterable, so a user could use this to import a
schema as a `Vec<Field>`, which would lose any schema metadata
- In general arrow-rs has [preferred APIs that don't materialize an entire
stream](https://github.com/apache/arrow-rs/issues/5295), so an API to convert
to `Vec<T>` would be an anti-pattern.
I would much prefer an API using lazy array readers (see
https://github.com/apache/arrow-rs/issues/6586 and
https://github.com/apache/arrow-rs/pull/11161)
Although I see there's [already an implementation of
this](https://docs.rs/arrow-pyarrow/60.0.0/arrow_pyarrow/trait.FromPyArrow.html#impl-FromPyArrow-for-Vec%3CT%3E),
so it's not a reason to block this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
