String dtype: use ObjectEngine for indexing for now correctness over performance
State: closed

A new `StringEngine` for indexing was added in https://github.com/pandas-dev/pandas/pull/56997, showing some performance improvements compared to the ObjectEngine. 
However, there are some issues with handling of missing values, see for example https://github.com/pandas-dev/pandas/issues/59879

The change in this PR switches back to object based engine, to for now have correct/desired behaviour, and we can see later if we can optimize this (but short term for 2.3/3.0 I would prioritize correct behaviour)



xref https://github.com/pandas-dev/pandas/issues/54792

Labels:
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Strings
description: String extension data type and string data

Comments:
FWIW I noticed the xfails in test_pivot.py are going to require this, as there are tests that working with missing values as column labels
> What's the reason for adding a new engine versus changing the existing StringEngine?

I was initially thinking to modify the StringEngine to be like the masked engine to properly handle missing values, but that turned out to be a bit more complicated and so to have something that _works_ correctly I thought to (for now) just fall back to the ObjectEngine (as we were using before for the string dtype as well). 

You can see in the first commit that's what I did, but then I realized that the ObjectEngine itself it not yet enough if we want to have compatibility to allow looking up missing values with None vs np.nan (the object engine is strict about that, but for back compat I would prefer that the `None` can still be used for the string dtype, because the constructor will now coerce None to NaN), and so that is why I then added a very small ObjectEngine subclass to handle the missing value in the scalar lookup case.


Thanks @jorisvandenbossche 
Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

```
git checkout 2.3.x
git pull
```

2. Cherry pick the first parent branch of the this PR on top of the older branch:
```
git cherry-pick -x -m1 98f7e4deeff26a5ef993ee27104387a1a6e0d3d3
```

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

```
git commit -am 'Backport PR #60329: String dtype: use ObjectEngine for indexing for now correctness over performance'
```

4. Push to a named branch:

```
git push YOURFORK 2.3.x:auto-backport-of-pr-60329-on-2.3.x
```

5. Create a PR against branch 2.3.x, I would have named this PR:

> "Backport PR #60329 on branch 2.3.x (String dtype: use ObjectEngine for indexing for now correctness over performance)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the `Still Needs Manual Backport` label once the PR gets merged.

If these instructions are inaccurate, feel free to [suggest an improvement](https://github.com/MeeseeksBox/MeeseeksDev).
                
Manual backport -> https://github.com/pandas-dev/pandas/pull/60453