viirya commented on code in PR #50430:
URL: https://github.com/apache/arrow/pull/50430#discussion_r3639451457
##########
python/pyarrow/tests/test_array.py:
##########
@@ -523,6 +523,61 @@ 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")
+
+ # 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,
+ # matching the Scalar-based behavior
+ assert pa.array([1, 2]).to_pylist(maps_as_pydicts="bogus") == [1, 2]
+
Review Comment:
Confirmed — `StructScalar.as_py` translates the nested `KeyError` into the
duplicate-field-names `ValueError` via its `except KeyError`, and the
scalar-free struct path was letting the `KeyError` propagate. Fixed in
e7a82e3727 by reproducing the translation in `StructArray._getitem_py`, and
added your test. Good catch.
##########
python/pyarrow/tests/test_array.py:
##########
@@ -523,6 +523,61 @@ 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
Review Comment:
Applied in e7a82e3727.
##########
python/pyarrow/tests/test_array.py:
##########
@@ -523,6 +523,61 @@ 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")
+
+ # 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,
+ # matching the Scalar-based behavior
+ assert pa.array([1, 2]).to_pylist(maps_as_pydicts="bogus") == [1, 2]
Review Comment:
Applied in e7a82e3727.
##########
python/pyarrow/tests/test_array.py:
##########
@@ -523,6 +523,61 @@ 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")
+
+ # 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,
+ # 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]
Review Comment:
Applied in e7a82e3727.
--
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]
