ENH(string dtype): fallback for HDF5 with UTF-8 surrogates
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.

One oddity I encountered here: only the index is currently encoded / decoded on write / read operations respectively. Columns on the other hand are written and read as strings. I haven't looked into why this is, and if we can avoid encode/decode for index.

It seemed best to only fallback when `errors="surrogatepass"`, though that use is a bit odd since there is no actual decode occurring. If there are other approaches (perhaps always falling back?), I'm certainly open to them.

Another option here is to fallback to `object` instead of `StringDtype(storage="python")`, but it seems with `infer_string=True` the latter is more appropriate.

Labels:
name: IO HDF5
description: read_hdf, HDFStore
name: Strings
description: String extension data type and string data

Comments:
Looks good to me!

> Another option here is to fallback to `object` instead of `StringDtype(storage="python")`, but it seems with `infer_string=True` the latter is more appropriate.

I think there is also something to say for using `object` dtype here, to make it clearer to the user that something is going wrong and that it is not actually valid (UTF-8) strings. 
But no strong opinion.

> to make it clearer to the user that something is going wrong and that it is not actually valid (UTF-8) strings.

Would raising a warning be a better flag to the user?
Thinking about this more, I would prefer to not issue a warning. The user is already passing `surrogatepass`, they are aware of surrogate issues. We should not be noisy.
@jorisvandenbossche - friendly ping.
@jorisvandenbossche - friendly ping.
@jorisvandenbossche @mroeschke - I think this is ready for merging.
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 a393c31931af4fc69ea0006fd851eeb00175be3c
```

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

```
git commit -am 'Backport PR #60993: ENH(string dtype): fallback for HDF5 with UTF-8 surrogates'
```

4. Push to a named branch:

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

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

> "Backport PR #60993 on branch 2.3.x (ENH(string dtype): fallback for HDF5 with UTF-8 surrogates)"

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).
                
Backport -> https://github.com/pandas-dev/pandas/pull/61639