Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-39377: json: Update doc about the encoding option. #18076
Conversation
I have a couple of minor grammar suggestions. Other than that, LGTM. |
Python 3.1, it was deprecated and ignored. And using it was emitting | ||
:exc:`DeprecationWarning` since Python 3.8. |
This comment has been minimized.
This comment has been minimized.
aeros
Jan 20, 2020
Member
Python 3.1, it was deprecated and ignored. As of Python 3.8, using it
emitted a :exc:`DeprecationWarning`.
This comment has been minimized.
This comment has been minimized.
methane
Jan 21, 2020
Author
Member
I'm not good at English so I don't understand the difference between current sentence and your suggestion.
I just copied upper entry and edit. If I should change "using it was emitting..." to "using it emitted", shoud I change the upper entry too?
This comment has been minimized.
This comment has been minimized.
ammaraskar
Jan 21, 2020
Member
Yeah, "emitted" is better since we're talking about a past version. Feel free to change was emitting -> emitted
on the entry above as well.
Co-Authored-By: Kyle Stanley <aeros167@gmail.com>
This comment has been minimized.
This comment has been minimized.
As Victor mentioned on the bpo issue, let's also note this in https://github.com/python/cpython/blob/master/Doc/whatsnew/3.9.rst#removed |
This comment has been minimized.
This comment has been minimized.
@ammaraskar Do you mean, I don't do it in this pull request? |
This comment has been minimized.
This comment has been minimized.
Oh sorry, my mistake, it is in the right place. |
LGTM with Kyle's suggestion. |
(Contributed by Victor Stinner in :issue:`39357`.) | ||
|
||
* The *encoding* parameter of :func:`json.loads` has been removed. As of | ||
Python 3.1, it was deprecated and ignored. And using it emitted a |
This comment has been minimized.
This comment has been minimized.
aeros
Jan 21, 2020
•
Member
One last point: I'd suggest starting the third sentence without the "And" or combining the sentences using a semicolon. Also, after reading over it again, I realized it would be more grammatically correct to write "using it has emitted" rather than "using it emitted". Here's a couple of examples:
-
As of Python 3.1, it was deprecated and ignored. Using it has emitted a :exc:
DeprecationWarning
since Python 3.8. -
As of Python 3.1, it was deprecated and ignored; using it has emitted a :exc:
DeprecationWarning
since Python 3.8.
Option (2) is likely more formally correct, but I think either would be fine.
Note: As a general English grammar rule, it's perfectly acceptable to start sentences with "And" (https://www.merriam-webster.com/words-at-play/words-to-not-begin-sentences-with), but it's typically discouraged to string together consecutive "and"s, even if they're separated by a period.
This comment has been minimized.
This comment has been minimized.
aeros
Jan 21, 2020
Member
In the future if you'd like me to review any documentation PRs or news entries for help with English grammar, I'd be happy to help! Feel free to request a review or @ mention me.
My primary areas of interest recently have been in asyncio, concurrent.futures, and subinterpreters, but I'm always glad to help with improving our documentation and news entries when I have the chance.
Also, I think your English skills are actually quite decent @methane, especially for someone who's native language is entirely dissimilar to English. I can generally understand the message you're trying to convey, there's just a few grammatical components that could be improved to make it more formally correct.
This comment has been minimized.
This comment has been minimized.
Thank you for your review! |
bpo-39377: json: Update doc about the encoding option. (pythonGH-18076)
methane commentedJan 20, 2020
•
edited by bedevere-bot
https://bugs.python.org/issue39377