GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-41842: Add a unregister function in _codecs module by shihai1991 · Pull Request #22360 · python/cpython

Conversation

shihai1991 commented Sep 22, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

Add a _codecs.unregister() in _codecs module to unregister search function.

https://bugs.python.org/issue41842

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

the-knights-who-say-ni added the CLA signed label

Sep 22, 2020

bedevere-bot added the awaiting review label

Sep 22, 2020

shihai1991 changed the title ~~Add a unregister function in _codecs module~~ [WIP] Add a unregister function in _codecs module

Sep 22, 2020

shihai1991 force-pushed the bpo_39337_new_3 branch from 0692346 to 03645a7 Compare

September 22, 2020 13:23


          add a _codecs.unregister in _codecs module

fefda70

shihai1991 force-pushed the bpo_39337_new_3 branch from 03645a7 to fefda70 Compare

September 22, 2020 13:57

shihai1991 changed the title ~~[WIP] Add a unregister function in _codecs module~~ bpo-39337: Add a unregister function in _codecs module

Sep 22, 2020

shihai1991 commented Sep 22, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member Author

Pls take a look if you have free times, thanks. @vstinner @serhiy-storchaka

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reviewed

Sep 23, 2020

View reviewed changes

vstinner left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you please add an entry in the Doc/whatsnew/3.10.rst document? You have to add a news "codecs" section near:
https://docs.python.org/dev/whatsnew/3.10.html#improved-modules

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/library/codecs.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/library/codecs.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Misc/NEWS.d/next/Library/2020-09-22-20-54-18.bpo-39337.zX2e5M.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_codecsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Sep 23, 2020

Copy link

Copy Markdown

Member

It would be nice to use this new function to remove an old side effect of test_codecs: codecs.register(_get_test_codec) registers a search function which is never unregistered.

Maybe ExceptionChainingTest.setUp() method should register the search function and use self.addCleanup(codecs.unregister, _get_test_codec)?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Sep 23, 2020

Copy link

Copy Markdown

Member

See this comment of test_codecs.py:

        # There's no way to unregister a codec search function, so we just
        # ensure we render this one fairly harmless after the test
        # case finishes by using the test case repr as the codec name
        # The codecs module normalizes codec names, although this doesn't
        # appear to be formally documented...
        # We also make sure we use a truly unique id for the custom codec
        # to avoid issues with the codec cache when running these tests
        # multiple times (e.g. when hunting for refleaks)

And https://bugs.python.org/issue22166

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Sep 23, 2020

Copy link

Copy Markdown

Member

Old email requesting codecs.unregister() function: https://mail.python.org/pipermail/python-dev/2011-September/113588.html

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Sep 23, 2020

Copy link

Copy Markdown

Member

https://bugs.python.org/issue39337 is not directly related to codecs.unregister().

@shihai1991: I created https://bugs.python.org/issue41842 "Add codecs.unregister() to unregister a codec search function": please use this bpo for your PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reviewed

Sep 23, 2020

View reviewed changes

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 commented Sep 23, 2020

Copy link

Copy Markdown

Member Author

Maybe ExceptionChainingTest.setUp() method should register the search function and use self.addCleanup(codecs.unregister, _get_test_codec)?

+1， it is in my TODO list :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner commented Sep 23, 2020

Copy link

Copy Markdown

Member

Updating test_codecs can be done in a second following PR if you prefer.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.


          apply victor's comment

f7775c0

shihai1991 changed the title ~~bpo-39337: Add a unregister function in _codecs module~~ bpo-41842: Add a unregister function in _codecs module

Sep 23, 2020

shihai1991 commented Sep 23, 2020

Copy link

Copy Markdown

Member Author

https://bugs.python.org/issue39337 is not directly related to codecs.unregister().

@shihai1991: I created https://bugs.python.org/issue41842 "Add codecs.unregister() to unregister a codec search function": please use this bpo for your PR.

copy that, this bpo have been used in this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner reviewed

Sep 23, 2020

View reviewed changes

Comment thread

Doc/library/codecs.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Include/codecs.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_codecs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Misc/NEWS.d/next/Library/2020-09-23-22-52-24.bpo-41842.lIuhC9.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_codecsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Modules/_codecsmodule.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.


          apply victor's comment

71b624e

shihai1991 requested a review from vstinner

September 23, 2020 17:38

vstinner reviewed

Sep 23, 2020

View reviewed changes

Comment thread

Include/codecs.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Include/codecs.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_codecs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 added 2 commits

September 24, 2020 12:45


          apply victor's comment

d186726


          retrigger checks

f8e912b

vstinner reviewed

Sep 24, 2020

View reviewed changes

Comment thread

Doc/c-api/codec.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Include/codecs.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Include/codecs.h Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_codecs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_codecs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 added 4 commits

September 24, 2020 19:27


          apply victor's comment

40666fc


          update doc

3257b6c


          update doc

a493a35


          Merge remote-tracking branch 'origin/master' into bpo_39337_new_3

e29ec44

* origin/master: (27 commits)
  bpo-41428: Fix compiler warnings in unionobject.c (pythonGH-22388)
  bpo-41654: Fix compiler warning in MemoryError_dealloc() (pythonGH-22387)
  bpo-41833: threading.Thread now uses the target name (pythonGH-22357)
  bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH-21633)
  bpo-33822: Update IDLE section of What's New 3.8 (pythonGH-22383)
  bpo-41844: Add IDLE section to What's New 3.9 (GN-22382)
  bpo-41841: Prepare IDLE News for 3.10 (pythonGH-22379)
  bpo-37779 : Add information about the overriding behavior of ConfigParser.read (pythonGH-15177)
  bpo-40170: Use inline _PyType_HasFeature() function (pythonGH-22375)
  bpo-40941: Fix stackdepth compiler warnings (pythonGH-22377)
  bpo-40941: Fix fold_tuple_on_constants() compiler warnings (pythonGH-22378)
  bpo-40521: Fix PyUnicode_InternInPlace() (pythonGH-22376)
  bpo-41834: Remove _Py_CheckRecursionLimit variable (pythonGH-22359)
  bpo-1635741, unicodedata: add ucd_type parameter to UCD_Check() macro (pythonGH-22328)
  bpo-1635741: Port _lsprof extension to multi-phase init (PEP 489) (pythonGH-22220)
  bpo-41513: Improve order of adding fractional values. Improve variable names. (pythonGH-22368)
  bpo-41816: `StrEnum.__str__` is `str.__str__` (pythonGH-22362)
  bpo-35764: Rewrite the IDLE Calltips doc section  (pythonGH-22363)
  bpo-41810: Reintroduce `types.EllipsisType`, `.NoneType` & `.NotImplementedType` (pythonGH-22336)
  bpo-41602: raise SIGINT exit code on KeyboardInterrupt from pymain_run_module (python#21956)
  ...


          update doc

db4631e

vstinner approved these changes

Sep 24, 2020

View reviewed changes

vstinner left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM. I just requested a bunch of minor edits.

I leave the PR open a few days if @malemburg wants to have a look.

cc @serhiy-storchaka

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_codecs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Lib/test/test_codecs.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot added awaiting merge and removed awaiting review labels

Sep 24, 2020


          apply victor's comment

5efff22

corona10 reviewed

Sep 24, 2020

View reviewed changes

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.


          apply Dong-hee Na's comment

f18cd4f

vstinner reviewed

Sep 26, 2020

View reviewed changes

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

ZackerySpytz reviewed

Sep 26, 2020

View reviewed changes

Comment thread

Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/codecs.c Outdated

		return 0;
		}

		Py_ssize_t n = PyList_Size(codec_search_path);

ZackerySpytz Sep 26, 2020

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please use the macro PyList_GET_SIZE() instead of PyList_Size(), as codec_search_path is guaranteed to be a list.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

vstinner Sep 26, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

While it's a list right now, its type can change tomorrow.

@shihai1991: If you modify the code to use fast macros, please add the following assertion before using it:

assert(PyList_CheckExact(interp->codec_search_path));

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

