REF: Dispatch string methods to ExtensionArray
State: closed

This is a large refactor of our string methods. The goal is to have the `Series.str` accessor dispatch the actual compute down to the ExtensionArray. The motivation is to allow a Series backed by Arrow memory to use the Arrow string kernel (e.g. `Series.str.lower()` on an Arrow-backed StringArray should eventually call `pyarrow.compute.utf8_lower()`.)

To facilitate this, I made the following changes

1. Split `core/strings.py` into a sub package:
  - `core/strings/accessor.py`: Implements the `Series.str` accessor, which (mostly) just delegates to the EA and wraps
  - `core/strings/object_array.py`: Implements the string methods for object-type ndarray. 
  - `core/strings/categorical.py`, `core/strings/string_array.py`, implements categorical & StringArray-specific methods
2. Defines a new `ExtensionArray._str` extension point. This is where EAs get to take over and use their compute

Note that there are a few methods like `cat`, `extract`, and `extractall` that don't yet dispatch. I need to look into them a bit more, there implementation is a bit confusing.

Closes https://github.com/pandas-dev/pandas/issues/36216

Labels:
name: Strings
description: String extension data type and string data
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Making sure I understand: core.strings.accessor is mostly a cut/paste of the existing core.strings?  strings.object_array corresponds to the ndarray[object]-backed StringArray we use now?

Is strings.categorical for a Categorical backed by a StringArray or a StringArray backed by a Categorical?  Something else?

Is the accessor central to what you're doing here, or could in principle be separate?  I expect to have an opinion about this, but am not there yet.
> core.strings.accessor is mostly a cut/paste of the existing core.strings

Not quite. Anywhere you see things like 

```python
def method(self, ...):
    result = self._array._str.method(...)
    return self._wrap_result(result)
```

that `result = self._array._str.method` was previously something like `map(self._array, func)`, or something like `return str_method`.

The core methods `map_object` and `map_na` which were previously in `strings.py` were moved to `ObjectArrayMethods._map` and `StringArrayMethods._map`.

> Is strings.categorical for a Categorical backed by a StringArray or a StringArray backed by a Categorical? Something else?

It's for a supporting `Series[Categorical].str` and `CategoricalIndex.str`. Which I think is just a Categorical containing strings (object-dtype index for the categories).

> Is the accessor central to what you're doing here, or could in principle be separate?

I assume you mean the `_str` accessor on ExtensionArrays, since that's what's new? It's central in the sense that it and inheritance is the mechanism I've used to dispatch. We could do it other ways.
> I assume you mean the _str accessor on ExtensionArrays, since that's what's new? It's central in the sense that it and inheritance is the mechanism I've used to dispatch. We could do it other ways.

In the past we've been very conservative about adding things to EA.  I'd find this easier to think about if that could be considered orthogonally.
> In the past we've been very conservative about adding things to EA. I'd find this easier to think about if that could be considered orthogonally.

Just to clarify, I wouldn't consider this isn't part of the official EA interface (I'm not adding it to the docs, for example). If someone comes along with another EA for storing string arrays and want access to the pandas' `.str` namespace, then I'm happy to discuss adding it to the interface. Right now, we just have the 3-4ish internal users in pandas (object-dtype, string dtype, categorical, and soon ArrowStringArray).

We just need some way to translate `Series.str.<method>` to "call the right <method> for a Series backed by this extension array". If you feel strongly about not adding this private attribute to `StringArray` and `Categorical`, then we can make a proxy similar to the `ObjectProxy` defined in `object_array.py`. It's just an unnecessary layer for the other cases, since we already have an extension array.
So I think our two options are either

1. Attach an accessor to string EAs

```python
class BaseStringArrayMethods(abc.ABC):
     def __init__(self, array: ExtensionArray):
        self._array = array

class ObjectProxy(ExtensionArray):
   _str = CachedAccessor("str", ObjectArrayMethods)
```

2. A mixin, where we prefex each method with some string (to disambiguate from regular methods of the same name).

```python
class StringMethodsMixin:
    def _str_isnumber(self):
        ...

    def _str_repeat(self, n):
        ...

class StringArray(ExtensionArray, StringMethodsMixin):
    ...
```

Sounds like people are roughly prefer the second version?
Not easily, since it's more of a split than a move. Right now the `pandas/core/strings.py` contains

1. The Series.str accessor (the class StringMethods and its function signatures)
2. The *implementations* of these methods on arrays (the bodies of StringMethod's methods, all the top-level `str_<method>` methods, `na_map`, `map_object`, `map_na`.

This PR moves the `StringMethods` accessor to `pandas/core/strings/accessor.py` and the implementations to `pandas/core/strings/{base,object_array,string_array,categorical}.py`
> Updated to use a mixin (`ObjectStringArrayMixin`) that implements the pandas string methods on object-dtype ndarrays. EAs implementing the string methods (StringArray, Categorical, and PandasArray for object-dtype strings) inherit from that.

are there concrete mixin's for each of these (even they are empty). I think the structure is not very clear at all.

Please somewhere can you put a diagram of the inheriteance pattern. someone who sees this will be totally confused by the indirection.
> Please somewhere can you put a diagram of the inheriteance pattern

Added a note to `pandas/core/strings/__init__.py` for the overall layout. The tldr is `Series.str.<method>` calls `Series.array._str_<method>`.

The inheritence is

```python
# BaseStringArrayMethods  # the base ABC
#  - ObjectStringArrayMixin  # most of the implementation
#     - StringArray
#     - PandasArray
#     - Categorical
```


> > Please somewhere can you put a diagram of the inheriteance pattern
> 
> Added a note to `pandas/core/strings/__init__.py` for the overall layout. The tldr is `Series.str.<method>` calls `Series.array._str_<method>`.
> 
> The inheritence is
> 
> ```python
> # BaseStringArrayMethods  # the base ABC
> #  - ObjectStringArrayMixin  # most of the implementation
> #     - StringArray
> #     - PandasArray
> #     - Categorical
> ```

great.

let me re-look again, but conceptually looked good.
@ivanovmg your suggestions to change some of the implementations are fine, but probably not appropriate for this PR. Right now I'm trying to limit the changes to what's necessary for allowing EAs to controls the implementation, so not changing any of the implementations. Those would be good as followup work.
@TomAugspurger needs a merge of master

Otherwise remaining comments?
Fixed the merge conflicts. This should be good on my end.
> can pandas/core/strings.py be deleted?

Yeah, apparently it was restored while I was fixing merge conflicts.
Just the ARM timeout. Anything else here?
thanks @TomAugspurger really nice. sorry for the long review time :->
Thanks, no worries. This wasn't especially easy to review since it was a
large diff that didn't just move stuff around.

On Wed, Sep 30, 2020 at 7:22 AM Jeff Reback <notifications@github.com>
wrote:

> thanks @TomAugspurger <https://github.com/TomAugspurger> really nice.
> sorry for the long review time :->
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/36357#issuecomment-701355322>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAKAOIUPLTPQTZCQFFTSW5LSIMPKFANCNFSM4RLS5ONA>
> .
>

FYI, I used this PR in `fletcher` https://github.com/xhochy/fletcher/pull/196 and it works quite nicely. Will now use that to add string algorithms that are now in Arrow bit-by-bit.
@xhochy Cool! If fletcher is using it then we should probably add it to the public API. Probably just importing it in `pandas.api.extensions`.
Unless we only want fletcher to use it for now .. (which still keeps it easier to change the implementation, given the good ties with the fletcher developers?)
I'm going to use it a bit more and see if I run into troubles while overloading it with the Arrow methods. If there are issues, we can easier to breaking changes as long as it is a private API.