Repository: pandas-dev/pandas
Pull request: BUG: fix HDFStore.append with all empty strings error (GH12242)
- [x] closes #12242
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
HDFStore.append fails when appending dataframe with empty string column for which min_itemsize < 8
Reproduced as follows:

```
In [3]: store = pd.HDFStore('teststore.h5', 'w')

In [4]: chunk = pd.DataFrame({'V1':['a','b','c','d','e'], 'data':np.arange(5)})

In [5]: store.append('df', chunk, min_itemsize={'V1': 4})

In [6]: chunk = pd.DataFrame({'V1':['', ''], 'data': [3, 5]})

In [7]: store.append('df', chunk, min_itemsize={'V1': 4})
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-7-c9bafa18ead0> in <module>()
----> 1 store.append('df', chunk, min_itemsize={'V1': 4})

/Users/amcpherson/Anaconda/lib/python2.7/site-packages/pandas/io/pytables.pyc in append(self, key, value, format, append, columns, dropna, **kwargs)
    905         kwargs = self._validate_format(format, kwargs)
    906         self._write_to_group(key, value, append=append, dropna=dropna,
--> 907                              **kwargs)
    908
    909     def append_to_multiple(self, d, value, selector, data_columns=None,

/Users/amcpherson/Anaconda/lib/python2.7/site-packages/pandas/io/pytables.pyc in _write_to_group(self, key, value, format, index, append, complib, encoding, **kwargs)
   1250
   1251         # write the object
-> 1252         s.write(obj=value, append=append, complib=complib, **kwargs)
   1253
   1254         if s.is_table and index:

/Users/amcpherson/Anaconda/lib/python2.7/site-packages/pandas/io/pytables.pyc in write(self, obj, axes, append, complib, complevel, fletcher32, min_itemsize, chunksize, expectedrows, dropna, **kwargs)
   3755         self.create_axes(axes=axes, obj=obj, validate=append,
   3756                          min_itemsize=min_itemsize,
-> 3757                          **kwargs)
   3758
   3759         for a in self.axes:

/Users/amcpherson/Anaconda/lib/python2.7/site-packages/pandas/io/pytables.pyc in create_axes(self, axes, obj, validate, nan_rep, data_columns, min_itemsize, **kwargs)
   3432                 self.values_axes.append(col)
   3433             except (NotImplementedError, ValueError, TypeError) as e:
-> 3434                 raise e
   3435             except Exception as detail:
   3436                 raise Exception(

ValueError: Trying to store a string with len [8] in [V1] column but
this column has a limit of [4]!
Consider using min_itemsize to preset the sizes on these columns

```

Does not raise unless all values in a column are empty strings.  A workaround is to set min_itemsize to 8 or higher.

Version information:

```
In [10]: pd.show_versions()

INSTALLED VERSIONS
------------------
commit: None
python: 2.7.11.final.0
python-bits: 64
OS: Darwin
OS-release: 14.5.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_CA.UTF-8

pandas: 0.17.1
nose: 1.3.7
pip: 8.0.2
setuptools: 19.6.2
Cython: 0.23.4
numpy: 1.10.2
scipy: 0.16.1
statsmodels: 0.6.1
IPython: 4.0.3
sphinx: 1.3.5
patsy: 0.4.0
dateutil: 2.4.2
pytz: 2015.7
blosc: None
bottleneck: 1.0.0
tables: 3.2.2
numexpr: 2.4.6
matplotlib: 1.5.1
openpyxl: 2.3.2
xlrd: 0.9.4
xlwt: 1.0.0
xlsxwriter: 0.8.4
lxml: 3.5.0
bs4: 4.4.1
html5lib: None
httplib2: None
apiclient: None
sqlalchemy: 1.0.11
pymysql: None
psycopg2: None
Jinja2: None
```

Hints from issue discussion:
So it looks like this comes from `np.asarray` doing something strange with length 0 fixed-width strings:

```
ipdb> data
array([[b'', b'']], dtype=object)
ipdb> n
> /Users/tom.augspurger/Envs/dev/lib/python3.4/site-packages/pandas/pandas/io/pytables.py(4453)_convert_string_array()
   4451
   4452     data = np.asarray(data, dtype="S%d" % itemsize)  # itemsize is 0
-> 4453     return data
   4454
   4455

ipdb> data
array([[b'', b'']],
      dtype='|S8')   # note the length 8
```

The `array` constructor converts to length 1 strings though:

``` python
In [12]: np.array([b'', b''], dtype='S0')
Out[12]:
array([b'', b''],
      dtype='|S1')   # length 1

```

