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-92536: PEP 623: Remove wstr from unicode by methane · Pull Request #92537 · python/cpython

Conversation

@methane

@methane methane commented May 9, 2022

Copy link
Copy Markdown
Member

gh-92536

Sorry, something went wrong.

Uh oh!

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

All reactions
@bedevere-bot bedevere-bot added the awaiting core review label May 9, 2022
@methane
methane force-pushed the remove-unicode-wchar branch from 390915a to eda0fb0 Compare May 9, 2022 08:20
methane added 4 commits May 9, 2022 17:26
@methane
Remove USE_UNICODE_WCHAR_CACHE
06fc65b
@methane
remove deprecated APIs
718df6d
@methane
Remove wstr and wstr_length
10517d7
@methane
Remove ready flag
d5a14c2
@methane
methane force-pushed the remove-unicode-wchar branch from eda0fb0 to d5a14c2 Compare May 9, 2022 08:26
methane added 4 commits May 9, 2022 17:28
@methane
fixup comment
f54ce3f
@methane
fix test_sys
185fa30
@methane
Update docs
a61d9fb
@methane
Ressurect PyUnicode_READY()
2bfef32
methane
methane commented May 9, 2022
View reviewed changes
Comment thread Doc/c-api/unicode.rst
.. versionadded:: 3.3

.. deprecated:: 3.10
This API do nothing since Python 3.12. Please remove code using this function.

@methane methane May 9, 2022

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.

Huge number of code using this API.
https://github.com/hpyproject/top4000-pypi-packages/search?q=PyUnicode_READY

Sorry, something went wrong.

Uh oh!

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

All reactions

@slateny slateny May 12, 2022

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.

This API does nothing..., and I think the 'Please remove ...' can be omitted

Sorry, something went wrong.

Uh oh!

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

All reactions
methane added 2 commits May 10, 2022 12:34
@methane
Merge remote-tracking branch 'upstream/main' into remove-unicode-wchar
87a80e5
@methane
Add NEWS and whatsnew entry
19c78fa
@methane
methane marked this pull request as ready for review May 10, 2022 03:40
@methane
methane requested review from a team and iritkatriel as code owners May 10, 2022 03:40
arhadthedev
arhadthedev reviewed May 10, 2022
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

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

@methane @arhadthedev
Update Doc/whatsnew/3.12.rst
2d50810
Co-authored-by: Oleg Iarygin <[email protected]>
@methane
methane requested a review from serhiy-storchaka May 10, 2022 11:40
@methane

methane commented May 10, 2022

Copy link
Copy Markdown
Member Author

I added @serhiy-storchaka to reviewer because he is author of USE_UNICODE_WCHAR_CACHE.

All reactions

Sorry, something went wrong.

Uh oh!

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

methane added 3 commits May 11, 2022 11:31
@methane
Mark PyUnicode_GetSize abi_only
eae02ff
@methane
Remove 'u' and 'Z' from c-api/arg doc.
01f7e8e
@methane
Add one more paragraph to what's new
12736cc
ericsnowcurrently
ericsnowcurrently reviewed May 11, 2022
View reviewed changes

@ericsnowcurrently ericsnowcurrently 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.

Nothing wrong stood out after a quick scan on my phone. The change seems mostly mechanical and limited to removals. Is that right?

Also, should the PEP be mentioned more prominently in the Misc/NEWS entry and whats_new?

Sorry, something went wrong.

Uh oh!

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

All reactions
@serhiy-storchaka

serhiy-storchaka commented May 11, 2022

Copy link
Copy Markdown
Member

What if merge #12409 first and set HAVE_UNICODE_WCHAR_CACHE to 0? It will have an effect of removing wstr, but keep possibility to return it in custom builds.

All reactions

Sorry, something went wrong.

Uh oh!

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

@arhadthedev

arhadthedev commented May 11, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Argument Clinic How-To also needs an update:

diff --git a/Doc/howto/clinic.rst b/Doc/howto/clinic.rst
index 04b1a2cac0..b969dc8f42 100644
--- a/Doc/howto/clinic.rst
+++ b/Doc/howto/clinic.rst
@@ -848,15 +848,11 @@ on the right is the text you'd replace it with.
 ``'s#'``    ``str(zeroes=True)``
 ``'s*'``    ``Py_buffer(accept={buffer, str})``
 ``'U'``     ``unicode``
-``'u'``     ``Py_UNICODE``
-``'u#'``    ``Py_UNICODE(zeroes=True)``
 ``'w*'``    ``Py_buffer(accept={rwbuffer})``
 ``'Y'``     ``PyByteArrayObject``
 ``'y'``     ``str(accept={bytes})``
 ``'y#'``    ``str(accept={robuffer}, zeroes=True)``
 ``'y*'``    ``Py_buffer``
-``'Z'``     ``Py_UNICODE(accept={str, NoneType})``
-``'Z#'``    ``Py_UNICODE(accept={str, NoneType}, zeroes=True)``
 ``'z'``     ``str(accept={str, NoneType})``
 ``'z#'``    ``str(accept={str, NoneType}, zeroes=True)``
 ``'z*'``    ``Py_buffer(accept={buffer, str, NoneType})``
All reactions

Sorry, something went wrong.

Uh oh!

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

@methane
Py_UNICODE -> wchar_t in clinic howto
e6992a0
@methane

