Skip to content
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

bpo-17909: Document that json.load can accept a binary IO #7366

Merged
merged 1 commit into from
Jun 7, 2018

Conversation

asottile
Copy link
Contributor

@asottile asottile commented Jun 3, 2018

@@ -262,6 +262,11 @@ Basic Usage
.. versionchanged:: 3.6
All optional parameters are now :ref:`keyword-only <keyword-only_parameter>`.

.. versionchanged:: 3.6
*fp* can now be a :term:`file-like object` whose ``.read()`` method
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use just :term:`binary file`?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can go with that, was mostly matching the verbiage above (load after all only cares that the object has a .read() method)

@asottile asottile force-pushed the document_json_binary_io branch from ac1c262 to dd11bbf Compare June 4, 2018 05:14
@@ -262,6 +262,10 @@ Basic Usage
.. versionchanged:: 3.6
All optional parameters are now :ref:`keyword-only <keyword-only_parameter>`.

.. versionchanged:: 3.6
*fp* can now be a :term:`binary file`. The input encoding should be
UTF-8, UTF-16 or UTF-32.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO the feature should also be documented in the body of the function documentation, not also in this versionchanged note.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, added!

@asottile asottile force-pushed the document_json_binary_io branch from dd11bbf to e976fea Compare June 4, 2018 19:31
@asottile
Copy link
Contributor Author

asottile commented Jun 6, 2018

@vstinner @serhiy-storchaka look good now?

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@methane methane merged commit bb6366b into python:master Jun 7, 2018
@miss-islington
Copy link
Contributor

Thanks @asottile for the PR, and @methane for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 7, 2018
)

(cherry picked from commit bb6366b)

Co-authored-by: Anthony Sottile <asottile@umich.edu>
@bedevere-bot
Copy link

GH-7474 is a backport of this pull request to the 3.7 branch.

@bedevere-bot
Copy link

GH-7475 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 7, 2018
)

(cherry picked from commit bb6366b)

Co-authored-by: Anthony Sottile <asottile@umich.edu>
miss-islington added a commit that referenced this pull request Jun 7, 2018
(cherry picked from commit bb6366b)

Co-authored-by: Anthony Sottile <asottile@umich.edu>
miss-islington added a commit that referenced this pull request Jun 7, 2018
(cherry picked from commit bb6366b)

Co-authored-by: Anthony Sottile <asottile@umich.edu>
@asottile asottile deleted the document_json_binary_io branch June 18, 2018 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants