viirya commented on PR #50327: URL: https://github.com/apache/arrow/pull/50327#issuecomment-4915939607
Thanks for the review, @pitrou! Agreed — the per-type `to_pylist` overrides don't generalize. I'll rework the PR along your suggestion, which I read as a scalar-free `getitem`: 1. Add `cdef object _getitem_py(self, int64_t i, ...)` on `Array`, whose base implementation preserves today's semantics exactly (`GetScalar(i)` + `as_py()`), so any type without a specialization behaves identically to today. 2. Make the baseline `Array.to_pylist` a single loop over `_getitem_py`. 3. Specialize `_getitem_py` for the common types: integers, floats, boolean, string/binary (+ large variants), list-likes (build each row's list directly from the child's `_getitem_py` over the offset range), struct and map — nested types then compose without allocating any Scalar or Array wrapper per row, and `maps_as_pydicts` threads through as an argument. Types with non-trivial `as_py` semantics (dates/timestamps/decimals/dictionary/...) would keep the exact fallback initially and can be specialized incrementally in follow-ups; your `int64` example above lands in the specialized bucket and should get within striking distance of `ndarray.tolist`. One question before I rework: do you prefer this mechanism in Cython as sketched, or as a C++ `ToPyList` visitor under `python/pyarrow/src/arrow/python/` (in the spirit of `MonthDayNanoIntervalArrayToPyList`)? The Cython version keeps the unspecialized fallback trivially exact and the diff reviewable; the C++ visitor is likely faster still (and would also serve `ChunkedArray`/`Table` uniformly) but is a much bigger surface. Happy to go either way. -- 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]
