Nishuuzz opened a new pull request, #50890:
URL: https://github.com/apache/arrow/pull/50890
### Rationale for this change
Fixes #35650, which has been open since May 2023. Passing an `Array` or
`ChunkedArray` as the fill value to `fill_null` blows up with an
`AttributeError` whenever its type doesn't match the values:
```python
>>> s1 = pa.array(["ab", None], pa.string())
>>> s2 = pa.array([bytearray([97, 98]), None], type=pa.binary(2))
>>> pa.compute.fill_null(s1, s2)
AttributeError: 'pyarrow.lib.FixedSizeBinaryArray' object has no attribute
'as_py'
```
It isn't specific to fixed size binary. Any mismatch does it, including the
plainest possible one:
```python
>>> arr = pa.array([1, 2, None], type=pa.int64())
>>> arr.fill_null(pa.array([10, 20, 30], type=pa.int32()))
AttributeError: 'pyarrow.lib.Int32Array' object has no attribute 'as_py'
```
and it comes through `Array.fill_null` and `ChunkedArray.fill_null` too,
since both route here.
The cause is this line:
```python
elif values.type != fill_value.type:
fill_value = pa.scalar(fill_value.as_py(), type=values.type)
```
`as_py()` only exists on `Scalar`. The branch is reached by arrays and
chunked arrays as well, which the parameter is documented to accept —
"fill_value : Array, ChunkedArray, or Scalar-like object. If not same type as
values, will attempt to cast." So the documented cast never happens for the
array cases; they just raise.
### What changes are included in this PR?
Cast the fill value instead of routing it through Python objects. `cast`
exists on `Scalar`, `Array` and `ChunkedArray` alike, so all three go down the
same path and the documented behaviour actually happens.
I did consider just dropping the branch and letting `coalesce` reconcile the
types itself, which it is perfectly capable of. I didn't, because it reconciles
them to a *common* type rather than to the values' type, so `fill_null` on a
string array with a binary fill value would hand back a binary array, and
filling an int64 array from a float64 one would turn the untouched int64 values
into floats. Casting the fill value keeps the result the type of `values`,
which is what the docstring promises and, I think, what anyone calling
`fill_null` expects.
For scalar fill values this is almost entirely the same operation as before
— I compared the old and new paths across int/float/null/string/timestamp
combinations and they agree, including which ones raise. The one difference is
that a case like an int32 scalar into a string array now casts to `'5'` where
it used to raise `ArrowTypeError`. That is a cast the docstring invites, and it
only widens what's accepted, but it is a behaviour change so I'd rather flag it
than leave you to find it.
### Are these changes tested?
Yes, `test_fill_null_array_different_type` covers a mismatched `Array` fill
value, a mismatched `ChunkedArray` one, the exact reproducer from #35650, and
the case that must still fail — a float64 fill value into an int64 array, which
raises rather than quietly promoting the result.
It fails on main with the `AttributeError` above and passes with the change.
I don't have a local C++ build, so I ran the updated `test_compute.py`
against an installed pyarrow 25.0.0 with the same one-line change applied.
Comparing the two runs, exactly one test moved and it was this one — 21
failures before, 20 after, and no test that passed before fails now. The
remaining failures and errors are identical in both runs and unrelated to this:
the `test_round_temporal_duration` group and three pickle fixture errors that
come from my environment.
### Are there any user-facing changes?
Yes. `fill_null` with a differently-typed `Array` or `ChunkedArray` fill
value now works instead of raising `AttributeError`, and the scalar case above
accepts one more conversion than it used to.
#35624 looks like the same root cause. I've left it alone rather than claim
it here, because the case in that report already works on current main and I
couldn't reproduce it as written.
--
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]
