diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi index 97cd9a8ebf8..4db5536525f 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,15 @@ 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 + 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': " + + "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 +3627,32 @@ 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 maps_as_pydicts is None: + # 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) + ] + # 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 dict result = {} + cdef int64_t k + for k in range(count): + key = keys_py[k] + 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}')." + ) + warnings.warn( + f"Encountered key '{key}' which was already encountered.") + result[key] = items._getitem_py(start + k, maps_as_pydicts) + return result @staticmethod def from_arrays(offsets, keys, items, DataType type=None, MemoryPool pool=None, mask=None): @@ -3768,7 +3791,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 +3799,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 +4087,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 +4126,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 +4164,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 +4176,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 +4196,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 +4216,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 +4381,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 @@ -4375,9 +4398,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) + 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/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..e07699fbe01 100644 --- a/python/pyarrow/tests/test_array.py +++ b/python/pyarrow/tests/test_array.py @@ -523,6 +523,79 @@ 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()) + flat = pa.array( + [None, [("k1", 1), ("k2", None)], []], type=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([[[('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}}]), + (pa.array([{"m": [("k", 1)]}, None], + type=pa.struct([("m", map_type)])), + [{"m": {"k": 1}}, None]), + ] + for arr, expected in cases: + 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"): + 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 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) 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'"): + 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] + + # 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())])), + 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) + + def test_array_slice(): arr = pa.array(range(10))