Copilot commented on code in PR #50430:
URL: https://github.com/apache/arrow/pull/50430#discussion_r3547640283


##########
python/pyarrow/array.pxi:
##########
@@ -3551,6 +3613,52 @@ cdef class MapArray(ListArray):
     Concrete class for Arrow arrays of a map data type.
     """
 
+    cdef object _getitem_py(self, int64_t i, object maps_as_pydicts):
+        cdef CListArray* arr = <CListArray*> 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:
+            self._children_cache = (self.keys, self.items)
+        cdef Array keys = <Array> (<tuple> self._children_cache)[0]
+        cdef Array items = <Array> (<tuple> self._children_cache)[1]
+        cdef int64_t j, start = arr.value_offset(i), end = arr.value_offset(i 
+ 1)
+        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

Review Comment:
   In the dict conversion fast path, values are converted before duplicate keys 
are detected. This changes MapScalar.as_py semantics for both 'strict' and 
'lossy': MapScalar checks for duplicates (and raises/warns) *before* converting 
the duplicate entry’s value, but the current first pass may raise from value 
conversion (or skip the warning) before the duplicate-key handling runs.



-- 
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]

Reply via email to