GH-50072:[Python] Add tests for replace_with_mask kernel#50102
GH-50072:[Python] Add tests for replace_with_mask kernel#50102AnkitAhlawat7742 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Python-level unit tests for the pyarrow.compute.replace_with_mask kernel to cover typical usage patterns that previously lacked coverage, as requested in GH-50072.
Changes:
- Added parameterized tests covering basic replacement behavior across several scalar/array types (ints, strings, floats) including nulls in values and mask.
- Added coverage for
ChunkedArrayinputs. - Added negative tests for invalid mask length and incorrect replacement counts.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
github-actions
Bot
added
awaiting merge
AlenkaF
left a comment
AlenkaF
left a comment
There was a problem hiding this comment.
Thanks for opening up a follow-up PR, much appreciated!
The tests cases are good. I added one nit comment and one connected to the heavy use of parametrization in this PR. I think in all the tests the code is actually less clear due to the bulk of it leaving in the parametrized part. I will not block the PR due to this but feel the tests could be clearer. That would help future maintenance and debugging in the situation when the tests fail.
| assert result.to_pylist() == [None] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("arr,mask,replacements,expected", [ |
There was a problem hiding this comment.
| @pytest.mark.parametrize("arr,mask,replacements,expected", [ | |
| @pytest.mark.parametrize("arr, mask, replacements, expected", [ |
Same for others.
| # Mask length does not match input array length | ||
| (pa.array([1, 2, 3]), pa.array([True, False]), pa.array([10]), None), | ||
| ]) | ||
| def test_replace_with_mask_errors(arr, mask, replacements, error_match): |
There was a problem hiding this comment.
I would remove the if branch and the whole parametrization here and do a clean test where array construction and pytest.raises check comes one after the other.
Fix : #50072
Rationale for this change
The
replace_with_maskcompute function in PyArrow currently has no Python-level tests for regular use cases.This PR adds test cases for standard usage of
replace_with_maskto ensure the function works correctly.What changes are included in this PR?
Added parameterized test cases in
python/pyarrow/tests/test_compute.pyAre these changes tested?
Yes, Manually
Are there any user-facing changes?
No user-facing changes.