viirya commented on code in PR #50327:
URL: https://github.com/apache/arrow/pull/50327#discussion_r3552797257
##########
python/pyarrow/array.pxi:
##########
@@ -2458,6 +2476,34 @@ cdef class NumericArray(Array):
A base class for Arrow numeric arrays.
"""
+ cdef object _getitem_py(self, int64_t i):
+ cdef Type tid = self.ap.type_id()
+ if self.ap.IsNull(i):
+ return None
+ if tid == _Type_INT64:
+ return (<CInt64Array*> self.ap).Value(i)
+ elif tid == _Type_INT32:
+ return (<CInt32Array*> self.ap).Value(i)
Review Comment:
Reordered to the regular `int8..uint64, float, double` sequence in
3d303ce487.
##########
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:
Thanks for the correction — I stand corrected on 32-bit support; agreed it's
moot either way since such a value can't exist in a 32-bit address space.
##########
python/pyarrow/array.pxi:
##########
@@ -4229,6 +4357,30 @@ cdef class StructArray(Array):
Concrete class for Arrow arrays of a struct data type.
"""
+ cdef object _getitem_py(self, int64_t i):
+ if self.ap.IsNull(i):
+ return None
+ cdef int64_t k, num_fields = self.type.num_fields
+ if self._children_cache is None:
+ names = [self.type.field(k).name for k in range(num_fields)]
+ if len(set(names)) != len(names):
+ # StructScalar.as_py raises ValueError on duplicate field
+ # names; mark the cache so we take the Scalar path below.
+ self._children_cache = (None, None)
Review Comment:
Done — it now raises the same `ValueError` as `StructScalar.as_py` directly
(the cache/fallback dance is gone), and the test asserts the message.
##########
python/pyarrow/array.pxi:
##########
@@ -1864,7 +1864,19 @@ cdef class Array(_PandasConvertible):
lst : list
"""
self._assert_cpu()
- return [x.as_py(maps_as_pydicts=maps_as_pydicts) for x in self]
+ cdef int64_t i, n = self.length()
+ if maps_as_pydicts is not None:
+ # Converting maps to dicts has per-entry semantics (duplicate-key
+ # detection); use the Scalar-based conversion for exact behavior.
+ return [x.as_py(maps_as_pydicts=maps_as_pydicts) for x in self]
+ return [self._getitem_py(i) for i in range(n)]
+
+ cdef object _getitem_py(self, int64_t i):
Review Comment:
Filed GH-50448 for the per-range conversion (folding in the
dispatch-hoisting and null-check ideas from this thread as well), and added a
TODO pointing at it.
##########
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)
Review Comment:
Nice — done. Switched string/binary (+ large variants) to `GetView(i)`; it
also made `StringViewArray`/`BinaryViewArray` specializations trivial, so those
are included now too.
##########
python/pyarrow/tests/test_array.py:
##########
@@ -465,6 +465,56 @@ def test_array_getitem_numpy_scalars():
assert arr[np.int32(idx)].as_py() == lst[idx]
+def test_to_pylist_bulk_paths():
+ # GH-50326: to_pylist converts through scalar-free _getitem_py
+ # specializations; 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([b"a\x00b", None, b"", b"\xff"], type=pa.binary()),
+ pa.array([b"a\x00b", None, b""], type=pa.large_binary()),
+ pa.array([[b"x", None, b"\x00y"], None, []],
+ type=pa.list_(pa.binary())),
+ pa.array([1, None, -(2**62), 2**62], type=pa.int64()),
+ pa.array([0, None, 2**63 + 7], type=pa.uint64()),
+ pa.array([-128, 127, None], type=pa.int8()),
+ pa.array([1.5, None, -0.5], type=pa.float64()),
+ pa.array([1.5, None], type=pa.float32()),
+ pa.array([True, None, False], type=pa.bool_()),
+ pa.array([{"a": 1, "b": "x"}, None, {"a": None, "b": None}],
+ type=pa.struct([("a", pa.int32()), ("b", pa.string())])),
+ pa.array([], type=pa.list_(pa.int32())),
+ pa.array([None, None], type=pa.list_(pa.string())),
+ ]
Review Comment:
Added `binary_view` and `string_view` cases — and since `GetView` covers the
view types, they now take real fast paths instead of the Scalar fallback.
##########
python/pyarrow/tests/test_array.py:
##########
@@ -465,6 +465,56 @@ def test_array_getitem_numpy_scalars():
assert arr[np.int32(idx)].as_py() == lst[idx]
+def test_to_pylist_bulk_paths():
+ # GH-50326: to_pylist converts through scalar-free _getitem_py
+ # specializations; 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([b"a\x00b", None, b"", b"\xff"], type=pa.binary()),
+ pa.array([b"a\x00b", None, b""], type=pa.large_binary()),
+ pa.array([[b"x", None, b"\x00y"], None, []],
+ type=pa.list_(pa.binary())),
+ pa.array([1, None, -(2**62), 2**62], type=pa.int64()),
+ pa.array([0, None, 2**63 + 7], type=pa.uint64()),
+ pa.array([-128, 127, None], type=pa.int8()),
+ pa.array([1.5, None, -0.5], type=pa.float64()),
+ pa.array([1.5, None], type=pa.float32()),
+ pa.array([True, None, False], type=pa.bool_()),
+ pa.array([{"a": 1, "b": "x"}, None, {"a": None, "b": None}],
+ type=pa.struct([("a", pa.int32()), ("b", pa.string())])),
+ pa.array([], type=pa.list_(pa.int32())),
+ pa.array([None, None], type=pa.list_(pa.string())),
+ ]
+ for arr in arrays:
+ for view in (arr, arr.slice(1), arr.slice(0, 2), arr.slice(2)):
+ assert view.to_pylist() == [x.as_py() for x in view]
+
+ # Values inside numeric lists must stay Python ints/None, never floats
+ result = pa.array([[1, None, 3]], type=pa.list_(pa.int32())).to_pylist()
+ assert result == [[1, None, 3]]
+ assert [type(x) for x in result[0]] == [int, type(None), int]
+
+ # Duplicate struct field names raise like StructScalar.as_py does
+ dup = pa.StructArray.from_arrays(
+ [pa.array([1, 2]), pa.array(["a", "b"])], names=["x", "x"])
+ with pytest.raises(ValueError):
Review Comment:
Done.
--
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]
