gh-76535: Make PyUnicode_ToLowerFull and friends public
#136176
Conversation
Make `PyUnicode_ToLowerFull`, `PyUnicode_ToUpperFull` and `PyUnicode_ToTitleFull` public and rename them to `PyUnicode_ToLower` etc.
|
Thanks for taking a look @vstinner! Feedback addressed. |
There was a problem hiding this comment.
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.
|
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) |
Would you mind to elaborate? I'm not aware of this special case. |
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. |
|
Further feedback addressed.
There's one special case, the Greek letter sigma, where the result of lower casing is context-specific. More specifically, |
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. |
Done. |
|
If we add too many parameters and runtime checks, this will make the API slower and more difficult to use. |
| Convert *ch* to lower case, store result in *buffer*, which should be | ||
| able to hold as many characters needed for *ch* to be lower cased | ||
| (e.g. a maximum of two character for Unicode 16.0), and | ||
| return the number of characters stored. Passing a ``NULL`` buffer returns |
There was a problem hiding this comment.
Is it an important usecase for you to be able to pass NULL buffer to get the buffer size? Removing this feature would make the implementation simpler.
There was a problem hiding this comment.
No, it's not an important usecase. I implemented because it was suggested on the issue and it also seems like standard practice for some C functions that accept buffers.
There was a problem hiding this comment.
Do you think I should remove it?
There was a problem hiding this comment.
I don't think that someone will call the function with NULL, allocate a buffer on the heap memory with the result (size), call again the function, and then release the buffer. It sounds complicated just to transform a single character.
Just overallocate 10 characters on the stack memory, call the function once, that's it. You get the size as well. If you don't need characters, just ignore them.
| assert(chars >= 1); | ||
| Py_UCS4 buf[3]; | ||
| int chars = function(c, buf, Py_ARRAY_LENGTH(buf)); | ||
| if (chars <= 0) { |
There was a problem hiding this comment.
| if (chars <= 0) { | |
| if (chars < 0) { |
There was a problem hiding this comment.
0 is also an unexpected result here that shouldn't be happening.
Make
_PyUnicode_ToLowerFull,_PyUnicode_ToUpperFull,_PyUnicode_ToTitleFulland_PyUnicode_ToFoldedFullpublic and rename them toPyUnicode_ToLoweretc.📚 Documentation preview 📚: https://cpython-previews--136176.org.readthedocs.build/