Skip to content

gh-92536: PEP 623: Remove wstr from unicode - #92537

Merged
methane merged 21 commits into
python:mainfrom
methane:remove-unicode-wchar
May 12, 2022
Merged

gh-92536: PEP 623: Remove wstr from unicode#92537
methane merged 21 commits into
python:mainfrom
methane:remove-unicode-wchar

Conversation

@methane

Copy link
Copy Markdown
Member

@methane
methaneforce-pushed the remove-unicode-wchar branch from eda0fb0 to d5a14c2CompareMay 9, 2022 08:26
Comment threadDoc/c-api/unicode.rst
.. versionadded:: 3.3

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

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

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

@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 ownersMay 10, 2022 03:40
Comment threadDoc/whatsnew/3.12.rst
Co-authored-by: Oleg Iarygin <dralife@yandex.ru>
@methane

Copy link
Copy Markdown
MemberAuthor

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

@ericsnowcurrentlyericsnowcurrently left a comment

Copy link
Copy Markdown
Member

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?

@serhiy-storchaka

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.

@arhadthedev

arhadthedev commented May 11, 2022

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})``

@methane

Copy link
Copy Markdown
MemberAuthor

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.

@methane
methane merged commit f9c9354 into python:mainMay 12, 2022
methane added a commit to methane/peps that referenced this pull request May 12, 2022

@slatenyslateny left a comment

Copy link
Copy Markdown
Contributor

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

Comment threadDoc/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*``

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
MemberAuthor

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

Comment threadDoc/c-api/unicode.rst
.. versionadded:: 3.3

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

Copy link
Copy Markdown
Contributor

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

@vstinner

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!

methane added a commit to python/peps that referenced this pull request May 13, 2022
* PEP 623: Mark it final
Implemented in python/cpython#92537
* Add Discussions-To and Resolution header
* Fix Discussions-To link
@methane

Copy link
Copy Markdown
MemberAuthor

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@methane@serhiy-storchaka@arhadthedev@vstinner@ericsnowcurrently@slateny@bedevere-bot