Replace locale.getpreferredencoding() with locale.getencoding()#13960
Replace locale.getpreferredencoding() with locale.getencoding()#13960blackwell-systems wants to merge 3 commits into
Conversation
Use locale.getencoding() (available since Python 3.11) to avoid the EncodingWarning raised under UTF-8 Mode on Python 3.15+. A compat helper in pip._internal.utils.compat falls back to locale.getpreferredencoding(False) on Python 3.10. Fixes pypa#13922
|
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
sepehr-rs
left a comment
There was a problem hiding this comment.
Hi @blackwell-systems, thank you for your contribution!
I reviewed your code, and overall it looks good. I also tested it locally, and the warning no longer seems to be emitted. I do have a few suggestions and requests, though. Please let me know if you have any questions!
| @@ -0,0 +1,2 @@ | |||
| Replace ``locale.getpreferredencoding()`` with ``locale.getencoding()`` | |||
| (Python 3.11+) to avoid ``EncodingWarning`` under UTF-8 Mode on Python 3.15+. | |||
There was a problem hiding this comment.
I think the changelog wording could be a bit more precise here:
| (Python 3.11+) to avoid ``EncodingWarning`` under UTF-8 Mode on Python 3.15+. | |
| (Python 3.11+) to avoid ``EncodingWarning`` under UTF-8 Mode. |
The warning isn't strictly a Python 3.15+ phenomenon. It becomes much more visible there because of the new warning, but PYTHONWARNDEFAULTENCODING and UTF-8 mode can already expose this behavior on earlier versions.
Also, since this is being implemented through a compatibility wrapper, it might be worth wording the entry in a way that reflects that, rather than implying a direct replacement of every locale.getpreferredencoding() call with locale.getencoding().
| ``EncodingWarning`` that ``locale.getpreferredencoding(False)`` raises | ||
| under UTF-8 Mode in Python 3.15+. | ||
| """ | ||
| if sys.version_info >= (3, 11): |
There was a problem hiding this comment.
Minor note: Once pip drops support for Python 3.10, this check will become redundant. Adding a TODO here for future removal would be helpful.
|
|
||
| Use ``locale.getencoding()`` when available (Python 3.11+) to avoid the | ||
| ``EncodingWarning`` that ``locale.getpreferredencoding(False)`` raises | ||
| under UTF-8 Mode in Python 3.15+. |
There was a problem hiding this comment.
I think we don't need the "Python 3.15+" part here either.
| under UTF-8 Mode in Python 3.15+. | |
| under UTF-8 Mode. |
| def test_unicode_decode_error(caplog: pytest.LogCaptureFixture) -> None: | ||
| if locale.getpreferredencoding() != "UTF-8": | ||
| pytest.skip("locale.getpreferredencoding() is not UTF-8") | ||
| from pip._internal.utils.compat import locale_getencoding |
There was a problem hiding this comment.
Any reason for the inline import here? If not, let’s move it to the top.
| return IS_PYOPENSSL | ||
|
|
||
|
|
||
| def locale_getencoding() -> str: |
There was a problem hiding this comment.
Minor naming nit: locale_getencoding initially looks like a direct alias of locale.getencoding(), but it's actually a compatibility wrapper. I think something like get_locale_encoding would be a clearer name.
| # Needed for locale.getpreferredencoding(False) to work | ||
| # Needed for locale encoding detection to work | ||
| # in pip._internal.utils.encoding.auto_decode | ||
| try: |
There was a problem hiding this comment.
If the eventual goal is to remove the compatibility fallback once Python 3.10 support is dropped, perhaps we should leave a TODO here to reevaluate whether this initialization is still needed as well.
There was a problem hiding this comment.
Can we also add a regression test to verify that running pip with PYTHONUTF8=1 and PYTHONWARNDEFAULTENCODING=1 no longer emits the warning?
Summary
Fixes #13922.
locale.getpreferredencoding()emitsEncodingWarningon Python 3.15 when UTF-8 Mode is enabled. This replaces all internal uses with a compat helper that callslocale.getencoding()on Python 3.11+ and falls back tolocale.getpreferredencoding(False)on Python 3.10.The second warning from the issue (
os.path.commonprefixinauth.py) was already fixed in #13847.Changes
src/pip/_internal/utils/compat.py: Addedlocale_getencoding()helper with version-gated dispatch. Follows the same pattern as the existingtomllibandopen_text_resourcecompat shims in the same file.src/pip/_internal/configuration.py: Replacedlocale.getpreferredencoding(False)withlocale_getencoding().src/pip/_internal/req/req_file.py: Same replacement.src/pip/_internal/commands/debug.py: Updated debug output label and uses the new helper.src/pip/_internal/cli/main.py: Updated comment referencing the old function name.test_debug.py,test_req_file.py,test_utils_subprocess.py.news/13922.bugfix: Towncrier news fragment.Compatibility
locale.getencoding()was added in Python 3.11. pip supports Python >= 3.10, so the helper gates onsys.version_info >= (3, 11). On 3.10, the oldlocale.getpreferredencoding(False)is used (no warning on 3.10).Test plan
All 4 affected unit tests pass locally. The functional
test_debug.pyassertion updated for the new label.