GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-88886: Remove excessive encoding name normalization by serhiy-storchaka · Pull Request #137167 · python/cpython

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented Jul 28, 2025 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

The codecs lookup function now performs only minimal normalization of the encoding name before passing it to the search functions: all ASCII letters are converted to lower case, spaces are replaced with hyphens.

Excessive normalization broke third-party codecs providers, like python-iconv.

Revert "bpo-37751: Fix codecs.lookup() normalization (GH-15092)"

This reverts commit 20f59fe.

  • Issue: Codec name normalization breaks custom codecs #88886

📚 Documentation preview 📚: https://cpython-previews--137167.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

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

All reactions
@serhiy-storchaka
serhiy-storchaka requested a review from malemburg July 28, 2025 14:06
@bedevere-app bedevere-app Bot added the awaiting core review label Jul 28, 2025
@serhiy-storchaka
serhiy-storchaka requested a review from vstinner July 28, 2025 14:06
@bedevere-app bedevere-app Bot mentioned this pull request Jul 28, 2025
Codec name normalization breaks custom codecs #88886
Open
StanFromIreland
StanFromIreland reviewed Jul 28, 2025
View reviewed changes

@StanFromIreland StanFromIreland left a comment •
edited
Loading

Uh oh!

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

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.

You need to update normalize_encodings doc.

@malemburg Which order should the two PRs be merged in, switching it to the C implementation and simplifying the implementation?

Sorry, something went wrong.

Uh oh!

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

All reactions
@serhiy-storchaka

serhiy-storchaka commented Jul 28, 2025 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

This mainly restores the status quo prior to bpo-37751 and updates the documentation. But I am planning more changes.

I was not sure about replacing spaces with hyphens? Should we kept this here, on leave it to the search function? Should we convert spaces to underscores instead? Or should we remove any transformation?

I plan also to change _Py_normalize_encoding() -- only convert to lower case and replace spaces, hyphens and underscores, so encoding like "utf#$^(&8" will no longer be accepted. And change encodings.normalize_encoding() in a similar way. Names that are not valid module names after normalization should not be found.

All reactions

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka
pythongh-88886: Remove excessive encoding name normalization
ae1cae2
The codecs lookup function now performs only minimal normalization of
the encoding name before passing it to the search functions:
all ASCII letters are converted to lower case, spaces are replaced
with hyphens.

Excessive normalization broke third-party codecs providers, like
python-iconv.

Revert "bpo-37751: Fix codecs.lookup() normalization (pythonGH-15092)"

This reverts commit 20f59fe.
@serhiy-storchaka
serhiy-storchaka force-pushed the normalize_encoding branch from 2df6169 to ae1cae2 Compare July 28, 2025 14:23
malemburg
malemburg approved these changes Jul 29, 2025
View reviewed changes

@malemburg malemburg 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 modulo the one change to the news entry.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-07-28-17-01-05.gh-issue-88886.g4XFPb.rst Resolved

Uh oh!

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

@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jul 29, 2025
StanFromIreland
StanFromIreland reviewed Jul 29, 2025
View reviewed changes

@StanFromIreland StanFromIreland left a comment •
edited
Loading

Uh oh!

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

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.

The docs still need updating: https://docs.python.org/3.15/library/codecs.html#encodings.normalize_encoding

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.

@malemburg malemburg mentioned this pull request Jul 29, 2025
encoding package's normalize_encoding() function is too slow #55531
Closed
@serhiy-storchaka
Update a NEWS entry.
9c0f595
vstinner
vstinner approved these changes Sep 2, 2025
View reviewed changes

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

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.

@malemburg malemburg mentioned this pull request Sep 9, 2025
gh-55531: Implement normalize_encoding in C #136643
Merged
@malemburg

malemburg commented Sep 9, 2025

Copy link
Copy Markdown
Member

Is there anything left to be done for this PR ?

After merging this one, I'd like to merge #136643

This will then give us a better story overall regarding encoding normalization in CPython: a central function which deals with normalization, avoiding duplicate normalizations as best as possible.

All reactions

Sorry, something went wrong.

Uh oh!

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

serhiy-storchaka and others added 2 commits September 9, 2025 14:56
@serhiy-storchaka @vstinner
Update Python/codecs.c
4546171
Co-authored-by: Victor Stinner <[email protected]>
@serhiy-storchaka
Merge branch 'main' into normalize_encoding
68308d9
@serhiy-storchaka

serhiy-storchaka commented Sep 9, 2025

Copy link
Copy Markdown
Member Author

I do not think #136643 should be merged. Normalization is an internal affair of the search function. Different search functions can have different normalizations (including no normalization).

All reactions

Sorry, something went wrong.

Uh oh!

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

@malemburg

malemburg commented Sep 9, 2025

Copy link
Copy Markdown
Member

I do not think #136643 should be merged. Normalization is an internal affair of the search function. Different search functions can have different normalizations (including no normalization).

Sure, but the encoding package uses the same normalization as the C implementation that's used internally (after all, the C implementation was crafted after the encoding package's normalization function), so it makes sense to reuse it that way.

Of course, other codec packages can have their own search functions and normalizations.

All reactions

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka

serhiy-storchaka commented Sep 9, 2025

Copy link
Copy Markdown
Member Author

The C code only needs support for few variants of few builting encodings (like "ISO-8859-1", "ISO_8859-1" and "iso8859-1"), but it does not need to support normalization for all encodings or more exotic forms of builting encodings (like "ISO_8859-1:1987" or "iso88591"). It is needed because some encodings should be accessible by their standard names before importing the encodings package.

All reactions

Sorry, something went wrong.

Uh oh!

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

@malemburg

malemburg commented Sep 9, 2025

Copy link
Copy Markdown
Member

The C code only needs support for few variants of few builting encodings (like "ISO-8859-1", "ISO_8859-1" and "iso8859-1"), but it does not need to support normalization for all encodings or more exotic forms of builting encodings (like "ISO_8859-1:1987" or "iso88591"). It is needed because some encodings should be accessible by their standard names before importing the encodings package.

Yes. I am well aware that it is internally only used for a few encodings, but since it is a C reimplementation of the encoding package's normalization function, it's a good idea to have this implemented in only one place. Given that the C version is faster, merging the PR is what I'd like to do.

All reactions

Sorry, something went wrong.

Uh oh!

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

@serhiy-storchaka
serhiy-storchaka merged commit af58a6f into python:main Sep 9, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 9, 2025
@serhiy-storchaka
serhiy-storchaka deleted the normalize_encoding branch September 9, 2025 18:07
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@serhiy-storchaka @lkollar
pythongh-88886: Remove excessive encoding name normalization (pythonG…
0a3e79b
…H-137167)

The codecs lookup function now performs only minimal normalization of
the encoding name before passing it to the search functions:
all ASCII letters are converted to lower case, spaces are replaced
with hyphens.

Excessive normalization broke third-party codecs providers, like
python-iconv.

Revert "bpo-37751: Fix codecs.lookup() normalization (pythonGH-15092)"

This reverts commit 20f59fe.
@bodograumann bodograumann mentioned this pull request Oct 31, 2025
ASCII//TRANSLIT not found with python 3.9 bodograumann/python-iconv#4
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malemburg malemburg malemburg approved these changes
@StanFromIreland StanFromIreland StanFromIreland left review comments
@vstinner vstinner vstinner approved these changes

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.

4 participants

@serhiy-storchaka @malemburg @vstinner @StanFromIreland
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.