BUG: ArrowStringArray.__setitem__ breaks views

[jbrockmendel]

arr = pd.array(['foo', 'bar'], dtype='string[pyarrow]')
arr2 = arr[:]

arr2[0] = 'baz'

>>> arr
<ArrowStringArray>
['foo', 'bar']
Length: 2, dtype: string

Looks like in ArrowStringArray.__setitem__ we set self._data = ...

I expect this is a consquence of Arrow arrays (just the string arrays?) being immutable and there's likely not much we can do about it.

This seems likely to cause a long tail of headaches, might be better to raise for immutable arrays (like we do with SparseArray)

[jorisvandenbossche]

I think we recently had some related discussion about this? (maybe related to the shares_memory testing) But indeed, pyarrow currently doesn't provide APIs to mutate arrays, so currently there is nothing to do about this.

This seems likely to cause a long tail of headaches, might be better to raise for immutable arrays

IMO we shouldn't do that. We can easily provide mutability on the Series/DataFrame level for the user, and pyarrow also provides some APIs to efficiently do parts of what setitem does (eg for changing values with a mask, there is pyarrow.compute.replace_with_mask)

[jbrockmendel]

I think we recently had some related discussion about this? (maybe related to the shares_memory testing)

Yah, i think ArrowStringArray.copy() was involved.

We can easily provide mutability on the Series/DataFrame level for the user, and pyarrow also provides some APIs to efficiently do parts of what setitem does (eg for changing values with a mask, there is pyarrow.compute.replace_with_mask)

This I'll leave to you.

[mroeschke]

So would the options here would be:

1. Document that ArrowExtentionArray doesn't support __setitem__ with views
2. Raise a NotImplementedError if the user tries to do this?

[jbrockmendel]

I dont know enough about how arrow arrays work. are they all immutable or just the string ones?

we have precedent for raising TypeError on SparseArray.__setitem__ (though there are some cases there im hoping to make work...)

Could make them immutable now (to avoid silent incorrect behavior) and then if/when CoW is implemented then revisit bc Nothing Is A View.

[mroeschke]

Seems like all Arrow objects are immutable: https://arrow.apache.org/docs/cpp/arrays.html#available-strategies

As Arrow objects are immutable...

I agree avoiding silent incorrect behavior for the view case would be good, but detecting the view case I'm guessing is impractical.

Is there an internal case where we want to rely on __setitem__ with views working as intended? My gut feeling is that users might want this to Just Work i.e. document that this always return new data and views are not supported

[jbrockmendel]

I agree avoiding silent incorrect behavior for the view case would be good, but detecting the view case I'm guessing is impractical.

Agreed we shouldn't be spending any effort trying to track/detect views.

Is there an internal case where we want to rely on __setitem__ with views working as intended?

The relevant behavior is so ubiquitous no one would bother to document # _this_ slice needs to be a view. i.e. that's an empirical question that we can't feasibly answer.

My gut feeling is that users might want this to Just Work

I think that ship has sailed since we have trouble defining what Just Work means. Presumably the arrow authors made the arrays immutable for a reason, and the hypothetical user is using a pyarrow dtype for a reason.

So I vote that immutable things are immutable. If pyarrow wants to implement an API for __setitem__ that'll be great.

[jreback]

well we could actually implement this by copying with the new values

but o suppose that should be explicit opt in