-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
BUG: numpy.loadtxt reads only 50000 lines when skip_rows >= max_rows #28319
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
Conversation
…iable skiplines as skiprows; added test in numpy/lib/tests/test_loadtxt.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I think we could just put it in as is also. I noted a few nits, some of which would be nice to address.
I wonder if we should flag this as a larger fix in the release notes (i guess as a "change" see doc/release/upcoming_changes/README.txt
), but we don't really have a habit around it, so maybe not.
(Mainly because it previously did silently return wrong things, and who knows if someone even worked around that in some weird way...)
Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Thank you for pointing out, I applied all the suggested changes!
Unfortunately I don't know that much about these policies, I have no clue on that... Personal curiosity: why does style matter? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consistent style helps readability, you can argue its a nit sometimes, but many nits make for a less nice experience reading the code. (And sure, maybe these nits don't bother you, but it does bother some.)
Let's not worry about a release note. I don't think there was any way to work around this bug (besides just not using skip_rows
).
Will squash-merge later after tests passed.
Just for the record, macOS tests ran into the matmul warning, the TSAN test just hung (freebsd is a general failure). |
Indeed I see... I thought there was some technical reason besides that... |
…umpy#28319) * fixed bug in function _read in numpy/lib/_npyio_impl.py, misnamed variable skiplines as skiprows; added test in numpy/lib/tests/test_loadtxt.py * fixed sintax in test_loadtxt.py * changed use of mkstemp with use of tmpdir provided by pytest * fixed bug in use of tmpdir in loadtxt test * Update numpy/lib/tests/test_loadtxt.py Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net> * Update file numpy/lib/tests/test_loadtxt.py * Update file numpy/lib/tests/test_loadtxt.py * Update numpy/lib/tests/test_loadtxt.py --------- Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>
Fixed wrong behavior of loadtxt when reading files in chunks.
Misnamed variable (skiprows instead of skiplines) in function _read in numpy/lib/npyio_impl.py caused the function to skip lines at every chunk read.
Closes #28315