GH-50429: [Python] Convert maps to dicts without per-element Scalars in to_pylist by viirya · Pull Request #50430 · apache/arrow

viirya · 2026-07-08T22:21:12Z

Rationale for this change

Follow-up of #50326, building on the merged #50327.

GH-50327 converts arrays to Python objects without per-element Scalars, but the maps_as_pydicts option still routes to the Scalar-based path: every map row allocates a MapScalar, converts its keys via per-element as_py, and builds the dict in Python. Map→dict is the natural consumption pattern for engines whose map values are Python dicts (e.g. Spark's Arrow-serialized Python UDFs currently receive association lists and rebuild a dict per row in pure Python — one of the dominant remaining costs in that path).

What changes are included in this PR?

Thread maps_as_pydicts through the scalar-free _getitem_py mechanism:

MapArray._getitem_py converts the row's keys first (as MapScalar.as_py does via keys()), then builds the dict with a per-key loop that checks each key immediately before converting its value — so the 'lossy' warnings, the 'strict' KeyError, and the TypeError for unhashable keys are raised at exactly the same points as MapScalar.as_py (including messages and warning-per-duplicate behavior). StructArray._getitem_py also reproduces StructScalar.as_py's translation of nested KeyErrors into the duplicate-field-names ValueError.
Invalid option values raise the same ValueError when a map value is converted — including null map rows, matching the Scalar path — while non-map arrays keep ignoring the option.
The option propagates through nested types (list/struct children, map values) as before, and unspecialized types keep the exact Scalar fallback, which now receives the option.

Benchmark (macOS arm64, M4 Max; 1M rows of 2-entry map<string,int64>, 10% nulls):

conversion	before	after	speedup
`to_pylist(maps_as_pydicts='lossy')`	2.45 s	0.13 s	~19x

Notably the dict form is now also faster than the default association-list form (0.49 s), which allocates a 2-tuple per entry.

Are these changes tested?

New test_to_pylist_maps_as_pydicts compares against the per-scalar conversion for flat maps, list<map>, map<string, map<...>> and struct<map> (plain and sliced) in both modes, and asserts the duplicate-key semantics ('lossy' warns and keeps the last value; 'strict' raises KeyError), the exact warning sequence when a raising value follows a duplicate key, the unhashable-key TypeError parity (struct-keyed maps), the nested KeyError→ValueError translation inside structs, the invalid-value ValueError (including on null map rows), and that non-map arrays ignore the option. Randomized differential tests against the Scalar path (exact type equality) and pytest test_array.py test_scalars.py test_convert_builtin.py test_table.py (1210 passed) also pass.

Are there any user-facing changes?

No behavior changes, only performance.

GitHub Issue: [Python] Convert maps to dicts without per-element Scalars in to_pylist(maps_as_pydicts=...) #50429

This pull request and its description were written by Isaac.

github-actions · 2026-07-08T22:21:36Z

⚠️ GitHub issue #50429 has been automatically assigned in GitHub to PR creator.

Copilot

Pull request overview

This PR extends the scalar-free Array.to_pylist() conversion path to support maps_as_pydicts for MapArray, enabling direct Map→dict materialization without per-element Scalar allocation while preserving existing MapScalar.as_py semantics.

Changes:

Add a cdef _getitem_py(i, maps_as_pydicts) mechanism and update Array.to_pylist() to use it for scalar-free conversions.
Implement _getitem_py specializations for common leaf and nested array types (numeric, boolean, string/binary, list, struct, map), with caching of wrapped child arrays.
Add tests comparing to_pylist() results against per-scalar conversion, including maps_as_pydicts behaviors and nested compositions.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
python/pyarrow/array.pxi	Implements `_getitem_py` and adds scalar-free `to_pylist` specializations, including `MapArray` dict conversion for `maps_as_pydicts`.
python/pyarrow/lib.pxd	Declares new `Array` Cython API surface (`_children_cache`, `_getitem_py`).
python/pyarrow/tests/test_array.py	Adds regression/differential tests for scalar-free `to_pylist` and `maps_as_pydicts` semantics.

+        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


Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

viirya · 2026-07-09T15:37:17Z

+        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


Good catch — this was a real ordering difference. Fixed in 8626b97: all keys are now converted first (matching MapScalar.as_py, which converts them via keys()), and duplicates are detected before any value conversion, so the 'strict' KeyError and the 'lossy' warning are emitted at the same point as the Scalar path even when a later value conversion raises. Added tests with a raising value placed after the duplicate key in both modes, and verified the exception/warning sequences match the Scalar path exactly (including nested-map warnings order).

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

viirya · 2026-07-22T14:41:35Z

+        if self._children_cache is None:
+            self._children_cache = pyarrow_wrap_array(arr.values())
+        cdef Array values = <Array> self._children_cache
+        cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i + 1)
+        return [values._getitem_py(j, maps_as_pydicts) for j in range(start, end)]


