From 5bc02668a8931c221a6e3699d6f0fee0039d2724 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Wed, 8 Jul 2026 15:20:43 -0700 Subject: [PATCH 1/8] GH-50429: [Python] Convert maps to dicts without per-element Scalars in to_pylist Thread maps_as_pydicts through the scalar-free _getitem_py mechanism instead of routing it to the Scalar-based path. MapArray builds the dict directly from the flattened keys/items children; when the dict size reveals duplicate keys, the row is redone with the per-key loop so the 'lossy' warnings and 'strict' KeyError match MapScalar.as_py exactly. Invalid values still raise the same ValueError when a map value is converted (including null rows), non-map arrays still ignore the option, and the option still propagates through nested types. Unspecialized types keep the exact Scalar fallback, which now receives the option. Benchmark (M4 Max, 1M rows of 2-entry maps, 10% nulls): to_pylist(maps_as_pydicts='lossy') 2.20s -> 0.10s (~21x). The dict form is also faster than the default association-list form (0.78s), which allocates a 2-tuple per entry. Co-authored-by: Isaac --- python/pyarrow/array.pxi | 88 +++++++++++++++++++----------- python/pyarrow/lib.pxd | 2 +- python/pyarrow/tests/test_array.py | 30 ++++++++++ 3 files changed, 88 insertions(+), 32 deletions(-) diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 97cd9a8ebf8..2e20c74f859 100644 --- a/python/pyarrow/array.pxi +++ b/python/pyarrow/array.pxi @@ -1865,23 +1865,16 @@ cdef class Array(_PandasConvertible): """ self._assert_cpu() 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. - # TODO(GH-50429): this falls back to the Scalar path for the whole - # array even when the type contains no maps; threading - # maps_as_pydicts through _getitem_py keeps the fast paths instead. - return [x.as_py(maps_as_pydicts=maps_as_pydicts) for x in self] # TODO(GH-50448): convert per range instead of per element to cut # the per-element call overhead further. - return [self._getitem_py(i) for i in range(n)] + return [self._getitem_py(i, maps_as_pydicts) for i in range(n)] - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): # Return self[i] as a Python object, without creating a Python Scalar # (nor, for nested types, per-row Array wrappers) where a subclass # provides a specialization; this base implementation goes through # Scalar.as_py and thus preserves its semantics exactly (see GH-50326). - return self.getitem(i).as_py() + return self.getitem(i).as_py(maps_as_pydicts=maps_as_pydicts) def tolist(self): """ @@ -2458,7 +2451,7 @@ cdef class BooleanArray(Array): Concrete class for Arrow arrays of boolean data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None return ( self.ap).Value(i) @@ -2477,7 +2470,7 @@ cdef class NumericArray(Array): A base class for Arrow numeric arrays. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): cdef Type tid = self.ap.type_id() if self.ap.IsNull(i): return None @@ -2503,7 +2496,7 @@ cdef class NumericArray(Array): return ( self.ap).Value(i) # Subclasses whose as_py returns non-primitive objects (dates, times, # timestamps, durations, half floats, ...) use the exact Scalar path. - return Array._getitem_py(self, i) + return Array._getitem_py(self, i, maps_as_pydicts) cdef class IntegerArray(NumericArray): @@ -2823,7 +2816,7 @@ cdef class ListArray(BaseListArray): Concrete class for Arrow arrays of a list data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): cdef CListArray* arr = self.ap if arr.IsNull(i): return None @@ -2831,7 +2824,7 @@ cdef class ListArray(BaseListArray): self._children_cache = pyarrow_wrap_array(arr.values()) cdef Array values = self._children_cache cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i + 1) - return [values._getitem_py(j) for j in range(start, end)] + return [values._getitem_py(j, maps_as_pydicts) for j in range(start, end)] @staticmethod def from_arrays(offsets, values, DataType type=None, MemoryPool pool=None, mask=None): @@ -3018,7 +3011,7 @@ cdef class LargeListArray(BaseListArray): Identical to ListArray, but 64-bit offsets. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): cdef CLargeListArray* arr = self.ap if arr.IsNull(i): return None @@ -3026,7 +3019,7 @@ cdef class LargeListArray(BaseListArray): self._children_cache = pyarrow_wrap_array(arr.values()) cdef Array values = self._children_cache cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i + 1) - return [values._getitem_py(j) for j in range(start, end)] + return [values._getitem_py(j, maps_as_pydicts) for j in range(start, end)] @staticmethod def from_arrays(offsets, values, DataType type=None, MemoryPool pool=None, mask=None): @@ -3618,8 +3611,16 @@ cdef class MapArray(ListArray): Concrete class for Arrow arrays of a map data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): cdef CListArray* arr = self.ap + cdef bint as_dicts = maps_as_pydicts is not None + if as_dicts and maps_as_pydicts != "lossy" and maps_as_pydicts != "strict": + # Matches MapScalar.as_py, which validates before the null check. + raise ValueError( + "Invalid value for 'maps_as_pydicts': " + + "valid values are 'lossy', 'strict' or `None` (default). " + + f"Received {maps_as_pydicts!r}." + ) if arr.IsNull(i): return None if self._children_cache is None: @@ -3627,9 +3628,34 @@ cdef class MapArray(ListArray): cdef Array keys = ( self._children_cache)[0] cdef Array items = ( self._children_cache)[1] cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i + 1) - # Matches MapScalar.as_py with the default maps_as_pydicts=None: - # an association list of (key, value) tuples. - return [(keys._getitem_py(j), items._getitem_py(j)) for j in range(start, end)] + if not as_dicts: + # Matches MapScalar.as_py with the default maps_as_pydicts=None: + # an association list of (key, value) tuples. + return [ + (keys._getitem_py(j, None), items._getitem_py(j, maps_as_pydicts)) + for j in range(start, end) + ] + cdef dict result = {} + for j in range(start, end): + result[keys._getitem_py(j, None)] = items._getitem_py(j, maps_as_pydicts) + if len(result) == end - start: + return result + # Duplicate keys: redo the row with the per-key loop so the 'lossy' + # warnings and 'strict' KeyError match MapScalar.as_py exactly. + result = {} + for j in range(start, end): + key = keys._getitem_py(j, None) + if key in result: + if maps_as_pydicts == "strict": + raise KeyError( + "Converting to Python dictionary is not supported in strict mode " + f"when duplicate keys are present (duplicate key was '{key}')." + ) + else: + warnings.warn( + f"Encountered key '{key}' which was already encountered.") + result[key] = items._getitem_py(j, maps_as_pydicts) + return result @staticmethod def from_arrays(offsets, keys, items, DataType type=None, MemoryPool pool=None, mask=None): @@ -3768,7 +3794,7 @@ cdef class FixedSizeListArray(BaseListArray): Concrete class for Arrow arrays of a fixed size list data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): cdef CFixedSizeListArray* arr = self.ap if arr.IsNull(i): return None @@ -3776,7 +3802,7 @@ cdef class FixedSizeListArray(BaseListArray): self._children_cache = pyarrow_wrap_array(arr.values()) cdef Array values = self._children_cache cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i + 1) - return [values._getitem_py(j) for j in range(start, end)] + return [values._getitem_py(j, maps_as_pydicts) for j in range(start, end)] @staticmethod def from_arrays(values, list_size=None, DataType type=None, mask=None): @@ -4064,7 +4090,7 @@ cdef class StringArray(Array): Concrete class for Arrow arrays of string (or utf8) data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef cpp_string_view view = ( self.ap).GetView(i) @@ -4103,7 +4129,7 @@ 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 object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef cpp_string_view view = ( self.ap).GetView(i) @@ -4141,7 +4167,7 @@ cdef class StringViewArray(Array): Concrete class for Arrow arrays of string (or utf8) view data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef cpp_string_view view = ( self.ap).GetView(i) @@ -4153,7 +4179,7 @@ cdef class BinaryArray(Array): Concrete class for Arrow arrays of variable-sized binary data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef cpp_string_view view = ( self.ap).GetView(i) @@ -4173,7 +4199,7 @@ cdef class LargeBinaryArray(Array): Concrete class for Arrow arrays of large variable-sized binary data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef cpp_string_view view = ( self.ap).GetView(i) @@ -4193,7 +4219,7 @@ cdef class BinaryViewArray(Array): Concrete class for Arrow arrays of variable-sized binary view data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef cpp_string_view view = ( self.ap).GetView(i) @@ -4358,7 +4384,7 @@ cdef class StructArray(Array): Concrete class for Arrow arrays of a struct data type. """ - cdef object _getitem_py(self, int64_t i): + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): if self.ap.IsNull(i): return None cdef int64_t k, num_fields = self.type.num_fields @@ -4377,7 +4403,7 @@ cdef class StructArray(Array): result = {} for k in range(num_fields): field_arr = fields[k] - result[names[k]] = field_arr._getitem_py(i) + result[names[k]] = field_arr._getitem_py(i, maps_as_pydicts) return result def field(self, index): diff --git a/python/pyarrow/lib.pxd b/python/pyarrow/lib.pxd index 38f1ac69a80..8e954323f8a 100644 --- a/python/pyarrow/lib.pxd +++ b/python/pyarrow/lib.pxd @@ -296,7 +296,7 @@ cdef class Array(_PandasConvertible): cdef void init(self, const shared_ptr[CArray]& sp_array) except * cdef getitem(self, int64_t i) - cdef object _getitem_py(self, int64_t i) + cdef object _getitem_py(self, int64_t i, object maps_as_pydicts) cdef int64_t length(self) cdef void _assert_cpu(self) except * diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index c1e3f0128be..ee8d99091d0 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -523,6 +523,36 @@ def test_to_pylist_bulk_paths(): dup.to_pylist() +def test_to_pylist_maps_as_pydicts(): + # GH-50429: maps_as_pydicts converts through the scalar-free path; the + # semantics must match MapScalar.as_py exactly. + map_type = pa.map_(pa.string(), pa.int32()) + arrays = [ + pa.array([[("k1", 1), ("k2", None)], None, []], type=map_type), + pa.array([[[("k", 1)], None], None], type=pa.list_(map_type)), + pa.array([[("o", [("i", 5)])]], + type=pa.map_(pa.string(), map_type)), + pa.array([{"m": [("k", 1)]}, None], + type=pa.struct([("m", map_type)])), + ] + for arr in arrays: + for mode in ("lossy", "strict"): + for view in (arr, arr.slice(1)): + expected = [x.as_py(maps_as_pydicts=mode) for x in view] + assert view.to_pylist(maps_as_pydicts=mode) == expected + + dup = pa.array([[("k", 1), ("k", 2)]], type=map_type) + with pytest.warns(UserWarning, match="already encountered"): + assert dup.to_pylist(maps_as_pydicts="lossy") == [{"k": 2}] + with pytest.raises(KeyError, match="strict mode"): + dup.to_pylist(maps_as_pydicts="strict") + with pytest.raises(ValueError, match="Invalid value for 'maps_as_pydicts'"): + dup.to_pylist(maps_as_pydicts="bogus") + # invalid values are only rejected when a map value is converted, + # matching the Scalar-based behavior + assert pa.array([1, 2]).to_pylist(maps_as_pydicts="bogus") == [1, 2] + + def test_array_slice(): arr = pa.array(range(10)) From 9442c69d308fe3d32b847988b1039417ae0df630 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 9 Jul 2026 08:33:15 -0700 Subject: [PATCH 2/8] GH-50429: [Python] Detect duplicate keys before converting values Convert all keys first (as MapScalar.as_py does via keys()) and check for duplicates before converting any value, so that the 'lossy' warning and the 'strict' KeyError are emitted at the same point as in MapScalar.as_py even when a later value conversion raises. Add tests with a raising value placed after the duplicate key in both modes. lossy 1M rows: 0.16s (vs 2.20s Scalar-based; the extra keys pass costs ~0.06s vs the previous single-pass form). Co-authored-by: Isaac --- python/pyarrow/array.pxi | 23 ++++++++++++++--------- python/pyarrow/tests/test_array.py | 11 +++++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 2e20c74f859..57c34cfeda0 100644 --- a/python/pyarrow/array.pxi +++ b/python/pyarrow/array.pxi @@ -3635,16 +3635,21 @@ cdef class MapArray(ListArray): (keys._getitem_py(j, None), items._getitem_py(j, maps_as_pydicts)) for j in range(start, end) ] + # Convert all keys first (as MapScalar.as_py does via keys()) and + # detect duplicates before converting any value, so that the 'lossy' + # warnings and the 'strict' KeyError are emitted at the same point as + # in MapScalar.as_py even when a later value conversion raises. + cdef int64_t count = end - start + cdef list keys_py = [keys._getitem_py(j, None) for j in range(start, end)] cdef dict result = {} - for j in range(start, end): - result[keys._getitem_py(j, None)] = items._getitem_py(j, maps_as_pydicts) - if len(result) == end - start: + cdef int64_t k + if len(set(keys_py)) == count: + for k in range(count): + result[keys_py[k]] = items._getitem_py(start + k, maps_as_pydicts) return result - # Duplicate keys: redo the row with the per-key loop so the 'lossy' - # warnings and 'strict' KeyError match MapScalar.as_py exactly. - result = {} - for j in range(start, end): - key = keys._getitem_py(j, None) + # Duplicate keys: per-key loop matching MapScalar.as_py exactly. + for k in range(count): + key = keys_py[k] if key in result: if maps_as_pydicts == "strict": raise KeyError( @@ -3654,7 +3659,7 @@ cdef class MapArray(ListArray): else: warnings.warn( f"Encountered key '{key}' which was already encountered.") - result[key] = items._getitem_py(j, maps_as_pydicts) + result[key] = items._getitem_py(start + k, maps_as_pydicts) return result @staticmethod diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index ee8d99091d0..941ef8c2ac2 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -546,6 +546,17 @@ def test_to_pylist_maps_as_pydicts(): assert dup.to_pylist(maps_as_pydicts="lossy") == [{"k": 2}] with pytest.raises(KeyError, match="strict mode"): dup.to_pylist(maps_as_pydicts="strict") + + # Duplicate keys must be detected before converting values: with a + # poison value *after* the duplicate, strict mode raises the outer + # duplicate-key error, and lossy mode still emits its warning first. + nested_map = pa.map_(pa.string(), map_type) + poison = pa.array( + [[("k1", [("a", 1)]), ("k1", [("d", 1), ("d", 2)])]], type=nested_map) + with pytest.raises(KeyError, match="duplicate key was 'k1'"): + poison.to_pylist(maps_as_pydicts="strict") + with pytest.warns(UserWarning, match="Encountered key 'k1'"): + assert poison.to_pylist(maps_as_pydicts="lossy") == [{"k1": {"d": 2}}] with pytest.raises(ValueError, match="Invalid value for 'maps_as_pydicts'"): dup.to_pylist(maps_as_pydicts="bogus") # invalid values are only rejected when a map value is converted, From da9f87d2dce7115ef87284e334768977e80d17c1 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Wed, 22 Jul 2026 07:56:03 -0700 Subject: [PATCH 3/8] GH-50429: [Python] Handle unhashable map keys in the dict fast path set(keys_py) raises TypeError on unhashable keys (struct/list keys) before duplicate detection, diverging from MapScalar.as_py which raises at the first dict membership test. Catch the TypeError and take the per-key loop, which reproduces the Scalar ordering exactly. Co-authored-by: Isaac --- python/pyarrow/array.pxi | 13 +++++++++++-- python/pyarrow/tests/test_array.py | 14 ++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 57c34cfeda0..1fb7d1e65f0 100644 --- a/python/pyarrow/array.pxi +++ b/python/pyarrow/array.pxi @@ -3643,11 +3643,20 @@ cdef class MapArray(ListArray): cdef list keys_py = [keys._getitem_py(j, None) for j in range(start, end)] cdef dict result = {} cdef int64_t k - if len(set(keys_py)) == count: + cdef bint no_dups + try: + no_dups = len(set(keys_py)) == count + except TypeError: + # Unhashable keys (e.g. struct or list keys): the per-key loop + # below reproduces MapScalar.as_py exactly, raising TypeError at + # the same membership test as the Scalar path does. + no_dups = False + if no_dups: for k in range(count): result[keys_py[k]] = items._getitem_py(start + k, maps_as_pydicts) return result - # Duplicate keys: per-key loop matching MapScalar.as_py exactly. + # Duplicate or unhashable keys: per-key loop matching MapScalar.as_py + # exactly. for k in range(count): key = keys_py[k] if key in result: diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index 941ef8c2ac2..855da1e7199 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -563,6 +563,20 @@ def test_to_pylist_maps_as_pydicts(): # matching the Scalar-based behavior assert pa.array([1, 2]).to_pylist(maps_as_pydicts="bogus") == [1, 2] + # Unhashable keys (e.g. struct keys) must fail exactly like the Scalar + # path: TypeError from the first dict membership test, in both modes. + # The association-list mode involves no hashing and must keep working. + struct_keyed = pa.MapArray.from_arrays( + [0, 2], + pa.array([{"a": 1}, {"a": 2}], type=pa.struct([("a", pa.int32())])), + pa.array([10, 20], type=pa.int32())) + assert struct_keyed.to_pylist() == [x.as_py() for x in struct_keyed] + for mode in ("lossy", "strict"): + with pytest.raises(TypeError, match="unhashable"): + struct_keyed.to_pylist(maps_as_pydicts=mode) + with pytest.raises(TypeError, match="unhashable"): + [x.as_py(maps_as_pydicts=mode) for x in struct_keyed] + def test_array_slice(): arr = pa.array(range(10)) From e309a6f768e91adcb207a13700c2555a99e7b216 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 23 Jul 2026 08:10:03 -0700 Subject: [PATCH 4/8] GH-50429: [Python] Always use the per-key loop for dict conversion Per rok's review: the set()-based duplicate pre-check allocates a temporary set and traverses the keys an extra time; the per-key loop hashes unique keys the same number of times, needs no unhashable-key special case, and benchmarks 5-20% faster across unique/duplicate key workloads. Co-authored-by: Isaac --- python/pyarrow/array.pxi | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 1fb7d1e65f0..569d5adf335 100644 --- a/python/pyarrow/array.pxi +++ b/python/pyarrow/array.pxi @@ -3635,28 +3635,13 @@ cdef class MapArray(ListArray): (keys._getitem_py(j, None), items._getitem_py(j, maps_as_pydicts)) for j in range(start, end) ] - # Convert all keys first (as MapScalar.as_py does via keys()) and - # detect duplicates before converting any value, so that the 'lossy' - # warnings and the 'strict' KeyError are emitted at the same point as - # in MapScalar.as_py even when a later value conversion raises. + # MapScalar.as_py converts every key before processing values, then + # checks each key immediately before converting its corresponding value. cdef int64_t count = end - start - cdef list keys_py = [keys._getitem_py(j, None) for j in range(start, end)] + cdef list keys_py = [keys._getitem_py(j, None) + for j in range(start, end)] cdef dict result = {} cdef int64_t k - cdef bint no_dups - try: - no_dups = len(set(keys_py)) == count - except TypeError: - # Unhashable keys (e.g. struct or list keys): the per-key loop - # below reproduces MapScalar.as_py exactly, raising TypeError at - # the same membership test as the Scalar path does. - no_dups = False - if no_dups: - for k in range(count): - result[keys_py[k]] = items._getitem_py(start + k, maps_as_pydicts) - return result - # Duplicate or unhashable keys: per-key loop matching MapScalar.as_py - # exactly. for k in range(count): key = keys_py[k] if key in result: @@ -3665,9 +3650,8 @@ cdef class MapArray(ListArray): "Converting to Python dictionary is not supported in strict mode " f"when duplicate keys are present (duplicate key was '{key}')." ) - else: - warnings.warn( - f"Encountered key '{key}' which was already encountered.") + warnings.warn( + f"Encountered key '{key}' which was already encountered.") result[key] = items._getitem_py(start + k, maps_as_pydicts) return result From e7a82e3727697fd88395d1b2e20283038d3a29cb Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 23 Jul 2026 08:36:48 -0700 Subject: [PATCH 5/8] GH-50429: [Python] Preserve StructScalar.as_py's nested KeyError translation StructScalar.as_py wraps field-value conversion in an except KeyError that raises the duplicate-field-names ValueError, so a nested map in 'strict' mode surfacing a duplicate-key KeyError comes out as that ValueError on the Scalar path. Reproduce the translation in StructArray._getitem_py, and apply the reviewer's test refinements: strict-only success matrix with a meaningful sliced case, explicit warning-sequence assertion, null-map invalid-mode validation, nested KeyError translation coverage, and a trimmed unhashable-keys block. Co-authored-by: Isaac --- python/pyarrow/array.pxi | 15 ++++++++--- python/pyarrow/tests/test_array.py | 42 ++++++++++++++++++------------ 2 files changed, 38 insertions(+), 19 deletions(-) diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 569d5adf335..527ddf37d22 100644 --- a/python/pyarrow/array.pxi +++ b/python/pyarrow/array.pxi @@ -4399,9 +4399,18 @@ cdef class StructArray(Array): fields = ( self._children_cache)[1] cdef Array field_arr result = {} - for k in range(num_fields): - field_arr = fields[k] - result[names[k]] = field_arr._getitem_py(i, maps_as_pydicts) + try: + for k in range(num_fields): + field_arr = fields[k] + result[names[k]] = field_arr._getitem_py(i, maps_as_pydicts) + except KeyError: + # StructScalar.as_py translates any KeyError raised while + # converting the field values (e.g. a nested map in 'strict' + # mode) into its duplicate-field-names ValueError; reproduce + # that translation exactly. + raise ValueError( + "Converting to Python dictionary is not supported when " + "duplicate field names are present") return result def field(self, index): diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index 855da1e7199..8758a490c3d 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -527,19 +527,20 @@ def test_to_pylist_maps_as_pydicts(): # GH-50429: maps_as_pydicts converts through the scalar-free path; the # semantics must match MapScalar.as_py exactly. map_type = pa.map_(pa.string(), pa.int32()) + flat = pa.array( + [None, [("k1", 1), ("k2", None)], []], type=map_type) arrays = [ - pa.array([[("k1", 1), ("k2", None)], None, []], type=map_type), - pa.array([[[("k", 1)], None], None], type=pa.list_(map_type)), + flat, + flat.slice(1), + pa.array([[[('k', 1)], None], None], type=pa.list_(map_type)), pa.array([[("o", [("i", 5)])]], type=pa.map_(pa.string(), map_type)), pa.array([{"m": [("k", 1)]}, None], type=pa.struct([("m", map_type)])), ] for arr in arrays: - for mode in ("lossy", "strict"): - for view in (arr, arr.slice(1)): - expected = [x.as_py(maps_as_pydicts=mode) for x in view] - assert view.to_pylist(maps_as_pydicts=mode) == expected + expected = [x.as_py(maps_as_pydicts="strict") for x in arr] + assert arr.to_pylist(maps_as_pydicts="strict") == expected dup = pa.array([[("k", 1), ("k", 2)]], type=map_type) with pytest.warns(UserWarning, match="already encountered"): @@ -549,23 +550,34 @@ def test_to_pylist_maps_as_pydicts(): # Duplicate keys must be detected before converting values: with a # poison value *after* the duplicate, strict mode raises the outer - # duplicate-key error, and lossy mode still emits its warning first. + # duplicate-key error, and lossy mode warns before converting that value. nested_map = pa.map_(pa.string(), map_type) poison = pa.array( [[("k1", [("a", 1)]), ("k1", [("d", 1), ("d", 2)])]], type=nested_map) with pytest.raises(KeyError, match="duplicate key was 'k1'"): poison.to_pylist(maps_as_pydicts="strict") - with pytest.warns(UserWarning, match="Encountered key 'k1'"): + with pytest.warns(UserWarning) as caught: assert poison.to_pylist(maps_as_pydicts="lossy") == [{"k1": {"d": 2}}] + assert [str(warning.message) for warning in caught] == [ + "Encountered key 'k1' which was already encountered.", + "Encountered key 'd' which was already encountered.", + ] + + # Preserve StructScalar.as_py's translation of nested KeyErrors. + nested_duplicate = pa.array( + [{"m": [("k", 1), ("k", 2)]}], + type=pa.struct([("m", map_type)])) + with pytest.raises(ValueError, match="duplicate field names"): + nested_duplicate.to_pylist(maps_as_pydicts="strict") + + null_map = pa.array([None], type=map_type) with pytest.raises(ValueError, match="Invalid value for 'maps_as_pydicts'"): - dup.to_pylist(maps_as_pydicts="bogus") - # invalid values are only rejected when a map value is converted, - # matching the Scalar-based behavior + null_map.to_pylist(maps_as_pydicts="bogus") + # Invalid values are only rejected when a map value is converted. assert pa.array([1, 2]).to_pylist(maps_as_pydicts="bogus") == [1, 2] - # Unhashable keys (e.g. struct keys) must fail exactly like the Scalar - # path: TypeError from the first dict membership test, in both modes. - # The association-list mode involves no hashing and must keep working. + # The association-list mode does not hash unhashable map keys, while + # either dictionary mode raises at the first membership test. struct_keyed = pa.MapArray.from_arrays( [0, 2], pa.array([{"a": 1}, {"a": 2}], type=pa.struct([("a", pa.int32())])), @@ -574,8 +586,6 @@ def test_to_pylist_maps_as_pydicts(): for mode in ("lossy", "strict"): with pytest.raises(TypeError, match="unhashable"): struct_keyed.to_pylist(maps_as_pydicts=mode) - with pytest.raises(TypeError, match="unhashable"): - [x.as_py(maps_as_pydicts=mode) for x in struct_keyed] def test_array_slice(): From 57e004dc5440e7a45b8659396d7c9efbaebf50ab Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 23 Jul 2026 10:08:37 -0700 Subject: [PATCH 6/8] GH-50429: [Python] Validate maps_as_pydicts like MapScalar.as_py Per review: drop the as_dicts flag and use the same membership-test validation form as MapScalar.as_py; no measurable perf difference on either map path. Co-authored-by: Isaac --- python/pyarrow/array.pxi | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 527ddf37d22..4db5536525f 100644 --- a/python/pyarrow/array.pxi +++ b/python/pyarrow/array.pxi @@ -3613,8 +3613,7 @@ cdef class MapArray(ListArray): cdef object _getitem_py(self, int64_t i, object maps_as_pydicts): cdef CListArray* arr = self.ap - cdef bint as_dicts = maps_as_pydicts is not None - if as_dicts and maps_as_pydicts != "lossy" and maps_as_pydicts != "strict": + if maps_as_pydicts not in (None, "lossy", "strict"): # Matches MapScalar.as_py, which validates before the null check. raise ValueError( "Invalid value for 'maps_as_pydicts': " @@ -3628,7 +3627,7 @@ cdef class MapArray(ListArray): cdef Array keys = ( self._children_cache)[0] cdef Array items = ( self._children_cache)[1] cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i + 1) - if not as_dicts: + if maps_as_pydicts is None: # Matches MapScalar.as_py with the default maps_as_pydicts=None: # an association list of (key, value) tuples. return [ From ffbb17c41a1be8f08cedfe6b337882960183727f Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 23 Jul 2026 10:14:48 -0700 Subject: [PATCH 7/8] GH-50429: [Python] Use literal expected values in the maps_as_pydicts test ListScalar.as_py delegates to Array.to_pylist, so the scalar-built reference for the list case exercised the code under test; write the expected rows out literally to keep the oracle independent. Co-authored-by: Isaac --- python/pyarrow/tests/test_array.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index 8758a490c3d..ff75a882a43 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -529,17 +529,21 @@ def test_to_pylist_maps_as_pydicts(): map_type = pa.map_(pa.string(), pa.int32()) flat = pa.array( [None, [("k1", 1), ("k2", None)], []], type=map_type) - arrays = [ - flat, - flat.slice(1), - pa.array([[[('k', 1)], None], None], type=pa.list_(map_type)), - pa.array([[("o", [("i", 5)])]], - type=pa.map_(pa.string(), map_type)), - pa.array([{"m": [("k", 1)]}, None], - type=pa.struct([("m", map_type)])), + # Expected values are written out literally so the reference stays + # independent of Array.to_pylist (ListScalar.as_py delegates to it). + cases = [ + (flat, [None, {"k1": 1, "k2": None}, {}]), + (flat.slice(1), [{"k1": 1, "k2": None}, {}]), + (pa.array([[[('k', 1)], None], None], type=pa.list_(map_type)), + [[{"k": 1}, None], None]), + (pa.array([[("o", [("i", 5)])]], + type=pa.map_(pa.string(), map_type)), + [{"o": {"i": 5}}]), + (pa.array([{"m": [("k", 1)]}, None], + type=pa.struct([("m", map_type)])), + [{"m": {"k": 1}}, None]), ] - for arr in arrays: - expected = [x.as_py(maps_as_pydicts="strict") for x in arr] + for arr, expected in cases: assert arr.to_pylist(maps_as_pydicts="strict") == expected dup = pa.array([[("k", 1), ("k", 2)]], type=map_type) From a650d5a77b0fe737ccb664204c7156f23df4d0b5 Mon Sep 17 00:00:00 2001 From: Liang-Chi Hsieh Date: Thu, 23 Jul 2026 10:50:20 -0700 Subject: [PATCH 8/8] GH-50429: [Python] Cover large_list and fixed_size_list in the maps parity matrix Per review: LargeListArray._getitem_py and FixedSizeListArray._getitem_py thread maps_as_pydicts to their values but were not exercised by the parity cases; add both with the same data and literal expected rows. Co-authored-by: Isaac --- python/pyarrow/tests/test_array.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py index ff75a882a43..e07699fbe01 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -536,6 +536,10 @@ def test_to_pylist_maps_as_pydicts(): (flat.slice(1), [{"k1": 1, "k2": None}, {}]), (pa.array([[[('k', 1)], None], None], type=pa.list_(map_type)), [[{"k": 1}, None], None]), + (pa.array([[[('k', 1)], None], None], type=pa.large_list(map_type)), + [[{"k": 1}, None], None]), + (pa.array([[[('k', 1)], None], None], type=pa.list_(map_type, 2)), + [[{"k": 1}, None], None]), (pa.array([[("o", [("i", 5)])]], type=pa.map_(pa.string(), map_type)), [{"o": {"i": 5}}]),