viirya commented on code in PR #50327:
URL: https://github.com/apache/arrow/pull/50327#discussion_r3546430562
##########
python/pyarrow/lib.pxd:
##########
@@ -282,6 +282,8 @@ cdef class Array(_PandasConvertible):
cdef:
shared_ptr[CArray] sp_array
CArray* ap
+ # Lazily wrapped child array(s) reused by _getitem_py (see GH-50326)
+ object _children_cache
Review Comment:
Good point — moved the declaration after the pre-existing attributes in
728584f8e6, so `type`/`_name` offsets stay stable and the new field follows the
append-only convention that Cython's size check assumes.
##########
python/pyarrow/array.pxi:
##########
@@ -4006,6 +4105,15 @@ cdef class LargeStringArray(Array):
Concrete class for Arrow arrays of large string (or utf8) data type.
"""
+ cdef object _getitem_py(self, int64_t i):
+ cdef:
+ int64_t length
+ const uint8_t* data
+ if self.ap.IsNull(i):
+ return None
+ data = (<CLargeStringArray*> self.ap).GetValue(i, &length)
+ return cp.PyUnicode_DecodeUTF8(<const char*> data, length, NULL)
Review Comment:
PyArrow only supports 64-bit platforms, where `Py_ssize_t` and `int64_t`
have the same width; and on a hypothetical 32-bit build a single value longer
than `PY_SSIZE_T_MAX` could not exist in the first place, since the child data
buffer itself is bounded by the process address space. So the implicit
conversion cannot truncate in practice.
##########
python/pyarrow/tests/test_array.py:
##########
@@ -465,6 +465,38 @@ def test_array_getitem_numpy_scalars():
assert arr[np.int32(idx)].as_py() == lst[idx]
+def test_to_pylist_bulk_paths():
+ # GH-50326: list-like and string arrays convert to Python objects in
+ # bulk instead of going through one Scalar per element; the result must
+ # match the per-scalar conversion exactly.
+ arrays = [
+ pa.array([[1, None, 3], None, [], [4]], type=pa.list_(pa.int32())),
+ pa.array([["a", None], None, [], ["bcd", ""]],
+ type=pa.list_(pa.string())),
+ pa.array([["a", None], None, [], ["bcd", ""]],
+ type=pa.large_list(pa.large_string())),
+ pa.array([[1, None], None, [3, 4]], type=pa.list_(pa.int32(), 2)),
+ pa.array([[[1], [2, None]], None, [None, [3]]],
+ type=pa.list_(pa.list_(pa.int32()))),
+ pa.array([[("k1", 1), ("k2", None)], None, []],
+ type=pa.map_(pa.string(), pa.int32())),
+ pa.array(["a", None, "", "\N{GRINNING FACE} \N{SNOWMAN}"],
+ type=pa.string()),
+ pa.array(["a", None, "", "\N{GRINNING FACE} \N{SNOWMAN}"],
+ type=pa.large_string()),
+ pa.array([], type=pa.list_(pa.int32())),
+ pa.array([None, None], type=pa.list_(pa.string())),
+ ]
Review Comment:
Added binary/large_binary (including embedded NUL bytes), list<binary>,
wide-range integers, floats, boolean and struct coverage in 728584f8e6, plus an
assertion that duplicate struct field names still raise `ValueError`.
--
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]