Good catch — the C++ signatures all take int64_t i, so the int declarations would let Cython silently truncate 64-bit indices on the new call paths. Fixed in the base PR #50327 (9b1863d), which owns these declarations and the list/map _getitem_py call sites; this branch is rebased on it. All list-like, binary and dense-union value_offset/value_length declarations now match the C++ headers.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

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

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

viirya · 2026-07-23T17:15:56Z

+    for arr in arrays:
+        expected = [x.as_py(maps_as_pydicts="strict") for x in arr]
+        assert arr.to_pylist(maps_as_pydicts="strict") == expected
+


Right — ListScalar.as_py delegates to Array.to_pylist, so that reference wasn't independent for the list<map> case. Fixed in ffbb17c by writing the expected rows out literally for all five cases, which keeps the oracle fully independent of the code under test.

…ydicts test ListScalar.as_py delegates to Array.to_pylist, so the scalar-built reference for the list<map> case exercised the code under test; write the expected rows out literally to keep the oracle independent. Co-authored-by: Isaac

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

rok · 2026-07-23T17:31:45Z

Nice work — the parity coverage here is careful (the poison-value-after-duplicate-key ordering test in particular).

One small test gap: LargeListArray._getitem_py and FixedSizeListArray._getitem_py now thread maps_as_pydicts through to their values, but the parity matrix in test_to_pylist_maps_as_pydicts only covers list_(map). Two more entries would exercise those overrides:

     arrays = [
         flat,
         flat.slice(1),
         pa.array([[[('k', 1)], None], None], type=pa.list_(map_type)),
+        pa.array([[[('k', 1)], None], None], type=pa.large_list(map_type)),
+        pa.array([[[('k', 1)], None], None], type=pa.list_(map_type, 2)),
         pa.array([[("o", [("i", 5)])]],
                  type=pa.map_(pa.string(), map_type)),
         pa.array([{"m": [("k", 1)]}, None],
                  type=pa.struct([("m", map_type)])),
     ]

The same data works for all three (each non-null outer element has exactly 2 children, so it's valid for the fixed-size list too), and it mirrors how test_to_pylist_bulk_paths covers large_list/list_(..., 2) alongside list_. The duplicate-key/strict/lossy sections don't need changes — those semantics live in MapArray._getitem_py, which is already covered; this just pins that the two list variants keep propagating the option.

…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

viirya · 2026-07-23T17:50:33Z

Added in a650d5a — both variants with the same data and literal expected rows. Thanks for the careful review!

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings July 8, 2026 22:21

viirya requested review from AlenkaF, raulcd and rok as code owners July 8, 2026 22:21

Copilot started reviewing on behalf of viirya July 8, 2026 22:21 View session

github-actions Bot added Component: Python awaiting committer review Awaiting committer review labels Jul 8, 2026

Copilot AI reviewed Jul 8, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 9, 2026 14:51

viirya force-pushed the GH-50429-maps-as-pydicts-fast branch from bca5c9a to 0a1fee8 Compare July 9, 2026 14:51

Copilot started reviewing on behalf of viirya July 9, 2026 14:52 View session

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 9, 2026 15:33

Copilot started reviewing on behalf of viirya July 9, 2026 15:34 View session

github-actions Bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jul 9, 2026

Copilot AI reviewed Jul 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 22, 2026 14:13

viirya force-pushed the GH-50429-maps-as-pydicts-fast branch from 8626b97 to e66acd7 Compare July 22, 2026 14:13

Copilot started reviewing on behalf of viirya July 22, 2026 14:14 View session

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 22, 2026

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 22, 2026 14:41

viirya force-pushed the GH-50429-maps-as-pydicts-fast branch from e66acd7 to 7e49564 Compare July 22, 2026 14:41

viirya mentioned this pull request Jul 22, 2026

GH-50326: [Python] Convert arrays to Python objects without per-element Scalars in to_pylist #50327

Merged

Copilot started reviewing on behalf of viirya July 22, 2026 14:42 View session

github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jul 22, 2026

Copilot started reviewing on behalf of viirya July 23, 2026 15:44 View session

github-actions Bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Jul 23, 2026

Copilot AI reviewed Jul 23, 2026

View reviewed changes

rok reviewed Jul 23, 2026

View reviewed changes

Comment thread python/pyarrow/array.pxi Outdated

apacheGH-50429: [Python] Validate maps_as_pydicts like MapScalar.as_py

57e004d

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

Copilot AI review requested due to automatic review settings July 23, 2026 17:08

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 23, 2026

Copilot started reviewing on behalf of viirya July 23, 2026 17:09 View session

github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Jul 23, 2026

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 17:15

Copilot started reviewing on behalf of viirya July 23, 2026 17:16 View session

github-actions Bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Jul 23, 2026

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings July 23, 2026 17:50

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 23, 2026

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Uh oh!

Conversation

viirya commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Jul 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

viirya Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

viirya Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

viirya Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

rok commented Jul 23, 2026

Uh oh!

viirya commented Jul 23, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

viirya commented Jul 8, 2026 •

edited

Loading