Skip to content

gh-133036: Deprecate codecs.open #133038

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
Merged
merged 9 commits into from
Apr 30, 2025
Merged

gh-133036: Deprecate codecs.open #133038

merged 9 commits into from
Apr 30, 2025

Conversation

methane
Copy link
Member

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

@methane methane added the stdlib Python modules in the Lib dir label Apr 27, 2025
@methane methane requested review from vstinner and malemburg April 27, 2025 06:24
@bedevere-app bedevere-app bot mentioned this pull request Apr 27, 2025
Closed
vstinner
vstinner approved these changes Apr 27, 2025
View reviewed changes
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

hugovk
hugovk reviewed Apr 27, 2025
View reviewed changes
@vstinner
Copy link
Member

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

malemburg
malemburg requested changes Apr 27, 2025
View reviewed changes
Copy link
Member

@malemburg malemburg left a comment

Choose a reason for hiding this comment

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

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
Copy link

bedevere-app bot commented Apr 27, 2025

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

@vstinner
Copy link
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
Copy link
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
Copy link
Member

hugovk commented Apr 28, 2025

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

@methane
Copy link
Member Author

methane commented Apr 29, 2025

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Apr 29, 2025

Thanks for making the requested changes!

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

@bedevere-app bedevere-app bot requested review from malemburg and vstinner April 29, 2025 01:30
methane and others added 2 commits April 29, 2025 17:23
@methane @hugovk
Apply suggestions from code review
b2dced5 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@methane @hugovk
Update Doc/deprecations/pending-removal-in-future.rst
c8af536 
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
vstinner
vstinner approved these changes Apr 29, 2025
View reviewed changes
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 @vstinner
Update Lib/test/test_codecs.py
ce5c94b 
Co-authored-by: Victor Stinner <vstinner@python.org>
@hugovk hugovk changed the title gh-133036: deprecate codecs.open gh-133036: Deprecate codecs.open Apr 29, 2025
@methane methane merged commit 4e294f6 into python:main Apr 30, 2025
42 checks passed
@methane methane deleted the deprecate-codecs-open branch April 30, 2025 01:11
@vstinner
Copy link
Member

Nicely done, thank you.

@malemburg
Copy link
Member

Thanks for making the requested changes!

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

Looks good now. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AA-Turner AA-Turner AA-Turner left review comments

@StanFromIreland StanFromIreland StanFromIreland left review comments

@vstinner vstinner vstinner approved these changes

@hugovk hugovk hugovk approved these changes

@malemburg malemburg Awaiting requested review from malemburg

Assignees
No one assigned
Labels
stdlib Python modules in the Lib dir
Projects
Codecs and encodings issues
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@methane @vstinner @hugovk @malemburg @AA-Turner @StanFromIreland