Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Value Error when doing HDFStore.Select of contiguous mixed-data #17670

Merged
merged 1 commit into from
Sep 28, 2017
Merged

Fixed Value Error when doing HDFStore.Select of contiguous mixed-data #17670

merged 1 commit into from
Sep 28, 2017

Conversation

amolkahat
Copy link
Contributor

Fixed Value Error when doing HDFStore.Select of contiguous mixed-data table ft. VLArray

Closes 17021

Signed-off-by: Amol Kahat akahat@redhat.com

@jreback jreback added Bug IO HDF5 read_hdf, HDFStore labels Sep 25, 2017
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks pretty good.

@@ -580,6 +580,7 @@ I/O
- Bug in :func:`read_html` where import check fails when run in multiple threads (:issue:`16928`)
- Bug in :func:`read_csv` where automatic delimiter detection caused a ``TypeError`` to be thrown when a bad line was encountered rather than the correct error message (:issue:`13374`)
- Bug in ``DataFrame.to_html()`` with ``notebook=True`` where DataFrames with named indices or non-MultiIndex indices had undesired horizontal or vertical alignment for column or row labels, respectively (:issue:`16792`)
- Bug in :func:`HDFStore.select` ValueError when doing HDFStore.Select of contiguous mixed-data table ft. VLArray (:issue:`17021`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have an extra ValueError here.

Bug in :func:`HDFStore.selct` when reading a contiguous mixed-data .....

@@ -5678,3 +5678,14 @@ def test_dst_transitions(self):
store.append('df', df)
result = store.select('df')
assert_frame_equal(result, df)

def test_contiguous_mixed_data_table(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also add a case that uses start & stop that are not the len

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this near the other selecting tests

@pep8speaks
Copy link

pep8speaks commented Sep 26, 2017

Hello @amolkahat! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on September 27, 2017 at 12:09 Hours UTC

'b': Series(['ab', 'cd', 'ab'])})
df.to_hdf('test.h5', 'test_dataset')

with pd.HDFStore('test.h5') as fd:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls use the existing style, IOW ensure_clean_store

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good as from some style comments.

@codecov
Copy link

codecov bot commented Sep 27, 2017

Codecov Report

Merging #17670 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17670      +/-   ##
==========================================
+ Coverage   91.24%   91.25%   +<.01%     
==========================================
  Files         163      163              
  Lines       49762    49761       -1     
==========================================
+ Hits        45406    45408       +2     
+ Misses       4356     4353       -3
Flag Coverage Δ
#multiple 89.04% <100%> (+0.02%) ⬆️
#single 40.33% <100%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/pytables.py 92.79% <100%> (-0.01%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.73% <0%> (-0.1%) ⬇️
pandas/plotting/_converter.py 65.2% <0%> (+1.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3be81a...2ed2da9. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 27, 2017

Codecov Report

Merging #17670 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17670      +/-   ##
==========================================
+ Coverage   91.24%   91.25%   +<.01%     
==========================================
  Files         163      163              
  Lines       49762    49761       -1     
==========================================
+ Hits        45406    45408       +2     
+ Misses       4356     4353       -3
Flag Coverage Δ
#multiple 89.04% <100%> (+0.02%) ⬆️
#single 40.33% <100%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/pytables.py 92.79% <100%> (-0.01%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.73% <0%> (-0.1%) ⬇️
pandas/plotting/_converter.py 65.2% <0%> (+1.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3be81a...2ed2da9. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 27, 2017

Codecov Report

Merging #17670 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17670      +/-   ##
==========================================
+ Coverage   91.24%   91.25%   +<.01%     
==========================================
  Files         163      163              
  Lines       49762    49761       -1     
==========================================
+ Hits        45406    45408       +2     
+ Misses       4356     4353       -3
Flag Coverage Δ
#multiple 89.04% <100%> (+0.02%) ⬆️
#single 40.34% <100%> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/io/pytables.py 92.79% <100%> (-0.01%) ⬇️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.73% <0%> (-0.1%) ⬇️
pandas/plotting/_converter.py 65.2% <0%> (+1.81%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 44747c8...4861f63. Read the comment docs.

@@ -580,6 +580,7 @@ I/O
- Bug in :func:`read_html` where import check fails when run in multiple threads (:issue:`16928`)
- Bug in :func:`read_csv` where automatic delimiter detection caused a ``TypeError`` to be thrown when a bad line was encountered rather than the correct error message (:issue:`13374`)
- Bug in ``DataFrame.to_html()`` with ``notebook=True`` where DataFrames with named indices or non-MultiIndex indices had undesired horizontal or vertical alignment for column or row labels, respectively (:issue:`16792`)
- Bug in :func:`HDFStore.select` when reading a contiguous mixed-data table ft. VLArray (:issue:`17021`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you expand ft.

@@ -4387,6 +4387,24 @@ def test_path_pathlib(self):
lambda p: pd.read_hdf(p, 'df'))
tm.assert_frame_equal(df, result)

def test_contiguous_mixed_data_table(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can parametrize this for start/stop, also include start=None, stop=None

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments, ping on green.

@jreback jreback added this to the 0.21.0 milestone Sep 27, 2017
table featuring VLArray

Closes 17021

Signed-off-by: Amol Kahat <akahat@redhat.com>
@jreback jreback merged commit 074b485 into pandas-dev:master Sep 28, 2017
@jreback
Copy link
Contributor

jreback commented Sep 28, 2017

thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug IO HDF5 read_hdf, HDFStore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: ValueError when doing HDFStore.Select of contiguous mixed-data table ft. VLArray
3 participants