Clean up kwargs #230
Closed
Clean up kwargs #230
Conversation
This file contains 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
piskvorky
reviewed
Sep 15, 2018
| # | ||
| # - open complains if you pass non-null errors in binary mode | ||
| # - our default mode is rb (incompatible with default Python open) | ||
| # - our default errors is non-null (to match default Python open) |
There was a problem hiding this comment.
What does non-null mean? Is null == None?
| # necessary because: | ||
| # | ||
| # - open complains if you pass non-null errors in binary mode | ||
| # - our default mode is rb (incompatible with default Python open) |
There was a problem hiding this comment.
How about we change this to be compatible?
Or at least make the mode mandatory, so people have to make a conscious decision between binary/text.
There was a problem hiding this comment.
Yes, we will do this in the next major release.
There was a problem hiding this comment.
BTW, we cannot make the mode mandatory, as that breaks compatibility with the built-in open.
We will switch to reading text mode as default in the next release.
We have to read in binary mode, otherwise the built-in open will not be called under Py2
|
Hi @mpenkov, how is going here? Where you plan to finish this (internal changes + warnings)? |
|
I'll have a look on the weekend. Thanks for reminding me. |
This was referenced Oct 11, 2018
|
I figured out a better way to do this. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We used to liberally pass arbitrary keyword arguments (e.g. **kwargs). This is a bad practice because it's difficult to keep track of function parameters and ensure they're being passed down to the right function.
This PR starts to clean up our use of arbitrary keyword arguments.