FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-133036: Deprecate codecs.open by methane · Pull Request #133038 · python/cpython · GitHub

/ cpython Public

gh-133036: Deprecate codecs.open - #133038

Merged
methane merged 9 commits into
python:mainfrom
methane:deprecate-codecs-open
Apr 30, 2025
ConversationCommits (9)ChecksFiles changed
Merged

gh-133036: Deprecate codecs.open#133038
methane merged 9 commits into
python:mainfrom
methane:deprecate-codecs-open

Conversation

methane commented Apr 27, 2025 •
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

methane added the stdlib Standard Library Python modules in the Lib/ directory label Apr 27, 2025
methane requested review from malemburg and vstinner April 27, 2025 06:24
bedevere-app Bot mentioned this pull request Apr 27, 2025
Closed
vstinner approved these changes Apr 27, 2025
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Comment thread Lib/test/test_codecs.py Outdated
hugovk reviewed Apr 27, 2025
View reviewed changes
Comment thread Lib/test/test_codecs.py Outdated
StanFromIreland reviewed Apr 27, 2025
View reviewed changes
Comment thread Doc/library/codecs.rst

vstinner commented Apr 27, 2025

Copy link
Copy Markdown
Member

Aha, 2 tests failed on the CI: test_multibytecodec and test_sax.

malemburg requested changes Apr 27, 2025
View reviewed changes

malemburg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for the PR. You are still missing a few places which need to be updated:

  • Lib/test/test_multibytecodec.py uses codecs.open()
  • Lib/test/test_sax.py uses codecs.open()
  • Lib/_pyio.py references codecs.open() for non-text encodings
  • Modules/_io/textio.c mentions codecs.open() as an alternative command

There may be more places. I have only searched for "codecs.open".

bedevere-app Bot commented Apr 27, 2025

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

vstinner commented Apr 28, 2025

Copy link
Copy Markdown
Member

Tests fail on Windows:

ERROR: test_code_page_decode_flags (test.test_codecs.CodePageTest.test_code_page_decode_flags)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\Lib\test\test_codecs.py", line 3466, in test_code_page_decode_flags
    if is_code_page_present(cp):
       ~~~~~~~~~~~~~~~~~~~~^^^^
  File "D:\a\cpython\cpython\Lib\test\test_codecs.py", line 44, in is_code_page_present
    class CPINFOEXW(ctypes.Structure):
                    ^^^^^^
NameError: name 'ctypes' is not defined. Did you mean: 'type'? Or did you forget to import 'ctypes'?

vstinner commented Apr 28, 2025

Copy link
Copy Markdown
Member

To fix test_codecs on Windows, you can apply this change for example:

diff --git a/Lib/test/test_codecs.py b/Lib/test/test_codecs.py
index a35de8ad87c..28039ae6cdd 100644
--- a/Lib/test/test_codecs.py
+++ b/Lib/test/test_codecs.py
@@ -35,13 +35,13 @@ def check(input, expect):

 # On small versions of Windows like Windows IoT or Windows Nano Server not all codepages are present
 def is_code_page_present(cp):
-    from ctypes import POINTER, WINFUNCTYPE, WinDLL
+    from ctypes import POINTER, WINFUNCTYPE, WinDLL, Structure
     from ctypes.wintypes import BOOL, BYTE, WCHAR, UINT, DWORD

     MAX_LEADBYTES = 12  # 5 ranges, 2 bytes ea., 0 term.
     MAX_DEFAULTCHAR = 2 # single or double byte
     MAX_PATH = 260
-    class CPINFOEXW(ctypes.Structure):
+    class CPINFOEXW(Structure):
         _fields_ = [("MaxCharSize", UINT),
                     ("DefaultChar", BYTE*MAX_DEFAULTCHAR),
                     ("LeadByte", BYTE*MAX_LEADBYTES),

hugovk commented Apr 28, 2025

Copy link
Copy Markdown
Member

Please could you also list it in https://github.com/python/cpython/blob/main/Doc/deprecations/pending-removal-in-future.rst ?

methane commented Apr 29, 2025

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again

bedevere-app Bot commented Apr 29, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@vstinner, @malemburg: please review the changes made to this pull request.

bedevere-app Bot requested review from malemburg and vstinner April 29, 2025 01:30
hugovk reviewed Apr 29, 2025
View reviewed changes
Comment thread Doc/deprecations/pending-removal-in-future.rst Outdated
methane and others added 2 commits April 29, 2025 17:23
Apply suggestions from code review
b2dced5 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Update Doc/deprecations/pending-removal-in-future.rst
c8af536 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
vstinner approved these changes Apr 29, 2025
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Comment thread Lib/test/test_codecs.py Outdated
Update Lib/test/test_codecs.py
ce5c94b 
Co-authored-by: Victor Stinner <vstinner@python.org>
hugovk changed the title gh-133036: deprecate codecs.open gh-133036: Deprecate codecs.open Apr 29, 2025
methane merged commit 4e294f6 into python:main Apr 30, 2025
methane deleted the deprecate-codecs-open branch April 30, 2025 01:11

vstinner commented Apr 30, 2025

Copy link
Copy Markdown
Member

Nicely done, thank you.

malemburg commented Apr 30, 2025

Copy link
Copy Markdown
Member

Thanks for making the requested changes!

@vstinner, @malemburg: please review the changes made to this pull request.

Looks good now. Thanks.

This was referenced Mar 11, 2026
Closed
Merged
Closed
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

AA-Turner AA-Turner left review comments
StanFromIreland StanFromIreland left review comments
vstinner vstinner approved these changes
hugovk hugovk approved these changes
malemburg Awaiting requested review from malemburg

Assignees

No one assigned

Labels

stdlib Standard Library Python modules in the Lib/ directory

Projects

Codecs and encodings issues
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL