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.

Sign up for GitHub

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

Jump to bottom

gh-101936: Update the default value of fp from io.StringIO to io.BytesIO #102100

Merged
merged 7 commits into from Feb 21, 2023
Merged

gh-101936: Update the default value of fp from io.StringIO to io.BytesIO #102100

merged 7 commits into from Feb 21, 2023

Conversation

longvhdeveloper
Copy link
Contributor

@longvhdeveloper longvhdeveloper commented Feb 21, 2023 •
edited by bedevere-bot

@bedevere-bot bedevere-bot mentioned this pull request Feb 21, 2023
Closed
@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Feb 21, 2023 •
edited

All commit authors signed the Contributor License Agreement.
CLA signed

@corona10 corona10 self-assigned this Feb 21, 2023
@corona10 corona10 self-requested a review February 21, 2023 07:33
@corona10 corona10 added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Feb 21, 2023
corona10
corona10 requested changes Feb 21, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

Please update the test to following codeline.

cpython/Lib/test/test_urllib2.py

Lines 1827 to 1830 in 6f25657

def test_gh_98778(self):
x = urllib.error.HTTPError("url", 405, "METHOD NOT ALLOWED", None, None)
self.assertEqual(getattr(x, "__notes__", ()), ())

Misc/NEWS.d/next/Library/2023-02-21-07-15-41.gh-issue-101936.QVOxHH.rst Outdated
@@ -0,0 +1 @@
HTTPError fp.read returns string instead of bytes
Copy link
Member

@corona10 corona10 Feb 21, 2023 •
edited

Choose a reason for hiding this comment

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

The default value of ``fp`` becomes :class:`io.BytesIO` if :exc:`~urllib.error.HTTPError` is initialized without a designated `fp` parameter. Patch by Long Vo.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@corona10 corona10 changed the title gh-101936: HTTPError fp.read returns string instead of bytes gh-101936: Update the default value of fp from io.StringIO to io.BytesIO Feb 21, 2023
corona10
corona10 requested changes Feb 21, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

Almost come :)

Misc/NEWS.d/next/Library/2023-02-21-07-48-03.gh-issue-101936.xPawo7.rst Outdated
Comment on lines 1 to 2
The default value of fp becomes :class:io.BytesIO if
:exc:~urllib.error.HTTPError is initialized without a designated fp
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The default value of fp becomes :class:io.BytesIO if
:exc:~urllib.error.HTTPError is initialized without a designated fp
The default value of fp becomes :class:`io.BytesIO` if
:exc:`~urllib.error.HTTPError` is initialized without a designated ``fp``

Lib/test/test_urllib2.py Outdated
@@ -1827,6 +1827,7 @@ def test_HTTPError_interface(self):
def test_gh_98778(self):
x = urllib.error.HTTPError("url", 405, "METHOD NOT ALLOWED", None, None)
self.assertEqual(getattr(x, "__notes__", ()), ())
self.assertEqual(isinstance(x.fp.read(), bytes), True)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self.assertEqual(isinstance(x.fp.read(), bytes), True)
self.assertIsInstance(x.fp.read(), bytes)

corona10
corona10 approved these changes Feb 21, 2023
View reviewed changes
Copy link
Member

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

lgtm

@corona10 corona10 merged commit 0d4c7fc into python:main Feb 21, 2023
@miss-islington
Copy link
Contributor

Thanks @longvhdeveloper for the PR, and @corona10 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2023
@longvhdeveloper @miss-islington
pythongh-101936: Update the default value of fp from io.StringIO to i…
e99397c 
…o.BytesIO (pythongh-102100)

(cherry picked from commit 0d4c7fc)

Co-authored-by: Vo Hoang Long <vohoanglong07@gmail.com>
Co-authored-by: Long Vo <long.vo@linecorp.com>
@bedevere-bot
Copy link

GH-102117 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Feb 21, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2023
@longvhdeveloper @miss-islington
pythongh-101936: Update the default value of fp from io.StringIO to i…
496a500 
…o.BytesIO (pythongh-102100)

(cherry picked from commit 0d4c7fc)

Co-authored-by: Vo Hoang Long <vohoanglong07@gmail.com>
Co-authored-by: Long Vo <long.vo@linecorp.com>
@bedevere-bot
Copy link

GH-102118 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Feb 21, 2023
corona10 pushed a commit that referenced this pull request Feb 22, 2023
@miss-islington @longvhdeveloper
[3.11] gh-101936: Update the default value of fp from io.StringIO to …
edbde8f 
…io.BytesIO (gh-102100) (gh-102117)

gh-101936: Update the default value of fp from io.StringIO to io.BytesIO (gh-102100)
(cherry picked from commit 0d4c7fc)

Co-authored-by: Long Vo <long.vo@linecorp.com>
corona10 pushed a commit that referenced this pull request Feb 22, 2023
@miss-islington @longvhdeveloper
[3.10] gh-101936: Update the default value of fp from io.StringIO to …
0f28af5 
…io.BytesIO (gh-102100) (#102118)

gh-101936: Update the default value of fp from io.StringIO to io.BytesIO (gh-102100)
(cherry picked from commit 0d4c7fc)

Co-authored-by: Long Vo <long.vo@linecorp.com>
carljm added a commit to carljm/cpython that referenced this pull request Feb 23, 2023
@carljm
Merge branch 'main' into lockfreecp
b257a16 
* main: (76 commits)
  Fix syntax error in struct doc example (python#102160)
  pythongh-99108: Import MD5 and SHA1 from HACL* (python#102089)
  pythonGH-101777: `queue.rst`: use 2 spaces after a period to be consistent. (python#102143)
  Few coverage nitpicks for the cmath module (python#102067)
  pythonGH-100982: Restrict `FOR_ITER_RANGE` to a single instruction to allow instrumentation. (pythonGH-101985)
  pythongh-102135: Update turtle docs to rename wikipedia demo to rosette (python#102137)
  pythongh-99942: python.pc on android/cygwin should link to libpython per configure.ac (pythonGH-100356)
  pythongh-95672 fix typo SkitTest to SkipTest (pythongh-102119)
  pythongh-101936: Update the default value of fp from io.StringIO to io.BytesIO (pythongh-102100)
  pythongh-102008: simplify test_except_star by using sys.exception() instead of sys.exc_info() (python#102009)
  pythongh-101903: Remove obsolete undefs for previously removed macros Py_EnterRecursiveCall and Py_LeaveRecursiveCall (python#101923)
  pythongh-100556: Improve clarity of `or` docs (python#100589)
  pythongh-101777: Make `PriorityQueue` docs slightly clearer (python#102026)
  pythongh-101965: Fix usage of Py_EnterRecursiveCall return value in _bisectmodule.c (pythonGH-101966)
  pythongh-101578: Amend exception docs (python#102057)
  pythongh-101961 fileinput.hookcompressed should not set the encoding value for the binary mode (pythongh-102068)
  pythongh-102056: Fix a few bugs in error handling of exception printing code (python#102078)
  pythongh-102011: use sys.exception() instead of sys.exc_info() in docs where possible (python#102012)
  pythongh-101566: Sync with zipp 3.14. (pythonGH-102018)
  pythonGH-99818: improve the documentation for zipfile.Path and Traversable (pythonGH-101589)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 corona10 approved these changes

Assignees

@corona10 corona10

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@longvhdeveloper @bedevere-bot @miss-islington @corona10