Skip to content

Comments

gh-133036: Deprecate codecs.open#133038

Merged
methane merged 9 commits intopython:mainfrom
methane:deprecate-codecs-open
Apr 30, 2025
Merged

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

Conversation

@methane
Copy link
Member

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

@methane methane added the stdlib Standard Library Python modules in the Lib/ directory label Apr 27, 2025
@methane methane requested review from malemburg and vstinner 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
StanFromIreland
StanFromIreland reviewed Apr 27, 2025
View reviewed changes
@vstinner
Copy link
Member

vstinner commented Apr 27, 2025

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

vstinner commented Apr 28, 2025

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

vstinner commented Apr 28, 2025

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
hugovk
hugovk reviewed Apr 29, 2025
View reviewed changes
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

vstinner commented Apr 30, 2025

Nicely done, thank you.

@malemburg
Copy link
Member

malemburg commented Apr 30, 2025

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

@methane @vstinner @hugovk @malemburg @AA-Turner @StanFromIreland