REF: Dispatch string methods to ExtensionArray #36357

TomAugspurger · 2020-09-14T13:17:27Z

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

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

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 #36216

…y-ops

…ethods

jbrockmendel · 2020-09-14T16:49:56Z

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.

TomAugspurger · 2020-09-14T19:21:56Z

core.strings.accessor is mostly a cut/paste of the existing core.strings

Not quite. Anywhere you see things like

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.

…ethods

pandas/core/strings/object_array.py

jbrockmendel · 2020-09-17T19:02:12Z

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.

TomAugspurger · 2020-09-17T19:33:02Z

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 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.

jorisvandenbossche

Looks good to me! This will nicely allow to start overriding a subset of the methods with arrow implementations where available for the ArrowStringArray.

I suppose one possible alternative to the "proxy" class holding the methods that is added as _str on the ExtensionArrays would be to add those methods directly on the EA. That's one layer of indirection less, but on the other hand also adds a whole bunch of methods to those classes of course (but it's similar to eg DatetimeArray holding the .dt methods/attributes, and the Methods classes could be turned into a mixin)

pandas/core/strings/base.py

pandas/core/strings/object_array.py

pandas/core/strings/__init__.py

TomAugspurger · 2020-09-18T18:40:40Z

So I think our two options are either

Attach an accessor to string EAs

class BaseStringArrayMethods(abc.ABC):
     def __init__(self, array: ExtensionArray):
        self._array = array

class ObjectProxy(ExtensionArray):
   _str = CachedAccessor("str", ObjectArrayMethods)

A mixin, where we prefex each method with some string (to disambiguate from regular methods of the same name).

class StringMethodsMixin:
    def _str_isnumber(self):
        ...

    def _str_repeat(self, n):
        ...

class StringArray(ExtensionArray, StringMethodsMixin):
    ...

Sounds like people are roughly prefer the second version?

jreback

can you do a precursor PR which just moves things. I have no idea what really changed here.

TomAugspurger · 2020-09-21T11:29:22Z

Not easily, since it's more of a split than a move. Right now the pandas/core/strings.py contains

The Series.str accessor (the class StringMethods and its function signatures)
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

…ethods

pandas/core/strings/object_array.py

pandas/core/strings/base.py

TomAugspurger · 2020-09-25T02:55:27Z

@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.

jorisvandenbossche · 2020-09-25T13:11:25Z

pandas/core/arrays/categorical.py

@@ -2316,6 +2316,9 @@ def replace(self, to_replace, value, inplace: bool = False):
    # ------------------------------------------------------------------------
    # String methods interface
    def _str_map(self, f, na_value=np.nan, dtype=np.dtype(object)):
+        # Optimization to apply the callable `f` to the categories once
+        # and rebuild the result by `take`ing from the result with the codes.
+        # Returns the same type as the object-dtype impelmentation though.


Suggested change

# Returns the same type as the object-dtype impelmentation though.

# Returns the same type as the object-dtype implementation though.

jorisvandenbossche · 2020-09-25T13:12:04Z

pandas/core/strings/__init__.py

+Most methods on the StringMethods accessor follow the pattern:
+
+    1. extract the array from the series (or index)
+    2. Call that array's impelmentation of the string method


Suggested change

2. Call that array's impelmentation of the string method

2. Call that array's implementation of the string method

…ethods

jorisvandenbossche · 2020-09-28T18:35:12Z

@TomAugspurger needs a merge of master

Otherwise remaining comments?

…ethods

TomAugspurger · 2020-09-29T11:51:29Z

Fixed the merge conflicts. This should be good on my end.

simonjayhawkins

can pandas/core/strings.py be deleted?

simonjayhawkins · 2020-09-29T12:47:08Z

pandas/core/arrays/string_.py

+            na_value = self.dtype.na_value
+
+        mask = isna(self)
+        arr = self


is this needed?

TomAugspurger · 2020-09-29T13:20:24Z

can pandas/core/strings.py be deleted?

Yeah, apparently it was restored while I was fixing merge conflicts.

pandas/core/arrays/categorical.py

pandas/core/arrays/string_.py

jbrockmendel · 2020-09-29T20:15:10Z

pandas/core/strings/base.py

+        flags: int = 0,
+        na: Scalar = np.nan,
+    ):
+        pass


could # pragma: no cover all of these

…ethods

TomAugspurger · 2020-09-30T11:12:48Z

Just the ARM timeout. Anything else here?

jreback

lgtm. some comments to create issues for followon

jreback · 2020-09-30T12:16:12Z

pandas/core/strings/__init__.py

+these are prefixed with `_str_`. So ``Series.str.upper()`` calls
+``Series.array._str_upper()``. The interface isn't currently public
+to other string extension arrays.
+"""


much more clear, thanks

jreback · 2020-09-30T12:17:49Z

pandas/core/strings/accessor.py

+        return None
+
+
+def _str_extract_noexpand(arr, pat, flags=0):


ok fair enough, can you create an issue to separate these other methods (maybe with check boxes) & align witht he strutcure)

jreback · 2020-09-30T12:21:18Z

pandas/tests/test_strings.py

@@ -728,10 +728,6 @@ def test_count(self):
            ["foo", "foofoo", np.nan, "foooofooofommmfoo"], dtype=np.object_
        )

-        result = strings.str_count(values, "f[o]+")


can you also create an issue to either make pandas/tests/strings/test_object.py or add a fixture to support th emultiple implementations (maybe better)

jreback · 2020-09-30T12:22:43Z

thanks @TomAugspurger really nice. sorry for the long review time :->

TomAugspurger · 2020-09-30T13:15:12Z

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 ***@***.***> 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 <#36357 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKAOIUPLTPQTZCQFFTSW5LSIMPKFANCNFSM4RLS5ONA> .

xhochy · 2020-10-15T10:15:11Z

FYI, I used this PR in fletcher xhochy/fletcher#196 and it works quite nicely. Will now use that to add string algorithms that are now in Arrow bit-by-bit.

TomAugspurger · 2020-10-15T11:18:39Z

@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.

jorisvandenbossche · 2020-10-15T11:21:50Z

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?)

xhochy · 2020-10-15T13:44:05Z

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.

xhochy and others added 7 commits September 11, 2020 09:48

Implement BaseDtypeTests for ArrowStringDtype

9e90d4e

Refactor to use parametrized StringDtype

92f1d26

wip

00096f0

Merge remote-tracking branch 'upstream/master' into arrow-string-arra…

5a89dbf

…y-ops

annoyed

89f8e6a

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

3f82225

…ethods

wip

fabc01e

TomAugspurger added ExtensionArray Extending pandas with custom dtypes or arrays. Strings String extension data type and string data labels Sep 14, 2020

remove old

a4d4ad5

TomAugspurger added 6 commits September 14, 2020 15:21

fixup

e76a3c1

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

49dff8a

…ethods

fixup

75831b3

doctest

1cf54cc

docstrings

fc81ebe

typing

6be1af6

TomAugspurger commented Sep 17, 2020

View reviewed changes

pandas/core/strings/object_array.py Show resolved Hide resolved

typing

95b3310

jorisvandenbossche reviewed Sep 18, 2020

View reviewed changes

pandas/core/strings/base.py Outdated Show resolved Hide resolved

pandas/core/strings/object_array.py Outdated Show resolved Hide resolved

pandas/core/strings/object_array.py Outdated Show resolved Hide resolved

pandas/core/strings/__init__.py Show resolved Hide resolved

jreback requested changes Sep 18, 2020

View reviewed changes

TomAugspurger added 4 commits September 21, 2020 06:35

wip

20a8705

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

136831a

…ethods

wip

38c1611

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

ea27e57

…ethods

ivanovmg reviewed Sep 24, 2020

View reviewed changes

pandas/core/strings/object_array.py Show resolved Hide resolved

pandas/core/strings/object_array.py Show resolved Hide resolved

pandas/core/strings/object_array.py Show resolved Hide resolved

ivanovmg reviewed Sep 24, 2020

View reviewed changes

pandas/core/strings/base.py Show resolved Hide resolved

jorisvandenbossche approved these changes Sep 25, 2020

View reviewed changes

TomAugspurger added 2 commits September 25, 2020 15:37

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

35a97ab

…ethods

fixup

d681f99

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

cc5ceed

…ethods

simonjayhawkins reviewed Sep 29, 2020

View reviewed changes

pandas/core/arrays/string_.py Outdated

na_value = self.dtype.na_value

mask = isna(self)

arr = self

Copy link

Member

simonjayhawkins Sep 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this needed?

fixup

457c112

jbrockmendel reviewed Sep 29, 2020

View reviewed changes

pandas/core/arrays/categorical.py Show resolved Hide resolved

jbrockmendel reviewed Sep 29, 2020

View reviewed changes

pandas/core/arrays/string_.py Outdated Show resolved Hide resolved

jbrockmendel reviewed Sep 29, 2020

View reviewed changes

pandas/core/strings/base.py

flags: int = 0,

na: Scalar = np.nan,

):

pass

Copy link

Member

jbrockmendel Sep 29, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could # pragma: no cover all of these

TomAugspurger added 2 commits September 29, 2020 15:44

Merge remote-tracking branch 'upstream/master' into dispatch-string-m…

58e1bb9

…ethods

simplify inheritance

cb2fb24

jreback approved these changes Sep 30, 2020

View reviewed changes

jreback added this to the 1.2 milestone Sep 30, 2020

jreback merged commit fbd13a9 into pandas-dev:master Sep 30, 2020

TomAugspurger deleted the dispatch-string-methods branch October 14, 2020 14:42

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

REF: Dispatch string methods to ExtensionArray (pandas-dev#36357)

f661a2f

asishm mentioned this pull request Jan 7, 2021

BUG: Different results from DataFrame.apply and str accessor #38979

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REF: Dispatch string methods to ExtensionArray #36357

REF: Dispatch string methods to ExtensionArray #36357

TomAugspurger commented Sep 14, 2020 •

edited

Loading

jbrockmendel commented Sep 14, 2020

TomAugspurger commented Sep 14, 2020

jbrockmendel commented Sep 17, 2020

TomAugspurger commented Sep 17, 2020 •

edited

Loading

jorisvandenbossche left a comment

TomAugspurger commented Sep 18, 2020

jreback left a comment

TomAugspurger commented Sep 21, 2020

TomAugspurger commented Sep 25, 2020

jorisvandenbossche Sep 25, 2020

jorisvandenbossche Sep 25, 2020

jorisvandenbossche commented Sep 28, 2020

TomAugspurger commented Sep 29, 2020

simonjayhawkins left a comment

simonjayhawkins Sep 29, 2020

TomAugspurger commented Sep 29, 2020

jbrockmendel Sep 29, 2020

TomAugspurger commented Sep 30, 2020

jreback left a comment

jreback Sep 30, 2020

jreback Sep 30, 2020

jreback Sep 30, 2020

jreback commented Sep 30, 2020

TomAugspurger commented Sep 30, 2020 via email

xhochy commented Oct 15, 2020

TomAugspurger commented Oct 15, 2020

jorisvandenbossche commented Oct 15, 2020

xhochy commented Oct 15, 2020

	# Returns the same type as the object-dtype impelmentation though.
	# Returns the same type as the object-dtype implementation though.

	2. Call that array's impelmentation of the string method
	2. Call that array's implementation of the string method

REF: Dispatch string methods to ExtensionArray #36357

REF: Dispatch string methods to ExtensionArray #36357

Conversation

TomAugspurger commented Sep 14, 2020 • edited Loading

jbrockmendel commented Sep 14, 2020

TomAugspurger commented Sep 14, 2020

jbrockmendel commented Sep 17, 2020

TomAugspurger commented Sep 17, 2020 • edited Loading

jorisvandenbossche left a comment

Choose a reason for hiding this comment

TomAugspurger commented Sep 18, 2020

jreback left a comment

Choose a reason for hiding this comment

TomAugspurger commented Sep 21, 2020

TomAugspurger commented Sep 25, 2020

jorisvandenbossche Sep 25, 2020

Choose a reason for hiding this comment

jorisvandenbossche Sep 25, 2020

Choose a reason for hiding this comment

jorisvandenbossche commented Sep 28, 2020

TomAugspurger commented Sep 29, 2020

simonjayhawkins left a comment

Choose a reason for hiding this comment

simonjayhawkins Sep 29, 2020

Choose a reason for hiding this comment

TomAugspurger commented Sep 29, 2020

jbrockmendel Sep 29, 2020

Choose a reason for hiding this comment

TomAugspurger commented Sep 30, 2020

jreback left a comment

Choose a reason for hiding this comment

jreback Sep 30, 2020

Choose a reason for hiding this comment

jreback Sep 30, 2020

Choose a reason for hiding this comment

jreback Sep 30, 2020

Choose a reason for hiding this comment

jreback commented Sep 30, 2020

TomAugspurger commented Sep 30, 2020 via email

xhochy commented Oct 15, 2020

TomAugspurger commented Oct 15, 2020

jorisvandenbossche commented Oct 15, 2020

xhochy commented Oct 15, 2020

TomAugspurger commented Sep 14, 2020 •

edited

Loading

TomAugspurger commented Sep 17, 2020 •

edited

Loading