BUG: Handle numpy strings in index names in HDF #13492
State: closed

Work in progress!
 - [ ] closes #13492
 - [ ] tests added / passed
 - [x] passes ``git diff upstream/master --name-only -- '*.py' | flake8 --diff``
 - [x] whatsnew entry


Labels:
name: IO HDF5
description: read_hdf, HDFStore
name: 32bit
description: 32-bit systems

Comments:
I found that pytables is returning a numpy.str_ as the index name for both indices. I don't think this has to do with how we write it, and pytables just chooses to write / read it that way. So we can handle this on the read side. There is some trickiness around numpy types in py2/3 and python string types in py2/3. So I decided to split the problem in two steps:

1) Handle converting numpy to python string types
2) Handle converting byte types to unicode codepoints.

The latter is done by _ensure_decoded, so for the former I added an _ensure_python_type. This way we should cover most situations.
The numpy type situation is complicated across py2/3 and string types, but luckily they have a function that handles all that: https://github.com/numpy/numpy/blob/v1.12.1/numpy/core/numerictypes.py#L74
as an additional test, try using a unicode index names as well :>
```
In [14]: s = np.str_('foo')

In [15]: type(s)
Out[15]: numpy.str_

In [16]: type(s.item())
Out[16]: str

In [17]: type(str(s))
Out[17]: str
```

I prefer [17] for the conversion I think.
@jreback so that was the original plan (and it works on my python 3.6 install) but the worry is that the behavior for what the heck happens with bytes vs strings (is it always going to return str_?) or py2/py3 (where in py2 np.str_ means bytes and in py3 np.str_ means unicode), and then how that interacts with .decode("utf-8") in the _ensure_decoded? If you're confident the above approach will work, more than happy to change.

Will add the test!
@makmanalp 

