bpo-33461: Emit DeprecationWarning when json.loads(encoding=...) is used#6762
bpo-33461: Emit DeprecationWarning when json.loads(encoding=...) is used#6762methane merged 5 commits intopython:masterpython/cpython:masterfrom Carreau:json-loads-deprecatedCarreau/cpython:json-loads-deprecatedCopy head branch name to clipboard
Conversation
f269cd0 to
273b316
Compare
273b316 to
505d680
Compare
|
Would you mind restarting the travis build ? The errors seem to be unrelated from the code changes ? Thanks. |
|
The docs check part of the CI build:
did fail with an error: It looks like should be |
|
I suggest to just remove this parameter. Seems it should be removed from |
| import warnings | ||
| warnings.warn( | ||
| "passing 'encoding' as a keyword argument is deprecated since" | ||
| " Python 3.1 and will be ignored.", DeprecationWarning, |
There was a problem hiding this comment.
| " Python 3.1 and will be ignored.", DeprecationWarning, | |
| " Python 3.1 and will be removed in Python 3.9.", DeprecationWarning, |
There was a problem hiding this comment.
I've done both, as "is ignored" is a current fact. "Deprecated" is not enough to know what the current behavior is. It could be deprecated but still having effect.
| f'not {s.__class__.__name__}') | ||
| s = s.decode(detect_encoding(s), 'surrogatepass') | ||
|
|
||
| if encoding is not _sentinel: |
There was a problem hiding this comment.
How about detecting encoding with kw?
Remove encoding in arguments and:
if 'encoding' in kw:
del kw['encoding']
# raise warningThere was a problem hiding this comment.
That's actually way better indeed.
…sed. I also remove the `encoding` keyword from the docs docs.
73bc3b5 to
06623b5
Compare
|
Updated. Thanks @methane for the review. |
| :exc:`JSONDecodeError` will be raised. | ||
|
|
||
| .. deprecated:: 3.1 | ||
| The *encoding* keyword argument has been deprecated and will be ignored. |
There was a problem hiding this comment.
"will be ignored" is wrong.
But "deprecated" and "ignored" are described above already. How about simple notation?
.. deprecated-removed:: 3.1 3.9
*encoding* keyword argument.
| import warnings | ||
| warnings.warn( | ||
| "passing 'encoding' as a keyword argument is deprecated since" | ||
| " Python 3.1, is ignored and will be removed in Python 3.9.", |
There was a problem hiding this comment.
"as a keyword argument" looks confusing. ("Does it supported as positional argument?")
"'encoding' is ignored and deprecated. It will be removed in Python 3.9".
Additionally, del kw['encoding'] is required. Otherwise, JSONDecoder(..., **kw) will fail.
Co-Authored-By: Carreau <bussonniermatthias@gmail.com>
|
updated according to your comments. |
06c2c24 to
0ccae85
Compare
|
Thanks ! |
I also move in the docs the
encoding=...keyword to the end of thesignature, as it is kw only. Having the first argument in the doc being
the deprecated one does not seem to be the most efficient way to make
people forget about it.
This does not, of course emit warnings if you pass
Noneexplicitly,this could be further refined by using a sentinel value, but I'm unsure
this would be useful.
https://bugs.python.org/issue33461