-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
ERR: Fail-fast with incompatible skipfooter combos #23711
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
ERR: Fail-fast with incompatible skipfooter combos #23711
Conversation
Hello @gfyoung! Thanks for submitting the PR.
|
Codecov Report
@@ Coverage Diff @@
## master #23711 +/- ##
==========================================
+ Coverage 92.25% 92.25% +<.01%
==========================================
Files 161 161
Lines 51383 51385 +2
==========================================
+ Hits 47404 47406 +2
Misses 3979 3979
Continue to review full report at Codecov.
|
lgtm. i guess this warrant a whatsnew as well, bug fix section is ok |
* Don't create the iterator and error immediately if the skipfooter parameter is passed in. * Raise the correct error message when nrows is passed in with skipfooter.
94351db
to
39ab28a
Compare
@jreback : Added the |
thanks @gfyoung |
* ERR: Fail-fast with incompatible skipfooter combos * Don't create the iterator and error immediately if the skipfooter parameter is passed in. * Raise the correct error message when nrows is passed in with skipfooter. * Fix doc lint errors
* ERR: Fail-fast with incompatible skipfooter combos * Don't create the iterator and error immediately if the skipfooter parameter is passed in. * Raise the correct error message when nrows is passed in with skipfooter. * Fix doc lint errors
* ERR: Fail-fast with incompatible skipfooter combos * Don't create the iterator and error immediately if the skipfooter parameter is passed in. * Raise the correct error message when nrows is passed in with skipfooter. * Fix doc lint errors
Setup:
Currently, we get:
In Case 1, the error message is not correct. True, passing in
nrows
andskipfooter
is not supported (skipfooter
should be skipping lines at the end of the file, NOT at the end of thenrows
of data that we read in, which is what happens currently), but we should raise a better error message.(BTW, the only way to make this combo work is that we read in the entire file before cutting off the last
skipfooter
rows but can look into that subsequently...)In Case 2, creating this reader is deceiving. Any attempt to call
.read()
on this will raise theValueError
seen in Case 1. It is better that we alert end-users immediately that this reader doesn't work. After this PR, we get some more useful error messages out of the gate: