You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "AlenkaF (via GitHub)" <gi...@apache.org> on 2024/03/04 13:55:33 UTC
[PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
AlenkaF opened a new pull request, #40341:
URL: https://github.com/apache/arrow/pull/40341
### Rationale for this change
We want to enable the construction of a Run-End Encoded arrays with `pyarrow.array `constructor
### What changes are included in this PR?
Added a check for Run-End Encoded Type in the `pyarrow.array` constructor code.
### Are these changes tested?
Yes, added test_run_end_encoded_from_array_with_type.
### Are there any user-facing changes?
No.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1512333326
##########
python/pyarrow/array.pxi:
##########
@@ -339,8 +339,12 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
pool)
else:
+ if type and type.id == _Type_RUN_END_ENCODED:
+ from pyarrow.compute import run_end_encode
+ result = run_end_encode(obj)
Review Comment:
Makes sense!
Would it be a good approach to keep the compute function to actually get `.run_ends` and `.values` and then call `RunEndEncodedArray.from_arrays(run_ends, values, ree_type)` with the correct ree type? (it shouldn't make a copy at this point)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #40341:
URL: https://github.com/apache/arrow/pull/40341#issuecomment-2009466858
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit dd3d3cd1be27da7c872bfced553f25b8a0240021.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/22881472746) has more details. It also includes information about 24 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1516562496
##########
python/pyarrow/array.pxi:
##########
@@ -118,11 +118,12 @@ def _handle_arrow_array_protocol(obj, type, mask, size):
return res
-def _handle_run_end_encoded_arrays(obj, type):
+cdef _handle_run_end_encoded_arrays(object sequence, object mask, object size,
+ DataType type, CMemoryPool* pool,
+ c_bool from_pandas):
from pyarrow.compute import run_end_encode
- ree_arr = run_end_encode(obj)
- return RunEndEncodedArray.from_arrays(
- ree_arr.run_ends.to_pylist(), ree_arr.values.to_pylist(), type)
+ arr = _sequence_to_array(sequence, mask, size, type.value_type, pool, from_pandas)
Review Comment:
Thinking that the code for each of the two separate cases (sequence and array-like) is now minimal and maybe we do not need a helper function anymore?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on PR #40341:
URL: https://github.com/apache/arrow/pull/40341#issuecomment-1983931630
I think the last change addresses al of your latest comments @jorisvandenbossche (https://github.com/apache/arrow/pull/40341/commits/4ebb3b91a5c26676f2b12c1881b379c13b90feea).
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #40341:
URL: https://github.com/apache/arrow/pull/40341#issuecomment-1999325573
Can you rebase this once more? (builds are all failing because of something that was fixed on main)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1516500530
##########
python/pyarrow/array.pxi:
##########
@@ -118,11 +118,12 @@ def _handle_arrow_array_protocol(obj, type, mask, size):
return res
-def _handle_run_end_encoded_arrays(obj, type):
+cdef _handle_run_end_encoded_arrays(object sequence, object mask, object size,
+ DataType type, CMemoryPool* pool,
+ c_bool from_pandas):
from pyarrow.compute import run_end_encode
- ree_arr = run_end_encode(obj)
- return RunEndEncodedArray.from_arrays(
- ree_arr.run_ends.to_pylist(), ree_arr.values.to_pylist(), type)
+ arr = _sequence_to_array(sequence, mask, size, type.value_type, pool, from_pandas)
Review Comment:
Here, we now use the more generic `_sequence_to_array`, also when the input is a ndarray. So maybe something like:
```
if isinstance(sequence, np,ndarray):
arr = _ndarray_to_array(values, mask, type, c_from_pandas, safe, pool)
else:
arr = _sequence_to_array(sequence, mask, size, type.value_type, pool, from_pandas)
```
?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1526664796
##########
python/pyarrow/tests/test_array.py:
##########
@@ -3579,6 +3579,47 @@ def test_run_end_encoded_from_buffers():
1, offset, children)
+def test_run_end_encoded_from_array_with_type():
+ run_ends = [1, 3, 6]
+ values = [1, 2, 3]
+ ree_type = pa.run_end_encoded(pa.int32(), pa.int64())
+ expected = pa.RunEndEncodedArray.from_arrays(run_ends, values,
+ ree_type)
+
+ arr = [1, 2, 2, 3, 3, 3]
+ result = pa.array(arr)
+ assert result.equals(expected)
Review Comment:
It's a bit annoying I can't test locally, it's making me quite confused. Looking at this part now, I do not know what I was trying to test really. Hope I will fix pytest soon =)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche merged PR #40341:
URL: https://github.com/apache/arrow/pull/40341
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1515784637
##########
python/pyarrow/array.pxi:
##########
@@ -336,11 +336,27 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
if pandas_api.have_pandas:
values, type = pandas_api.compat.get_datetimetz_type(
values, obj.dtype, type)
- result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
- pool)
+ if type and type.id == _Type_RUN_END_ENCODED:
+ if mask is not None:
+ raise ValueError("Cannot pass a mask for Run-End Encoded arrays.")
Review Comment:
The discussion we had before was mostly about when you passed _pyarrow_ arrays, while here we are treating generic Python input. So for this case, we ideally handle that keyword, I think, or otherwise at least error about not yet supporting it (as you did, but maybe can then be a NotImplementedError?)
##########
python/pyarrow/array.pxi:
##########
@@ -118,6 +118,13 @@ def _handle_arrow_array_protocol(obj, type, mask, size):
return res
+def _handle_run_end_encoded_arrays(obj, type):
+ from pyarrow.compute import run_end_encode
+ ree_arr = run_end_encode(obj)
Review Comment:
The kernel should take an option about which run_end type to use, so based on `type.run_end_type` you should be able to specify this up front (to avoid another conversion in the next step)
##########
python/pyarrow/array.pxi:
##########
@@ -336,11 +336,27 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
if pandas_api.have_pandas:
values, type = pandas_api.compat.get_datetimetz_type(
values, obj.dtype, type)
- result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
- pool)
+ if type and type.id == _Type_RUN_END_ENCODED:
+ if mask is not None:
+ raise ValueError("Cannot pass a mask for Run-End Encoded arrays.")
Review Comment:
If we keep the error, you can also move it into the helper function
##########
python/pyarrow/array.pxi:
##########
@@ -118,6 +118,13 @@ def _handle_arrow_array_protocol(obj, type, mask, size):
return res
+def _handle_run_end_encoded_arrays(obj, type):
+ from pyarrow.compute import run_end_encode
+ ree_arr = run_end_encode(obj)
+ return RunEndEncodedArray.from_arrays(
+ ree_arr.run_ends.to_pylist(), ree_arr.values.to_pylist(), type)
Review Comment:
Converting to a python list means we do the conversion "python object -> arrow data" twice. It doesn't work to just pass the pyarrow arrays?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1516630924
##########
python/pyarrow/array.pxi:
##########
@@ -118,11 +118,12 @@ def _handle_arrow_array_protocol(obj, type, mask, size):
return res
-def _handle_run_end_encoded_arrays(obj, type):
+cdef _handle_run_end_encoded_arrays(object sequence, object mask, object size,
+ DataType type, CMemoryPool* pool,
+ c_bool from_pandas):
from pyarrow.compute import run_end_encode
- ree_arr = run_end_encode(obj)
- return RunEndEncodedArray.from_arrays(
- ree_arr.run_ends.to_pylist(), ree_arr.values.to_pylist(), type)
+ arr = _sequence_to_array(sequence, mask, size, type.value_type, pool, from_pandas)
Review Comment:
That seems fine to me! Could we move the import to the beginning of the file or is there a benefit to keeping it within scope of this code?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1524478032
##########
python/pyarrow/array.pxi:
##########
@@ -336,11 +338,21 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
if pandas_api.have_pandas:
values, type = pandas_api.compat.get_datetimetz_type(
values, obj.dtype, type)
- result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
- pool)
+ if type and type.id == _Type_RUN_END_ENCODED:
+ arr = _ndarray_to_array(
+ values, mask, type.value_type, c_from_pandas, safe, pool)
+ result = run_end_encode(arr, run_end_type=type.run_end_type)
Review Comment:
You can pass the `pool` here as well
##########
python/pyarrow/array.pxi:
##########
@@ -21,6 +21,8 @@ import os
import warnings
from cython import sizeof
+from pyarrow.compute import run_end_encode
Review Comment:
Instead of this import here (which isn't guaranteed to work, as one can build pyarrow without compute support), you can use `_pc()` to get the compute module where calling `run_end_encode` (see the other examples of that in this file)
##########
python/pyarrow/tests/test_array.py:
##########
@@ -3579,6 +3579,42 @@ def test_run_end_encoded_from_buffers():
1, offset, children)
+def test_run_end_encoded_from_array_with_type():
+ run_ends = pa.array([1, 3, 6], type=pa.int32())
+ values = pa.array([1, 2, 3], type=pa.int64())
+
+ ree_type = pa.run_end_encoded(pa.int32(), pa.int64())
+
+ arr = [1, 2, 2, 3, 3, 3]
+ result = pa.array(arr, type=ree_type)
+
+ assert result.run_ends.equals(run_ends)
+ assert result.values.equals(values)
Review Comment:
Instead of each time checking both run ends and values, I think you could also once created the expected result (`expected = pa.RunEndEncodedArray.from_arrays(run_ends, values, ree_type)`), and then just check `assert result.equals(expected)`
##########
python/pyarrow/tests/test_array.py:
##########
@@ -3579,6 +3579,42 @@ def test_run_end_encoded_from_buffers():
1, offset, children)
+def test_run_end_encoded_from_array_with_type():
+ run_ends = pa.array([1, 3, 6], type=pa.int32())
+ values = pa.array([1, 2, 3], type=pa.int64())
+
+ ree_type = pa.run_end_encoded(pa.int32(), pa.int64())
+
+ arr = [1, 2, 2, 3, 3, 3]
+ result = pa.array(arr, type=ree_type)
+
+ assert result.run_ends.equals(run_ends)
+ assert result.values.equals(values)
+
+ result = pa.array(np.array(arr), type=ree_type)
Review Comment:
Can you also do one more version of this with `pa.array(arr)` as input?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1512277241
##########
python/pyarrow/array.pxi:
##########
@@ -339,8 +339,12 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
pool)
else:
+ if type and type.id == _Type_RUN_END_ENCODED:
+ from pyarrow.compute import run_end_encode
+ result = run_end_encode(obj)
Review Comment:
We should ensure that we actually honor the values type of the REE type. Eg if using `pa.run_end_encoded(run_end_type=pa.int32(), value_type=pa.float64())`, the result should use that exact type, and not whatever is inferred by `pc.run_end_encode`
##########
python/pyarrow/tests/test_array.py:
##########
@@ -3579,6 +3579,18 @@ def test_run_end_encoded_from_buffers():
1, offset, children)
+def test_run_end_encoded_from_array_with_type():
+ arr = [1, 2, 2, 3, 3, 3]
Review Comment:
This will also have to work for other kinds of inputs (eg np.ndarray instead of a python list)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1513999862
##########
python/pyarrow/array.pxi:
##########
@@ -336,11 +336,27 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
if pandas_api.have_pandas:
values, type = pandas_api.compat.get_datetimetz_type(
values, obj.dtype, type)
- result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
- pool)
+ if type and type.id == _Type_RUN_END_ENCODED:
+ if mask is not None:
+ raise ValueError("Cannot pass a mask for Run-End Encoded arrays.")
Review Comment:
I am not sure if checking for `mask` parameter makes sense as we would then need to check for other parameters also. Connected to: https://github.com/apache/arrow/issues/39394#issuecomment-1881112769
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "danepitkin (via GitHub)" <gi...@apache.org>.
danepitkin commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1513077190
##########
python/pyarrow/array.pxi:
##########
@@ -336,11 +336,27 @@ def array(object obj, type=None, mask=None, size=None, from_pandas=None,
if pandas_api.have_pandas:
values, type = pandas_api.compat.get_datetimetz_type(
values, obj.dtype, type)
- result = _ndarray_to_array(values, mask, type, c_from_pandas, safe,
- pool)
+ if type and type.id == _Type_RUN_END_ENCODED:
+ if mask is not None:
+ raise ValueError("Cannot pass a mask for Run-End Encoded arrays.")
+ from pyarrow.compute import run_end_encode
+ ree_arr = run_end_encode(obj)
+ result = RunEndEncodedArray.from_arrays(
+ ree_arr.run_ends.to_pylist(), ree_arr.values.to_pylist(), type)
Review Comment:
Is it worth moving this into its own function to avoid duplication?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1517447376
##########
python/pyarrow/array.pxi:
##########
@@ -118,11 +118,12 @@ def _handle_arrow_array_protocol(obj, type, mask, size):
return res
-def _handle_run_end_encoded_arrays(obj, type):
+cdef _handle_run_end_encoded_arrays(object sequence, object mask, object size,
+ DataType type, CMemoryPool* pool,
+ c_bool from_pandas):
from pyarrow.compute import run_end_encode
- ree_arr = run_end_encode(obj)
- return RunEndEncodedArray.from_arrays(
- ree_arr.run_ends.to_pylist(), ree_arr.values.to_pylist(), type)
+ arr = _sequence_to_array(sequence, mask, size, type.value_type, pool, from_pandas)
Review Comment:
No, no benefit, I think. WIll move the import to the beginning of the file.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-40273: [Python] Support construction of Run-End Encoded arrays in pa.array(..) [arrow]
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #40341:
URL: https://github.com/apache/arrow/pull/40341#discussion_r1526501142
##########
python/pyarrow/tests/test_array.py:
##########
@@ -3579,6 +3579,47 @@ def test_run_end_encoded_from_buffers():
1, offset, children)
+def test_run_end_encoded_from_array_with_type():
+ run_ends = [1, 3, 6]
+ values = [1, 2, 3]
+ ree_type = pa.run_end_encoded(pa.int32(), pa.int64())
+ expected = pa.RunEndEncodedArray.from_arrays(run_ends, values,
+ ree_type)
+
+ arr = [1, 2, 2, 3, 3, 3]
+ result = pa.array(arr)
+ assert result.equals(expected)
Review Comment:
```suggestion
```
This part is failing because to `type` is specified
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org