gh-123803: Support arbitrary code page encodings on Windows#123804
Conversation
If the cpXXX encoding is not directly implemented in Python, fall back to use the Windows-specific API codecs.code_page_encode() and codecs.code_page_decode().
serhiy-storchaka
force-pushed
the
code-page-codecs
branch
from
64d487b to
c9a5861
Compare
zooba
left a comment
zooba
left a comment
There was a problem hiding this comment.
This change looks fine, and I can see now that it's not so trivial to raise a different exception. Maybe we can improve the current "unknown encoding" exception string to say "encoding '{name}' is not registered" or something that suggests it might be possible to fix by adding the encoding? I expect many users would think that encodings are static.
|
When you're done making the requested changes, leave the comment: |
serhiy-storchaka
requested review from
lysnikolaou and
pablogsal
as code owners
|
I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @malemburg: please review the changes made to this pull request. |
malemburg
left a comment
malemburg
left a comment
There was a problem hiding this comment.
Thanks, @serhiy-storchaka
This looks good now.
|
Thank you for your review @zooba and @malemburg. I do not particularly like the change in the error message. Can we do without it? |
|
I guess if people would rather wait and see if the difference in availability causes confusion, then we can do that. I'd prefer to be more clear in the docs, as I don't think developers or users currently expect codecs to be platform-specific like this. |
How about "encoding XYZ not available" ?! I'd also be fine with leaving the current error message in place. The "not registered" is not quite correct, since it assumes that there is a codec with that name available somewhere, it's just not registered. This is not always the case, though, e.g. if you mistype an encoding name. |
|
"Not registered" at least implies that there's a potential way to fix it, whereas "not available" suggests the user has to come to us to ask us to make it available. (I'd assume in this case they wouldn't read our docs either.) "Not available on this platform" would be okay, but feels less accurate overall, since as you say a mistyped codec isn't going to be available on any platform. We can't seem to customise the message for a known-but-not-present codec though, so it's a bit of a tough spot. |
This reverts commit 57ad6d2.
|
Thanks, @serhiy-storchaka |
…thonGH-123804) If the cpXXX encoding is not directly implemented in Python, fall back to use the Windows-specific API codecs.code_page_encode() and codecs.code_page_decode().
3 participants
If the cpXXX encoding is not directly implemented in Python, fall back to use the Windows-specific API codecs.code_page_encode() and codecs.code_page_decode().