shihai1991 Sep 26, 2020

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@vstinner Copy that, I have already updated it.
But I have a question. assert(PyList_Check(op) in _PyList_CAST() is not good enough?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Python/codecs.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 added 2 commits

September 27, 2020 00:02


          apply victor and Zackery's comment

8f6dc4d


          apply victor's comment

4308ea8

vstinner reviewed

Sep 27, 2020

View reviewed changes

Comment thread

Misc/NEWS.d/next/Library/2020-09-23-22-52-24.bpo-41842.lIuhC9.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.


          apply victor's comment

c9a0c99

taleinat mentioned this pull request

Sep 27, 2020

bpo-39337: Add a test case for normalizing of codec names #19069

Merged

vstinner merged commit d332e7b into python:master

Sep 28, 2020

bedevere-bot removed the awaiting merge label

Sep 28, 2020

vstinner commented Sep 28, 2020

Copy link

Copy Markdown

Member

Merged, thank you @shihai1991!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 commented Sep 29, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member Author

Thanks for everyone's review: ).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 added a commit to shihai1991/cpython that referenced this pull request

Sep 29, 2020


          Merge remote-tracking branch 'origin/master' into bpo_1635741_1

e3b9365

* origin/master: (113 commits)
  bpo-41773: Raise exception for non-finite weights in random.choices().  (pythonGH-22441)
  bpo-41873: Add vectorcall for float() (pythonGH-22432)
  bpo-41861: Convert _sqlite3 PrepareProtocolType to heap type (pythonGH-22428)
  bpo-41842: Add codecs.unregister() function (pythonGH-22360)
  bpo-41875: Use __builtin_unreachable when possible (pythonGH-22433)
  bpo-40105: ZipFile truncate in append mode with shorter comment (pythonGH-19337)
  bpo-41870: Use PEP 590 vectorcall to speed up bool()  (pythonGH-22427)
  [doc] Leverage the fact that the actual types can now be indexed for typing (pythonGH-22340)
  bpo-41861: Convert _sqlite3 cache and node static types to heap types (pythonGH-22417)
  bpo-41858: Clarify line in optparse doc (pythonGH-22407)
  Revert "Fix all Python Cookbook links (python#22205)" (pythonGH-22424)
  bpo-1635741: Port _bisect module to multi-phase init (pythonGH-22415)
  bpo-41428: Fix compiler warning in unionobject.c (pythonGH-22416)
  Fix logging error message (pythonGH-22410)
  bpo-39934: Account for control blocks in 'except' in compiler. (pythonGH-22395)
  bpo-41775: Make 'IDLE Shell' the shell title  (python#22399)
  bpo-41428: Fix compiler warnings in unionobject.c (pythonGH-22388)
  bpo-41654: Fix compiler warning in MemoryError_dealloc() (pythonGH-22387)
  bpo-41833: threading.Thread now uses the target name (pythonGH-22357)
  bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH-21633)
  ...

xzy3 pushed a commit to xzy3/cpython that referenced this pull request

Oct 18, 2020


          bpo-41842: Add codecs.unregister() function (pythonGH-22360)

e3625f2

Add codecs.unregister() and PyCodec_Unregister() functions
to unregister a codec search function.

lancelote mentioned this pull request

May 27, 2021

Add codecs.unregister() for Python 3.10 python/typeshed#5539

Merged

Reviewers

vstinner

vstinner approved these changes

corona10

corona10 left review comments

+1 more reviewer

ZackerySpytz

ZackerySpytz left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

shihai1991 commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shihai1991 commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

Uh oh!

shihai1991 commented Sep 23, 2020

Uh oh!

vstinner commented Sep 23, 2020

Uh oh!

shihai1991 commented Sep 23, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZackerySpytz Sep 26, 2020

Choose a reason for hiding this comment

Uh oh!

vstinner Sep 26, 2020

Choose a reason for hiding this comment

Uh oh!

shihai1991 Sep 26, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner commented Sep 28, 2020

Uh oh!

shihai1991 commented Sep 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

shihai1991 commented Sep 22, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 commented Sep 22, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

shihai1991 commented Sep 29, 2020 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.