-
Notifications
You must be signed in to change notification settings - Fork 4.2k
GH-50326: [Python] Convert arrays to Python objects without per-element Scalars in to_pylist #50327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
12b921e
7a15077
985a2e3
728584f
3d303ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -465,6 +465,38 @@ def test_array_getitem_numpy_scalars(): | |
| assert arr[np.int32(idx)].as_py() == lst[idx] | ||
|
|
||
|
|
||
| def test_to_pylist_bulk_paths(): | ||
| # GH-50326: list-like and string arrays convert to Python objects in | ||
| # bulk instead of going through one Scalar per element; the result must | ||
| # match the per-scalar conversion exactly. | ||
| arrays = [ | ||
| pa.array([[1, None, 3], None, [], [4]], type=pa.list_(pa.int32())), | ||
| pa.array([["a", None], None, [], ["bcd", ""]], | ||
| type=pa.list_(pa.string())), | ||
| pa.array([["a", None], None, [], ["bcd", ""]], | ||
| type=pa.large_list(pa.large_string())), | ||
| pa.array([[1, None], None, [3, 4]], type=pa.list_(pa.int32(), 2)), | ||
| pa.array([[[1], [2, None]], None, [None, [3]]], | ||
| type=pa.list_(pa.list_(pa.int32()))), | ||
| pa.array([[("k1", 1), ("k2", None)], None, []], | ||
| type=pa.map_(pa.string(), pa.int32())), | ||
| pa.array(["a", None, "", "\N{GRINNING FACE} \N{SNOWMAN}"], | ||
| type=pa.string()), | ||
| pa.array(["a", None, "", "\N{GRINNING FACE} \N{SNOWMAN}"], | ||
| type=pa.large_string()), | ||
| pa.array([], type=pa.list_(pa.int32())), | ||
| pa.array([None, None], type=pa.list_(pa.string())), | ||
| ] | ||
|
Comment on lines
+472
to
+501
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added binary/large_binary (including embedded NUL bytes), list, wide-range integers, floats, boolean and struct coverage in 728584f, plus an assertion that duplicate struct field names still raise
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we do
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
| for arr in arrays: | ||
| for view in (arr, arr.slice(1), arr.slice(0, 2), arr.slice(2)): | ||
| assert view.to_pylist() == [x.as_py() for x in view] | ||
|
|
||
| # Values inside numeric lists must stay Python ints/None, never floats | ||
| result = pa.array([[1, None, 3]], type=pa.list_(pa.int32())).to_pylist() | ||
| assert result == [[1, None, 3]] | ||
| assert [type(x) for x in result[0]] == [int, type(None), int] | ||
|
|
||
|
|
||
| def test_array_slice(): | ||
| arr = pa.array(range(10)) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null_count()is a one-time vectorized popcount over the validity bitmap (~n/8 bytes, well under a millisecond for 2M rows), computed and cached perArrayData. In exchange, the no-null branch skips the per-elementIsNull()check entirely. Branching onnull_bitmap_data() == NULLinstead would save that single scan but degrade the common case of a sliced/combined array that has a bitmap yet contains no nulls in range — that would take the per-elementIsNull()path forever. So the current form should be the better trade-off in practice.