Looks like a numpy bug maybe? Either way, we can work around by changing [this line](https://github.com/pydata/pandas/blob/6693a723aa2a8a53a071860a43804c173a7f92c6/pandas/io/pytables.py#L4450) to

``` python
        itemsize = max(1, lib.max_len_string_array(com._ensure_object(data.ravel())))
```

@amcpherson thanks for the report. Are you interested in submitting a Pull Request with that fix and a test?

i can do a pull request, cant guarantee how soon though
On Sat, Feb 6, 2016 at 8:02 AM Tom Augspurger notifications@github.com
wrote:

> So it looks like this comes from np.asarray doing something strange with
> length 0 fixed-width strings:
> 
> ipdb> data
> array([[b'', b'']], dtype=object)
> ipdb> n
> 
> > /Users/tom.augspurger/Envs/dev/lib/python3.4/site-packages/pandas/pandas/io/pytables.py(4453)_convert_string_array()
> >    4451
> >    4452     data = np.asarray(data, dtype="S%d" % itemsize)  # itemsize is 0
> > -> 4453     return data
> >    4454
> >    4455
> 
> ipdb> data
> array([[b'', b'']],
>       dtype='|S8')
> 
> The array constructor converts to length 1 strings though:
> 
> In [12]: np.array([b'', b''], dtype='S0')
> Out[12]:
> array([b'', b''],
>       dtype='|S1')
> 
> Looks like a numpy bug maybe? Either way, we can work around by changing this
> line
> https://github.com/pydata/pandas/blob/6693a723aa2a8a53a071860a43804c173a7f92c6/pandas/io/pytables.py#L4450
> to
> 
> ```
>     itemsize = max(1, lib.max_len_string_array(com._ensure_object(data.ravel())))
> ```
> 
> @amcpherson https://github.com/amcpherson thanks for the report. Are
> you interested in submitting a Pull Request with that fix and a test?
> 
> —
> Reply to this email directly or view it on GitHub
> https://github.com/pydata/pandas/issues/12242#issuecomment-180797773.

This still hasn't been factored in.
> This still hasn't been factored in.

hence the open issue

Commit message: BUG: fix HDFStore.append with all empty strings error (GH12242)

Patch to review:
diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt
index de111072bef02..6d5a88d09b0e1 100644
--- a/doc/source/whatsnew/v0.24.0.txt
+++ b/doc/source/whatsnew/v0.24.0.txt
@@ -1216,6 +1216,7 @@ Notice how we now instead output ``np.nan`` itself instead of a stringified form
 - :func:`read_sas()` will correctly parse sas7bdat files with data page types having also bit 7 set (so page type is 128 + 256 = 384) (:issue:`16615`)
 - Bug in :meth:`detect_client_encoding` where potential ``IOError`` goes unhandled when importing in a mod_wsgi process due to restricted access to stdout. (:issue:`21552`)
 - Bug in :func:`to_string()` that broke column alignment when ``index=False`` and width of first column's values is greater than the width of first column's header (:issue:`16839`, :issue:`13032`)
+- Bug in :meth:`HDFStore.append` when appending a :class:`DataFrame` with an empty string column and min_itemsize < 8 (:issue:`12242`)
 
 Plotting
 ^^^^^^^^
diff --git a/pandas/io/pytables.py b/pandas/io/pytables.py
index 56b63fddd96ad..41e14e482d061 100644
--- a/pandas/io/pytables.py
+++ b/pandas/io/pytables.py
@@ -4637,7 +4637,7 @@ def _convert_string_array(data, encoding, errors, itemsize=None):
     # create the sized dtype
     if itemsize is None:
         ensured = ensure_object(data.ravel())
-        itemsize = libwriters.max_len_string_array(ensured)
+        itemsize = max(1, libwriters.max_len_string_array(ensured))
 
     data = np.asarray(data, dtype="S%d" % itemsize)
     return data
diff --git a/pandas/tests/io/test_pytables.py b/pandas/tests/io/test_pytables.py
index 337eb74b3b51a..7717dc0d80358 100644
--- a/pandas/tests/io/test_pytables.py
+++ b/pandas/tests/io/test_pytables.py
@@ -1441,6 +1441,15 @@ def check_col(key, name, size):
                 result = store.select('df')
                 tm.assert_frame_equal(result, df)
 
+                # with all empty strings (GH 12242)
+                _maybe_remove(store, 'df')
+                df1 = DataFrame({'x': list('abcdef')})
+                df2 = DataFrame({'x': ['']})
+                store.append('df', df1, min_itemsize={'x': 1})
+                store.append('df', df2, min_itemsize={'x': 1})
+                tm.assert_frame_equal(store.select('df'),
+                                      pd.concat([df1, df2]))
+
         with ensure_clean_store(self.path) as store:
 
             def check_col(key, name, size):


Code review comments:
diff_hunk: @@ -1441,6 +1441,15 @@ def check_col(key, name, size):
                 result = store.select('df')
                 tm.assert_frame_equal(result, df)
 
+                # with all empty strings (GH 12242)
+                _maybe_remove(store, 'df')
+                df1 = DataFrame({'x': list('abcdef')})
+                df2 = DataFrame({'x': ['']})
+                store.append('df', df1, min_itemsize={'x': 1})
+                store.append('df', df2, min_itemsize={'x': 1})
+                tm.assert_frame_equal(store.select('df'),
+                                      pd.concat([df1, df2]))
path: pandas/tests/io/test_pytables.py
text: @user1:
Instead of comparing to `pd.concat`. Can you explicitly construct the `DataFrame`?

@user2:
can you push this to a new test, the existing ones are really long already