methane commented May 11, 2022

Copy link
Copy Markdown
Member Author

What if merge #12409 first and set HAVE_UNICODE_WCHAR_CACHE to 0? It will have an effect of removing wstr, but keep possibility to return it in custom builds.

I think it is very difficult to maintain both of HAVE_UNICODE_WCHAR_CACHE is 0 and 1.

Argument Clinic How-To also needs an update:

AC supports u and Z even after removing wstr. I will replace Py_UNICODE with wchar_t in the clinic howto.

All reactions

Sorry, something went wrong.

Uh oh!

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

@methane
Merge remote-tracking branch 'upstream/main' into remove-unicode-wchar
5b7611b
methane added 5 commits May 11, 2022 13:31
@methane
Fix reST syntax
d7a534e
@methane
fix reST syntax
b6fdb1a
@methane
make regen-limited-abi
0e0a564
@methane
Remove PyUnicode_GetSize from public header.
3ebaf93
@methane
Merge remote-tracking branch 'upstream/main' into remove-unicode-wchar
0ddef52
@methane
methane merged commit f9c9354 into python:main May 12, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label May 12, 2022
methane added a commit to methane/peps that referenced this pull request May 12, 2022
@methane
PEP 623: Mark it final
ddba002
Implemented in python/cpython#92537
slateny
slateny reviewed May 12, 2022
View reviewed changes

@slateny slateny left a comment

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.

some optional grammar changes if you revisit some of these files later down the line

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Doc/c-api/arg.rst
encoding passed in as parameter.

.. versionchanged:: 3.12
``u``, ``u#``, ``Z``, and ``Z#`` are removed because they used legacy ``Py_UNICODE*``

@slateny slateny May 12, 2022

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.

... because they used a/the legacy ...

Sorry, something went wrong.

Uh oh!

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

All reactions

@methane methane May 13, 2022

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.

Thank you. I made follow-up PR: #92756

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Doc/c-api/unicode.rst
.. versionadded:: 3.3

.. deprecated:: 3.10
This API do nothing since Python 3.12. Please remove code using this function.

@slateny slateny May 12, 2022

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.

This API does nothing..., and I think the 'Please remove ...' can be omitted

Sorry, something went wrong.

Uh oh!

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

All reactions
@methane methane mentioned this pull request May 12, 2022
PEP 623: Change status to Final python/peps#2601
Merged
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 12, 2022
bpo-36346: Prepare for removing the legacy Unicode C API. #12409
Closed
@vstinner

vstinner commented May 12, 2022

Copy link
Copy Markdown
Member

Congratulations for finally managing to remove this legacy API. A big thank you! I hated this API since Python 3.3, having to call "PyUnicode_Ready()" was super annoying. Thanks for you tenacity to remove it!

All reactions

Sorry, something went wrong.

Uh oh!

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

methane added a commit to python/peps that referenced this pull request May 13, 2022
@methane
PEP 623: Change status to Final (#2601)
9111678
* PEP 623: Mark it final

Implemented in python/cpython#92537

* Add Discussions-To and Resolution header

* Fix Discussions-To link
@methane

methane commented May 13, 2022

Copy link
Copy Markdown
Member Author

Congratulations for finally managing to remove this legacy API. A big thank you! I hated this API since Python 3.3, having to call "PyUnicode_Ready()" was super annoying. Thanks for you tenacity to remove it!

I'm sorry but I am considering to un-deprecate PyUnicode_READY().
Please visit this thread.
https://discuss.python.org/t/undeprecate-pyunicode-ready-for-future-unicode-improvement/15717

All reactions

Sorry, something went wrong.

Uh oh!

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

This was referenced May 13, 2022
[BUG] py312 compatibility cython/cython#4788
Closed
MNT: switch how to check for removal of unicode machinery cython/cython#4789
Closed
@ericsnowcurrently ericsnowcurrently mentioned this pull request Sep 1, 2022
_PyStaticUnicode_Dealloc should not exist. #96458
Closed
@mattip mattip mentioned this pull request Nov 25, 2022
PyUnicode_AsUnicode is deprecated, slated for removal in 3.12 cython/cython#5149
Closed
@methane
methane deleted the remove-unicode-wchar branch April 20, 2023 14:10
erlend-aasland pushed a commit to erlend-aasland/devguide that referenced this pull request Sep 8, 2023
@methane @erlend-aasland
python/cpython#92536: PEP 623: Remove wstr and legacy APIs from Unico…
e1df607
…de (python/cpython#92537)
AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request Sep 13, 2023
@methane
python/cpython#92536: PEP 623: Remove wstr and legacy APIs from Unico…
03d39ac
…de (python/cpython#92537)
erlend-aasland pushed a commit to python/devguide that referenced this pull request Sep 26, 2023
@methane @erlend-aasland
python/cpython#92536: PEP 623: Remove wstr and legacy APIs from Unico…
09f1afa
…de (python/cpython#92537)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ericsnowcurrently ericsnowcurrently ericsnowcurrently left review comments
@arhadthedev arhadthedev arhadthedev left review comments
@slateny slateny slateny left review comments
@iritkatriel iritkatriel Awaiting requested review from iritkatriel
@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

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.

7 participants

@methane @serhiy-storchaka @arhadthedev @vstinner @ericsnowcurrently @slateny @bedevere-bot
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.