-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REF/CLN: pandas/io/parsers.py #36852
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
0a87ba4
REF: ext method _refresh_kwargs_based_on_dialect
ivanovmg 63e625d
REF: extract method _validate_skipfooter
ivanovmg 7ac3b6d
REF: drop local variable engine_specified
ivanovmg 8dc74fd
CLN: clean FutureWarning issue
ivanovmg 06fcf4c
CLN: make validation methods as static
ivanovmg da7a9b2
Merge branch 'master' into refactor/parsers
ivanovmg 4c024da
TYP: annotate methods and move to module level
ivanovmg f10984f
DOC: add docstrings
ivanovmg 19f6533
CLN: read dialect name directly from its instance
ivanovmg c8e0bc9
REF: reverse args in _merge_with_dialect_properties
ivanovmg fc1b627
REF: _check_defaults_read -> _refine_defaults_read
ivanovmg 6e7c0d0
Merge branch 'master' into refactor/parsers
ivanovmg ba4a389
REF: drop getting __name__ attr of dialect
ivanovmg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
reverse these (always put kwds things last).
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.
rename kwds -> defaults
so similar to the _check_defaults_read
also not averse to renaming _check_defaults_read to something better.
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.
I have done it.
Also, I found the issue with the ValueError message, when dialect is incorrect.
Originally,
kwds['dialect']
, but I decided to usedialect.__name__
for better readability, which is not accepted by mypy (throws error"Dialect" has no attribute "__name__" [attr-defined]
).So, now I use string representation of dialect instance, however I can still pass one extra parameter (dialect_name) to
_merge_with_dialect_properties
and use it in the error message.