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

[jameshiebert]

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

closes #9057

[jreback]

This is not the right place for this. Should go here: https://github.com/pydata/pandas/blob/master/pandas/io/pytables.py#L1546

Just add on logic to validate. This is an IndexCol at this point, so should be straightforward.

[jameshiebert]

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 happen after table_creation (i.e. where the original error 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()? Or am I missing something obvious here?

[jreback]

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:

[jameshiebert]

Hmm, yeah, that works. Can I ask, what's the expected behaviour when a user tries to use unicode column name? Is this test actually testing for the right behaviour or is it irrelevant?

[jreback]

@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.

[jameshiebert]

@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.

[jreback]

for the ones that should run, I would then select out say the first element and make sure you are getting a non-zero length frame, e.g.

result = pd.read_hdf(path,'df',where="index = [{0}]".format(df.index[0]))

[jreback]

can you update to my comments?

[jameshiebert]

Sorry, I'm not following. Do you want me address this comment above? I thought that I did so two lines down...

[jreback]

ahh, didn't see that

[jreback]

pls add a release note in v0.17.0.txt

[jreback]

can you rebase and squash, contributing docs are here if you need help

[jameshiebert]

@jreback It should be good to go.

[jreback]

we are now going to release 0.16.2, so can you remove from here (it no longer exists) and put there?

[jreback]

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

[jameshiebert]

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

[jreback]

thanks!