BUG: fix categories in HDFStore not filtering correctly (#13322)

[shawnheide]

• closes categories in HDFStore don't filter correctly #13322
• tests passed
• passes git diff upstream/master | flake8 --diff
• whatsnew entry

Fixed bug in pytables.py where series with categories were not being filtered properly.

[jreback]

always start with the tests

[shawnheide]

Sorry Jeff, I'm not sure what you mean. Are you saying I should add a unit test for this?

[jreback]

ANY change requires a test.

[jreback]

at a minimum, tests that reproduce the original issue. You FIRST right the tests, make sure they replicate the failure, THEN write code to fix. You have fixed it when your tests then pass and nothing else breaks.

[shawnheide]

Thanks for filling me in Jeff. Sorry for having to spell it out for me, but this is the first non-documentation PR I've submitted for pandas. I couldn't find any tests related to pytables but it seems like tests/frame/test_misc_api might be a good spot. Any insight would be appreciated. Thanks.

This is the code I wrote to test the fix before I submitted the pull:

obsids = ['ESP_012345_6789', 'ESP_987654_3210']
imgids = ['APF00006np', 'APF0001imm']
data = [4.3, 9.8]

df = pd.DataFrame(dict(obsids=obsids, imgids=imgids, data=data))
df.to_hdf('testdf_no_cats.hdf', 'df',format='t', data_columns=True)

df.obsids = df.obsids.astype('category')
df.imgids = df.imgids.astype('category')
df.to_hdf('testdf_with_cats.hdf', 'df',format='t', data_columns=True)

# df without categories
df2 = pd.read_hdf('testdf_no_cats.hdf', 'df', where='obsids=B')
assert(len(df2) == 0)

# df with categories
df3 = pd.read_hdf('testdf_with_cats.hdf', 'df', where='obsids=B')
assert(len(df3) == 0)

[jreback]

https://github.com/pydata/pandas/blob/master/pandas/io/tests/test_pytables.py

[shawnheide]

Thanks Jeff, I completely missed that, I incorrectly assumed everything was in the one tests folder. Just submitted a new commit after confirming the test passed with my changes.

[codecov-io]

Current coverage is 85.23% (diff: 100%)

Merging #13792 into master will increase coverage by <.01%

@@             master     #13792   diff @@
==========================================
  Files           140        140          
  Lines         50420      50422     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          42975      42977     +2   
  Misses         7445       7445          
  Partials          0          0          

Powered by Codecov. Last update cc216ad...a461208

[jreback]

move this right after test_categorical, and rename to test_categorical_conversion

[jreback]

pls add a whatsnew in bug fix section.

[jreback]

I would do this like:

result = metadata.searchsorted(v, side='left')
if not result and v not in metadata:
    result = -1

then the 'in' is only done if the result is not found (as opposed to always)

[shawnheide]

@jreback Thanks for all of the feedback. I really appreciate you showing me how to do things the right way. I made the changes you requested and rebased the commit.

[jreback]

be more specific; this only doesn't work when the query has NO values that are found in the categorical.

[jreback]

thanks @shawnheide