Skip to content

BUG: Fixed grow_buffer to grow when capacity is reached #12504

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

Closed

Conversation

gfyoung
Copy link
Member

@gfyoung gfyoung commented Mar 1, 2016

Addresses issue in #12494 by allowing grow_buffer to grow the size of the parser buffer when buffer capacity is achieved. Previously, you had to exceed capacity for this to occur, but that was inconsistent with the end_field check later on when handling the EOF terminator, where reached capacity was considered a buffer overflow.

@gfyoung gfyoung force-pushed the read_csv_empty_header branch 2 times, most recently from 67aa639 to 3d1078e Compare March 2, 2016 01:11
@jreback jreback added Bug IO CSV read_csv, to_csv labels Mar 2, 2016
for count in range(1, 101):
try:
read_empty_header(count)
except pandas.parser.CParserError as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

import the exception (I think its actually imported already though)

Copy link
Contributor

Choose a reason for hiding this comment

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

scratch that. Why are you catching an exception here at all? These should always be empty frames. In fact you can construct the expected frame and compare

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough. Done.

@gfyoung gfyoung force-pushed the read_csv_empty_header branch 2 times, most recently from 499d9c5 to 2f8a608 Compare March 2, 2016 21:20
@gfyoung gfyoung force-pushed the read_csv_empty_header branch from 2f8a608 to 8ba3dd0 Compare March 3, 2016 13:30
@gfyoung
Copy link
Member Author

gfyoung commented Mar 3, 2016

Tests are passing. Should be good to merge, given how simple the fix was.

@jreback jreback added this to the 0.18.0 milestone Mar 3, 2016
@jreback jreback closed this in c69037c Mar 3, 2016
@jreback
Copy link
Contributor

jreback commented Mar 3, 2016

thank you sir!

@gfyoung
Copy link
Member Author

gfyoung commented Mar 3, 2016

Could you also close the associated issue? It doesn't seem to have been closed with the commit you made.

@gfyoung gfyoung deleted the read_csv_empty_header branch March 3, 2016 22:09
@jreback
Copy link
Contributor

jreback commented Mar 3, 2016

hmm, addresses #issue number should have worked (I usually put ``#closes in the top of the pr to have this done automagically)

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

Successfully merging this pull request may close these issues.

2 participants