BUG: invalid column names in a HDF5 table format #9057
State: closed

Have DataFrame.to_hdf() raise an error when using pytables with non-string
column types

closes #9057


Labels:
name: Bug
name: IO HDF5
description: read_hdf, HDFStore

Comments:
I'm definitely open to moving this logic to the most appropriate place, and will rely on your judgement as to where that is. As far as I can tell, though, the code path through `to_hdf()` doesn't ever get to `validate_col()` prior to the error that we're trying to prevent/mask with this patch.

The [calls to validate](https://github.com/pydata/pandas/blob/master/pandas/io/pytables.py#L3767) happen after [table_creation](https://github.com/pydata/pandas/blob/master/pandas/io/pytables.py#L3767) (i.e. where the [original error](https://github.com/pydata/pandas/issues/9057) comes from).

For example:

```
james@basalt ~/code/git/pandas/pandas $ git --no-pager diff | tail
     if append:
         f = lambda store: store.append(key, value, **kwargs)
@@ -1569,6 +1569,7 @@ class IndexCol(StringMixin):

     def validate_col(self, itemsize=None):
         """ validate this column: return the compared against itemsize """
+        raise Exception("We'll never make it here")

         # validate this column for string truncation (or reset to the max size)
         if _ensure_decoded(self.kind) == u('string'):
```

```
(testenv)james@basalt ~/code/git/pandas $ nosetests -v pandas/io/tests/test_pytables.py:TestHDFStore.test_to_hdf_with_integer_column_names
test_to_hdf_with_integer_column_names (pandas.io.tests.test_pytables.TestHDFStore) ... ERROR

======================================================================
ERROR: test_to_hdf_with_integer_column_names (pandas.io.tests.test_pytables.TestHDFStore)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/james/code/git/pandas/pandas/io/tests/test_pytables.py", line 4625, in test_to_hdf_with_integer_column_names
    self.assertRaises(ValueError, df.to_hdf, path, 'df', format='table', data_columns=True)
  File "/home/james/code/git/pandas/pandas/util/testing.py", line 1576, in assertRaises
    _callable(*args, **kwargs)
  File "/home/james/code/git/pandas/pandas/core/generic.py", line 918, in to_hdf
    return pytables.to_hdf(path_or_buf, key, self, **kwargs)
  File "/home/james/code/git/pandas/pandas/io/pytables.py", line 292, in to_hdf
    f(store)
  File "/home/james/code/git/pandas/pandas/io/pytables.py", line 287, in <lambda>
    f = lambda store: store.put(key, value, **kwargs)
  File "/home/james/code/git/pandas/pandas/io/pytables.py", line 835, in put
    self._write_to_group(key, value, append=append, **kwargs)
  File "/home/james/code/git/pandas/pandas/io/pytables.py", line 1284, in _write_to_group
    s.write(obj=value, append=append, complib=complib, **kwargs)
  File "/home/james/code/git/pandas/pandas/io/pytables.py", line 3792, in write
    table = self._handle.create_table(self.group, **options)
  File "/home/james/miniconda3/envs/testenv/lib/python3.4/site-packages/tables/file.py", line 1060, in create_table
    chunkshape=chunkshape, byteorder=byteorder)
  File "/home/james/miniconda3/envs/testenv/lib/python3.4/site-packages/tables/table.py", line 811, in __init__
    self.description = Description(description)
  File "/home/james/miniconda3/envs/testenv/lib/python3.4/site-packages/tables/description.py", line 466, in __init__
    if name.startswith('_v_'):
AttributeError: 'numpy.int64' object has no attribute 'startswith'

----------------------------------------------------------------------
Ran 1 test in 0.010s

FAILED (errors=1)
```

Should there be a separate validation call somewhere higher in [write()](https://github.com/pydata/pandas/blob/master/pandas/io/pytables.py#L3743)? Or am I missing something obvious here?

Try something like this

```
diff --git a/pandas/io/pytables.py b/pandas/io/pytables.py
index 458a245..aea9336 100644
--- a/pandas/io/pytables.py
+++ b/pandas/io/pytables.py
@@ -1535,6 +1535,12 @@ class IndexCol(StringMixin):
                 self.typ = _tables(
                 ).StringCol(itemsize=min_itemsize, pos=self.pos)

+    def validate(self, handler, append, **kwargs):
+        self.validate_names()
+
+    def validate_names(self):
+        pass
+
     def validate_and_set(self, handler, append, **kwargs):
         self.set_table(handler.table)
         self.validate_col()
@@ -2080,6 +2086,10 @@ class DataIndexableCol(DataCol):
     """ represent a data column that can be indexed """
     is_data_indexable = True

+    def validate_names(self):
+        if not Index(self.values).is_object():
+            raise ValueError("cannot have a non-object label DataIndexableCol")
+
     def get_atom_string(self, block, itemsize):
         return _tables().StringCol(itemsize=itemsize)

@@ -3752,6 +3762,9 @@ class AppendableTable(LegacyTable):
                          min_itemsize=min_itemsize,
                          **kwargs)

+        for a in self.axes:
+            a.validate(self, append)
+
         if not self.is_exists:
```

Hmm, yeah, that works. Can I ask, what's the expected behaviour when a user tries to use unicode column name? Is [this test](https://github.com/jameshiebert/pandas/commit/bde903fc6e2fa87928d77ea10809b79a50e3946e#diff-b021c9b0cc5f9f0cdde791c4d933b1d9R4627) actually testing for the right behaviour or is it irrelevant?

@jameshiebert 
- can you comprehensively test for the this a bit more (e.g. make datetimes the actual names of the columns for example), I think we can _only_ allow string-likes.
- I think your unicode test is good. best to prob go thru all of the index types and make sure that ONLY the string-like ones work.

e.g. 

```
for i in [tm.makeIntegerIndex(),tm.makeUnicodeIndex()......]:
    # test that these raise
```

All that said, I `unicode` _should_ work in PY3! (but not PY2). 

What we are actually asking here is if a non-ascii encoded string will work.

@jreback Thanks for the suggestions. I used your `pytables.py` code straight-up, but as for the test, I _think_ that what I pushed covers all of the index types. If I missed anything, please let me know.

pls add a release note in v0.17.0.txt

can you rebase and squash, contributing docs are [here](http://pandas.pydata.org/pandas-docs/stable/contributing.html) if you need help

@jreback It should be good to go.

ok, why don't you rebase; ping when green.

@jreback This PR should be good to go once again.

thanks!