Skip to content

gh-76535: Make PyUnicode_ToLowerFull and friends public #136176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented Jul 1, 2025

Make _PyUnicode_ToLowerFull, _PyUnicode_ToUpperFull, _PyUnicode_ToTitleFull and _PyUnicode_ToFoldedFull public and rename them to PyUnicode_ToLower etc.


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

Make `PyUnicode_ToLowerFull`, `PyUnicode_ToUpperFull` and
`PyUnicode_ToTitleFull` public and rename them to `PyUnicode_ToLower`
etc.
@lysnikolaou
Copy link
Member Author

Thanks for taking a look @vstinner! Feedback addressed.

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

In #76535 (comment) @vstinner suggested to provide a constant which is the minimum buffer size.

If this is indeed a hard constant which will never be changed in future Unicode standards, then I prefer this way. It is too expensive to allocate the output buffer dynamically.

cc @ezio-melotti, our Unicode expert.

@lysnikolaou
Copy link
Member Author

Another question I have is whether we want to expose something like the following to handle the Greek letter sigma edge case:

int PyUnicode_ToLowerHandleSigma(Py_UCS4 *str, Py_UCS4 ch, Py_UCS4 *buffer, int size)

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.

Thanks, I prefer this API which is more future-proof, it doesn't depend on a specific Unicode version.

@vstinner
Copy link
Member

vstinner commented Jul 1, 2025

PyUnicode_ToLowerHandleSigma

Would you mind to elaborate? I'm not aware of this special case.

@vstinner
Copy link
Member

vstinner commented Jul 1, 2025

If this is indeed a hard constant which will never be changed in future Unicode standards

Even if it's a constant which will never (!) change, IMO it's better to request a size as an argument to make the caller responsible to check the buffer size. APIs which accept a pointer with no size are a bad pattern, like the deprecated gets() function.

@lysnikolaou
Copy link
Member Author

Further feedback addressed.

Would you mind to elaborate? I'm not aware of this special case.

There's one special case, the Greek letter sigma, where the result of lower casing is context-specific. More specifically, Σ gets lower-cased to ς if it's at the end of the word or to σ otherwise. This is handled in lower_ucs4 right now.

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.

Can you try to add tests to Modules/_testcapi/unicode.c and Lib/test/test_capi/test_unicode.py?

@vstinner
Copy link
Member

vstinner commented Jul 1, 2025

There's one special case, the Greek letter sigma, where the result of lower casing is context-specific. More specifically, Σ gets lower-cased to ς if it's at the end of the word or to σ otherwise.

Oh, that's a tricky case. Proposed API takes a single character, so we don't know if Σ is at the end of a word or not. I don't think that it's worth it to handle this special case in proposed API.

@lysnikolaou
Copy link
Member Author

Can you try to add tests to Modules/_testcapi/unicode.c and Lib/test/test_capi/test_unicode.py?

Done.

@serhiy-storchaka
Copy link
Member

If we add too many parameters and runtime checks, this will make the API slower and more difficult to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants