API: Add string extension type
State: closed

This adds a new extension type 'string' for storing string data.

The data model is essentially unchanged from master. String are still
stored in an object-dtype ndarray. Scalar elements are still Python
strs, and `np.nan` is still used as the string dtype.

Things are pretty well contained. The major changes outside the new array are

1. docs
2. `core/strings.py` to handle things correctly (mostly, returning `string` dtype when there's a `string` input.

No rush on reviewing this. Just parking it here for now.

Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Strings
description: String extension data type and string data
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Does the .str accessor only show up on string-dtype Series, or also object-coincidentally-all-string Series as in master?

Also on object-dtype as in master.

This PR doesn't change the current behavior at all (modulo new bugs).
May be a more suitable question for an issue, but `dtype=str` will still point to `object` in this PR and in the future(?) 
> dtype=str will still point to object in this PR and in the future(?)

It'll have to for now (in my first commit I allowed `dtype='str'` as well as `dtype='string'`, but that of course broke things.)

We'll want to think of a migration path for making `'str'` mean StringDtype, and probably we'll want  to infer `['a', 'b']` as string rather than object.
I was curious about how much the string validation (ruling out `pd.array(['a', 1], dtype="string")` cost.

relative:

![gh](https://user-images.githubusercontent.com/1312546/63194128-cc4de480-c034-11e9-801e-122aa99bd58d.png)

absolute:

![gh-2](https://user-images.githubusercontent.com/1312546/63194127-cc4de480-c034-11e9-93a7-350c904cca51.png)


code

<details>

```
from timeit import default_timer as tic
import pandas as pd
import numpy as np
import pandas.util.testing as tm

ns = [0, 10, 100, 1_000, 10_000, 100_000, 1_000_000]
times = []
for n in ns:
    data = tm.makeStringIndex(n).tolist()
    t0 = tic()
    np.array(data, dtype=object)
    t1 = tic()
    pd.core.arrays.StringArray._from_sequence(data)
    t2 = tic()
    
    times.append((n, t1 - t0, t2 - t1))

df = pd.DataFrame(times, columns=['n', 'numpy', 'pandas']).set_index('n')
df.div(df['numpy'], axis=0).plot(
    logx=True, title="Overhead Relative to NumPy"
);
```

</details>

I believe there are two sources of relative slowdown

1. per-value validation that the scalars are strings or NA
2. Ensuring that we have a consistent NA value (no mix of None and `np.nan`. Right now, this is done with an `isna()` (linear scan over the values) and a setitem if needed. I suspect that this could be rolled into the per-value scalar validation if desired, but I'm less concerned about perf right now.
(CI is passing now).
are you looking to get this merged quickly?  i.e. can this go on the "review later this week" pile or does it belong in the "later today" pile?
No. This can go in the "when I'm bored" pile. I don't think there's a huge
rush. It'd be nice for 1.0 but not essential.

On Mon, Aug 19, 2019 at 3:43 PM jbrockmendel <notifications@github.com>
wrote:

> are you looking to get this merged quickly? i.e. can this go on the
> "review later this week" pile or does it belong in the "later today" pile?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/27949?email_source=notifications&email_token=AAKAOIULOTZCKS2XW7YFRHDQFMAYPA5CNFSM4IMKKYTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4UHSDI#issuecomment-522746125>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIXGPYKMAKIMN55YAO3QFMAYPANCNFSM4IMKKYTA>
> .
>

> is the intention to have this have an encoding (maybe parameterized)? or is that a unicode type?

Mmm, this type is for in-memory strings, so I don't think an encoding is necessary. I don't think we're exposing the actual storage anywhere, which is when the encoding would matter.

If we had a ByteArray, then yes I think that should be parametrized by encoding (with UTF8 as the default).
Hello @TomAugspurger! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





























There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2019-10-04 14:03:39 UTC
For the sake of clarity this doesn't offer any real speed or memory savings over the object dtype right? 

Correct. The implementation is the same, aside from validation that the scalar elements are actually strings, rather than just object. So things are slightly slower.

In the future, we'll be able to transparently swap out the storage to use a native variable width string array to back the StringArray.
> In the future, we'll be able to transparently swap out the storage to use a native variable width string array to back the StringArray.

So just thinking through this should we just wait until that happens to implement anyway? Sounds like there aren't major advantages to this current implementation (save validation of contained elements) so I think just being conservative so that we don't lock ourselves into any design elements now that would need to be deprecated or changed for the real implementation
> > In the future, we'll be able to transparently swap out the storage to use a native variable width string array to back the StringArray.
> 
> So just thinking through this should we just wait until that happens to implement anyway? Sounds like there aren't major advantages to this current implementation (save validation of contained elements) so I think just being conservative so that we don't lock ourselves into any design elements now that would need to be deprecated or changed for the real implementation

well there are things we can do now

for example not constantly inferring the sting type


@TomAugspurger just for clarity, since people started reviewing this :): do you want to pursue this on the short term and should we start reviewing / discussing it in more detail?
I think it's ready for review.

My preference is to include it in 1.0 if we think it's ready. I think the
problem of overloading object-dtype to mean "usually strings, but really
anything" is problematic enough that it's worth fixing.

Though as Will notes, we should be aware of places where the
underlying data model leaks through to the user. Thus far, I think that's

1. The choice of NA (what do you get back when you do a scalar slice at a
position that's NA). That's being discussed in several places.
2. Uwe mentioned some potential performance issues with setting in an Arrow
array (will find the link).

On Wed, Sep 11, 2019 at 3:43 AM Joris Van den Bossche <
notifications@github.com> wrote:

> @TomAugspurger <https://github.com/TomAugspurger> just for clarity, since
> people started reviewing this :): do you want to pursue this on the short
> term and should we start reviewing / discussing it in more detail?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/27949?email_source=notifications&email_token=AAKAOIRULUULY7JSWLC5L2LQJCVT7A5CNFSM4IMKKYTKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6NXYPA#issuecomment-530283580>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAKAOIQFOQ2H2Y3GTZYWWM3QJCVT7ANCNFSM4IMKKYTA>
> .
>

Misc questions: if I have a td64 Series with a NaT and do `astype("string")` does the NaT become "NaT" or np.nan?  Does this astype round-trip correctly?
> Misc questions: if I have a td64 Series with a NaT and do astype("string") does the NaT become "NaT" or np.nan? Does this astype round-trip correctly?

```python
In [11]: s = pd.Series(pd.date_range('2000', periods=12)); s[0] = None

In [12]: s.astype("string").astype("datetime64[ns]")
Out[12]:
0           NaT
1    2000-01-02
2    2000-01-03
3    2000-01-04
4    2000-01-05
5    2000-01-06
6    2000-01-07
7    2000-01-08
8    2000-01-09
9    2000-01-10
10   2000-01-11
11   2000-01-12
dtype: datetime64[ns]

```

I'll add this as a test.
Have we ever considered not requiring the `.str` accessor for `StringArray` objects? It would seem more natural to me from a user perspective if `.upper()` for instance "just worked" on a `StringArray` and maybe the `.str` accessor becomes a convenience method for converting an object array to a `StringArrray` for legacy support

Not sure of effort
> Have we ever considered not requiring the .str accessor for StringArray objects?

Can you clarify this? Right now StringArray doesn't have the `.str` methods (we could add them with a bit of effort). And they could probably be added under a `StringArray.str` accessor or directly to the StringArray object.

Are you saying not requiring `.str` for these methods on `Series[string]` objects? So you can do `Series.upper()` when it's a string object?

If you're talking about `Series`, then no I don't think that's possible. There are conflicting methods like `Series.replace` and `Series.str.replace`.
> Are you saying not requiring `.str` for these methods on `Series[string]` objects? So you can do `Series.upper()` when it's a string object?

Yea that's what I was referring to. I get your point though too; have to think about more
It would make sense for the str accessor methods to exist directly on the StringArray, analogous to how the dt accessor methods exist on DTA/TDA/PA.
Was there a decision not to use NaS or is that tabled for now?
> Was there a decision not to use NaS or is that tabled for now?

No decision. I'll make sure the experimental warnings call this point out in particular.
What do people think the return type of `StringArray.__eq__(other)` should be?

* ndarray[bool]
* PandasArray[bool]

Using ndarray would make this closer to the current behavior.
> What do people think the return type of StringArray.__eq__(other) should be?

ndarray[bool] to match the behavior of all our other EAs.
Note that `PandasArray.__eq__` returns a `PandasArrray[bool]` (which probably makes sense).

And a hypothetical `BoolArray.__eq__` would of course return a BoolArray.

So the tightest type we can put on the return value for `ExtensionArray.__eq__(other)` is `ArrayLike`.
Thanks for the review @jbrockmendel. I believe everything has been addressed. In particular, the ops implementation has been updated quite a bit, if you have another chance to look.


I think the primary outstanding API decision is what to call this:

1. StringDtype / StringArray / `dtype='string'`
2. TextDtype / TextArray / `dtype='text'`

Do people (@jreback @jbrockmendel @jorisvandenbossche @WillAyd) have a preference? At the moment I'm leaning towards option 2 because of the cognitive clash of `dtype='str'` and `dtype='string'` being two different things.
Yea I think option 2 will minimize confusion
> I think the primary outstanding API decision is what to call this:
>
>    1. StringDtype / StringArray / dtype='string'
>    2. TextDtype / TextArray / dtype='text'

I have a preference for "string". 
It is the most natural choice IMO (we already have "string methods", it's what we would choose if we didn't have the back compat concerns, Arrow calls it string as well, ...). "text" on the other hand gives me the notion of "a longer body of text", not necessarily short strings.
I don't think it is more confusing as the int64/Int64 subtle difference (but ok, that's not a good reason ;-)). But moreover, I think we want to make this the default string dtype at some point, meaning that "str" will also become this new dtype. And then having "str"== "string" seems a lot less confusing than "str" == "text" (the second case would just give a completely different alias, while the first is kind of a natural abbreviation)