So all string data is stored as bytes in PyTables. I think we encode to utf-8 (and decode the same). Though this is really just for table format. I dont' really remember what happens for fixed format for meta data. You might want to trace it and see.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=h1) Report
> Merging [#16444](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/4ec98d8b10728af9695c7e35e04b7e3fd9c408d3?src=pr&el=desc) will **increase** coverage by `0.16%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/16444/graphs/tree.svg?width=650&height=150&src=pr&token=eZ4WkYLtcO)](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #16444      +/-   ##
==========================================
+ Coverage   90.75%   90.92%   +0.16%     
==========================================
  Files         161      161              
  Lines       51089    49244    -1845     
==========================================
- Hits        46368    44777    -1591     
+ Misses       4721     4467     -254
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.68% <80%> (+0.08%)` | :arrow_up: |
| #single | `40.24% <100%> (+0.07%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/io/pytables.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2lvL3B5dGFibGVzLnB5) | `93.12% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/computation/expressions.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tcHV0YXRpb24vZXhwcmVzc2lvbnMucHk=) | `0% <0%> (-90.48%)` | :arrow_down: |
| [pandas/io/parsers.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhcnNlcnMucHk=) | `95.42% <0%> (-0.27%)` | :arrow_down: |
| [pandas/io/msgpack/\_version.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2lvL21zZ3BhY2svX3ZlcnNpb24ucHk=) | `44.65% <0%> (-0.22%)` | :arrow_down: |
| [pandas/io/excel.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2lvL2V4Y2VsLnB5) | `80.55% <0%> (-0.09%)` | :arrow_down: |
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `96.65% <0%> (ø)` | :arrow_up: |
| [pandas/core/groupby.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS5weQ==) | `92.07% <0%> (+0.03%)` | :arrow_up: |
| [pandas/io/formats/style.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2lvL2Zvcm1hdHMvc3R5bGUucHk=) | `100% <0%> (+4.07%)` | :arrow_up: |
| [pandas/core/util/hashing.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvdXRpbC9oYXNoaW5nLnB5) | `98.34% <0%> (+5.37%)` | :arrow_up: |
| [pandas/io/json/json.py](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vanNvbi5weQ==) | `100% <0%> (+9.63%)` | :arrow_up: |
| ... and [1 more](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=footer). Last update [4ec98d8...70a55ec](https://codecov.io/gh/pandas-dev/pandas/pull/16444?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@makmanalp it seems like all the tests passed, so that's good.

Would you mind including the example from

```python
import pandas as pd
import numpy as np
import datetime

idx = pd.Index(pd.to_datetime([datetime.date(2000, 1, 1), datetime.date(2000, 1, 2)]), name='cols')
idx1 = pd.Index(pd.to_datetime([datetime.date(2010, 1, 1), datetime.date(2010, 1, 2)]), name='rows')
s = pd.DataFrame(np.arange(4).reshape(2,2), columns=idx, index=idx1)

with pd.HDFStore("test.h5", "w") as store:
    store.put("test", s, "fixed")

with pd.HDFStore("test.h5", "r") as store:
    s1 = store["test"]
```

and ensure that `type(s1.index.name)` is indeed a str. And maybe use a unicode name that isn't encodable as ASCII?
@jreback OK, if it's bytes only then that should simplify things a bit.
@TomAugspurger will do!
@makmanalp will you have a chance to update this in the next day or so? We're releasing 0.20.2 by the end of the week. Not a big deal to push.
@TomAugspurger sorry for dropping the ball on this. I'm going to give it a shot tomorrow during the day. The test and whatsnew is easy, but double checking how things work in fixed format is a bit harder for me. Feel free to push it off the release if so.
OK, oddly enough, in the test cases, I can't seem to use the ensure_clean_store() helper, perhaps there is something about the closing / opening of the file that changes things? I'm removing the bugfix that I made, and expecting the test to fail:

```python
    with ensure_clean_store(self.path) as store:
        store.put('frame', df, format='fixed')
        recons = store['frame']

        tm.assert_frame_equal(recons, df)

        assert type(df.index.name) == str
        assert type(df.columns.name) == str
        # Won't fail (surprising)

    with pd.HDFStore("test.h5", "w") as store:
        store.put("test", df, "fixed")

    with pd.HDFStore("test.h5", "r") as store:
        df2 = store["test"]
        assert type(df2.index.name) == str
        assert type(df2.columns.name) == str
        # Will fail! (which is what I want)
```

I thought of using `self._check_roundtrip` but then I don't have access to the dataframe to be able to assert the assertions about the name types. Any pointers on the right way to do this?
OK so I fixed the issue with ensure_clean_store because I found ensure_clean_path. Re: picking a unicode index name for the test, how are we going to make this work for py2/py3 again? Do we want a `u"blah"` string? Do we need a `# coding=utf-8` on top of the file?
Ah, OK, so I found the `compat` module and `u_safe` and `text_type`, will give a shot at those.
I think the `# coding` is only required if you use non-ascii characters in the source.
@TomAugspurger yeah - I've now realized we don't need to care about because I can use the u_safe thing helps because you can use the `\u05d0` style notation. Thank you!
Just for posterity, conversation on numpy strings / bytestrings / unicode in numpy gitter channel:

```

Mehmet Ali "Mali" Akmanalp @makmanalp May 22 19:12
Hi, does anyone know the difference between numpy.str and numpy.string (i.e. numpy.bytes_) and how to handle them in different python versions?
I'm trying to do a pull request for pandas where pytables is creating a np.str_ object that we need to decode into a regular ol' string

Julian Taylor @juliantaylor May 23 04:52
np.str_ is the equivalent to python str and maps to bytes in python2 and unicode in python3
np.string is always bytes and should not be used
the only real string type numpy has is np.unicode
that works in both python2 and 3

Mehmet Ali "Mali" Akmanalp @makmanalp May 23 09:27
@juliantaylor and how do conversions within those numpy types and/or from those to python types work?
(and thank you for the clarification)
My current understanding is that .item() does the right thing in terms of numpy->python, regardless of python version

Julian Taylor @juliantaylor May 23 11:31
@makmanalp no, item will return bytes in python3, which happens to be string in python2
the 'S' dtype is flat out broken, if you need actual bytes you are better of using int8 arrays, if you need strings you need to use unicode
the 'S' type has no concept of encoding, so even putting encoded data into it is troublesome as it strips trailing nulls

```
So I don't think the types converted / encoded / decoded at all for index names in fixed format, at least not within pandas:

https://github.com/pandas-dev/pandas/blob/fb47ee56d95ff1d5fcdff5dc589491c63a238bac/pandas/io/pytables.py#L2485

and also

https://github.com/pandas-dev/pandas/blob/fb47ee56d95ff1d5fcdff5dc589491c63a238bac/pandas/io/pytables.py#L2529

But I may not be understanding this code fully.

edit: It's also odd that calling type() on the on the index name is what's blowing up with an unicode error, and not printing it out or something: 
https://travis-ci.org/pandas-dev/pandas/jobs/238406379#L1371
Thanks a bunch @TomAugspurger! The latest push should handle all your comments correctly, and pass the 2.7 test this time!
(last push fixes a minor lint error that escaped me after the latest changes: `pandas/tests/io/test_pytables.py:2931:24: E128 continuation line under-indented for visual indent`)
@makmanalp can you update 
Got it, merging on green and will cherry pick for 0.20.2;
Thanks @makmanalp