REF (string): de-duplicate ArrowStringArray methods
State: closed

- [ ] closes #xxxx (Replace xxxx with the GitHub issue number)
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.


Labels:
name: Refactor
description: Internal refactoring of code
name: Strings
description: String extension data type and string data
name: Arrow
description: pyarrow functionality
name: backported

Comments:
Could we do it the other way around? i.e. have the method in ArrowExtensionArray point to the one defined in ArrowStringArray? 
Personally, I feel it better belongs in string_arrow.py, to keep the string specific functionality in there (the ArrowExtensionArray definition is already huge ..)
> Could we do it the other way around? i.e. have the method in ArrowExtensionArray point to the one defined in ArrowStringArray?

I think the thing to do is put it in the ArrowStringArrayMixin.  I'm vaguely planning to move all the relevant stuff up there after the current crop of branches (some still local) are done.
If we're doing this moving of methods away from ArrowExtensionArray, using `pandas.ArrowDtype(pyarrow.string())` will continue to use NA semantics correct?
This REF is behavior-neutral

On Wed, Aug 21, 2024 at 10:38 AM Matthew Roeschke ***@***.***>
wrote:

> If we're doing this moving of methods away from ArrowExtensionArray, using
> pandas.ArrowDtype(pyarrow.string()) will continue to use NA semantics
> correct?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/59555#issuecomment-2302624046>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AB5UM6BAA2VILEGLUIJ6H33ZSTGA5AVCNFSM6AAAAABMYUEVH2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBSGYZDIMBUGY>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>

> I'm vaguely planning to move all the relevant stuff up there after the current crop of branches (some still local) are done.

If that's the plan, my preference would be do to it at once, instead of spread over multiple PRs. I know we generally want smaller more granular PRs, but given that this will need to be backported, doing this in one go instead of multiple steps will make that easier.

> I think the thing to do is put it in the ArrowStringArrayMixin.

Also a question on the mixin. What's the reason we need the mixin? (compared to keeping things in ArrowStringArray itself) 

Right now ArrowStringArray doesn't actually inherit from the mixin, but uses explicit `_str_method = ArrowStringArrayMixin._str_method` (well, that's actually only done in ArrowStringArrayNumpySemantics, I see, and not in ArrowStringArray, that's another question). 
So using that pattern, we could also keep things in ArrowStringArray, and add the methods to ArrowExtensionArray by doing `_str_method = ArrowStringArray._str_method` in that class. 

Or is the idea that ArrowStringArray(NumpySemantics) eventually actually inherits from ArrowStringArrayMixin?
> Also a question on the mixin. What's the reason we need the mixin? (compared to keeping things in ArrowStringArray itself)

It is not really needed.  ArrowEA subclasses the mixin and ArrowStringArray subclasses ArrowEA.  So all the methods could go directly on ArrowEA, its just that class is already really bulky.  Putting the methods directly on ArrowStringArray and reversing the inheritance would surely be possible but that would be more invasive.
Ah, yes, I forgot about ArrowStringArray subclassing ArrowExtensionArray and therefore also subclassing the mixin. 
But the problem with that inheritance is that currently it's not actually used in ArrowStringArray, right? Because ArrowStringArray also subclasses ObjectStringArrayMixin which will get chosen first as fall-back for a method that doesn't exist on ArrowStringArray

That means we should either add the methods explicitly with `_str_method = ArrowStringArrayMixin._str_method`, or let ArrowStringArray subclass ArrowStringArrayMixin directly with the proper order. However, I think the latter does not work? ("Cannot create a consistent method resolution order (MRO)")

> That means we should either add the methods explicitly with _str_method = ArrowStringArrayMixin._str_method

Yes, one of my local de-dup branches does this for the relevant methods.
If we deduplicate those methods, my preference would be to directly move those methods from ArrowEA to the mixin, and get them from there in ArrowStringArray. Or is there a reason we do not do that here, but use that pattern (shared methods in the mixin instead of in ArrowEA) for other methods?
> my preference would be to directly move those methods from ArrowEA to the mixin, and get them from there in ArrowStringArray.

Sure, will update.  Will wait until after #59615, #59568, #59562 and then rebase on top of those to minimize headaches.
There are a couple of failures after the latest update
Those failures are happening on other PRs as well, I think, so merging.