BUG: Fix segfault in csv tokenizer #32566
Conversation
There was a problem hiding this comment.
Great thanks for the PR @roberthdevries . We don't get a ton of contributions to the C extensions so much appreciate what you are doing here
| columns=list("ab"), | ||
| ) | ||
| csv = "\nheader\n\na,b\n\n\n1,2\n\n3,4" | ||
| for nrows in range(1, 6): |
There was a problem hiding this comment.
Instead of doing this can you just parametrize on the values that matter?
There was a problem hiding this comment.
Can you elaborate which values you would like to see parameterized?
I basically took the example in the ticket to reproduce the issue and checked that for all sensible values for nrows I get the expected outcome.
The reason for this is that this covers more cases that could cause errors like the one in the original ticket due to various edge cases.
There was a problem hiding this comment.
@roberthdevries : @WillAyd is asking that you use pytest.mark.parameterize over the nrows parameter.
| @@ -341,6 +341,17 @@ def test_empty_csv_input(self): | |||
| df = read_csv(StringIO(), chunksize=20, header=None, names=["a", "b", "c"]) | |||
| assert isinstance(df, TextFileReader) | |||
|
|
|||
| def test_blank_lines_between_header_and_data_rows(self): | |||
There was a problem hiding this comment.
this test needs to be in test_common.py and use the all_parsers fixture so it testsed.
datapythonista
changed the title
roberthdevries
force-pushed
the
fix-28071-read-csv-empty-lines-segfault
branch
from
a9e6294 to
5007c28
Compare
|
Hello @roberthdevries! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-03-15 07:19:35 UTC |
roberthdevries
force-pushed
the
fix-28071-read-csv-empty-lines-segfault
branch
from
1b4cc74 to
07a136f
Compare
|
@roberthdevries pls merge master; ping on green. |
This looks like the most sensible value as the number of words that is deleted is 0 therefore the char_count of characters to be skipped is also 0.
roberthdevries
force-pushed
the
fix-28071-read-csv-empty-lines-segfault
branch
from
7ba3e24 to
c6b64c7
Compare
|
ping @jreback everything is green |
|
thanks @roberthdevries |
read_csv) with blank lines between header and data rows quits Python interpreter #28071black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff