Skip to content

gh-124969: Make locale.nl_langinfo(locale.ALT_DIGITS) returning a string again #125774

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

Merged

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Oct 21, 2024

This is a follow up of GH-124974. Only Glibc needed a fix. Now the returned value is a string consisting of semicolon-separated symbols on all Posix platforms.


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

… a string again

This is a follow up of pythonGH-124974. Only Glibc needed a fix.
Now the returned value is a string consisting of semicolon-separated
symbols on all Posix platforms.
@serhiy-storchaka
Copy link
Member Author

@kulikjak, could you please test that the modified test is passed on Solaris?

@kulikjak
Copy link
Contributor

Hi @serhiy-storchaka, thanks!

I tested the patch, and locale.nl_langinfo(locale.ALT_DIGITS) returns the same thing as before (which was the goal), and the tests now pass.

0:00:00 load avg: 1.98 [1/1] test__locale
test_alt_digits_nl_langinfo (test.test__locale._LocaleTests.test_alt_digits_nl_langinfo) ... 
  test_alt_digits_nl_langinfo (test.test__locale._LocaleTests.test_alt_digits_nl_langinfo) (locale='ja_JP') ... skipped "ALT_DIGITS is not set for locale 'ja_JP' on this platform"
  test_alt_digits_nl_langinfo (test.test__locale._LocaleTests.test_alt_digits_nl_langinfo) (locale='or_IN') ... skipped "ALT_DIGITS is not set for locale 'or_IN' on this platform"
test_float_parsing (test.test__locale._LocaleTests.test_float_parsing) ... ok
test_lc_numeric_basic (test.test__locale._LocaleTests.test_lc_numeric_basic) ... ok
test_lc_numeric_localeconv (test.test__locale._LocaleTests.test_lc_numeric_localeconv) ... ok
test_lc_numeric_nl_langinfo (test.test__locale._LocaleTests.test_lc_numeric_nl_langinfo) ... ok

There is currently no locale in known_alt_digits that has ALT_DIGITS set on Solaris, so I added the following two:

    'ar_AE.UTF-8': (100, {0: '\u0660', 10: '\u0661\u0660', 99: '\u0669\u0669'}),
    'bn_IN.UTF-8': (100, {0: '\u09e6', 10: '\u09e7\u09e6', 99: '\u09ef\u09ef'}),

(and adjusted those two in candidate_locales to include the *.UTF-8 suffix).

@serhiy-storchaka
Copy link
Member Author

Is not ar_AE equivalent to ar_AE.UTF-8?

@kulikjak
Copy link
Contributor

Is not ar_AE equivalent to ar_AE.UTF-8?

Unfortunately not. Mostly due to historical reasons, default locales on Solaris are not UTF-8. And I guess that for similar reasons, only the UTF-8 variants of these two include ALT_DIGITS.

@kulikjak
Copy link
Contributor

Uh, so with this second change I found that 'ja_JP.PCK' on Solaris returns 101 symbols. At least those three tested match, which is nice.

The standard allows max 100, but considering that the last value is a new symbol (and not just a concatenation of those before), I can see why it is there. That said, I propose removing 'ja_JP.PCK' from tested locales (I don't think PCK is available on other platforms anyway).

@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) October 21, 2024 18:33
@serhiy-storchaka
Copy link
Member Author

Thank you for your help @kulikjak.

@serhiy-storchaka serhiy-storchaka merged commit dcc4fb2 into python:main Oct 21, 2024
38 checks passed
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 21, 2024
… a string again (pythonGH-125774)

This is a follow up of pythonGH-124974. Only Glibc needed a fix.
Now the returned value is a string consisting of semicolon-separated
symbols on all Posix platforms.
(cherry picked from commit dcc4fb2)

Co-authored-by: Serhiy Storchaka <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 21, 2024
… a string again (pythonGH-125774)

This is a follow up of pythonGH-124974. Only Glibc needed a fix.
Now the returned value is a string consisting of semicolon-separated
symbols on all Posix platforms.
(cherry picked from commit dcc4fb2)

Co-authored-by: Serhiy Storchaka <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2024

GH-125804 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 21, 2024
@bedevere-app
Copy link

bedevere-app bot commented Oct 21, 2024

GH-125805 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 21, 2024
serhiy-storchaka added a commit that referenced this pull request Oct 21, 2024
…g a string again (GH-125774) (GH-125805)

This is a follow up of GH-124974. Only Glibc needed a fix.
Now the returned value is a string consisting of semicolon-separated
symbols on all Posix platforms.
(cherry picked from commit dcc4fb2)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this pull request Oct 21, 2024
…g a string again (GH-125774) (GH-125804)

This is a follow up of GH-124974. Only Glibc needed a fix.
Now the returned value is a string consisting of semicolon-separated
symbols on all Posix platforms.
(cherry picked from commit dcc4fb2)

Co-authored-by: Serhiy Storchaka <[email protected]>
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
… a string again (pythonGH-125774)

This is a follow up of pythonGH-124974. Only Glibc needed a fix.
Now the returned value is a string consisting of semicolon-separated
symbols on all Posix platforms.
Sign up for free to 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.

2 participants