Skip to content

gh-134887: Add references to locale module for locale-aware number formatting references in string module docs#134888

Open
stefmolin wants to merge 11 commits into
python:mainfrom
stefmolin:patch-1
Open

gh-134887: Add references to locale module for locale-aware number formatting references in string module docs#134888
stefmolin wants to merge 11 commits into
python:mainfrom
stefmolin:patch-1

Conversation

@stefmolin
Copy link
Copy Markdown
Contributor

@stefmolin stefmolin commented May 29, 2025

Add references to locale module for locale-aware number formatting references in string module docs to avoid users thinking that the locale doesn't have a thousands separator for example.

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot Bot commented May 29, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir skip news labels May 29, 2025
@github-project-automation github-project-automation Bot moved this to Todo in Docs PRs May 29, 2025
@stefmolin stefmolin changed the title Add references to locale module for locale-aware number formatting references in string module docs gh-134888: Add references to locale module for locale-aware number formatting references in string module docs May 29, 2025
@stefmolin stefmolin marked this pull request as ready for review May 29, 2025 14:15
@stefmolin stefmolin changed the title gh-134888: Add references to locale module for locale-aware number formatting references in string module docs gh-134887: Add references to locale module for locale-aware number formatting references in string module docs May 29, 2025
Comment thread Doc/library/string.rst Outdated
Comment thread Doc/library/string.rst Outdated
Copy link
Copy Markdown
Member

@ericvsmith ericvsmith left a comment

Choose a reason for hiding this comment

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

I'm not sure we need to add a reference to setting the locale, since the text already says it's local aware.

Comment thread Doc/library/string.rst Outdated
Comment thread Doc/library/string.rst Outdated
| | digit group separators. |
| | digit group separators. Note that the default locale is |
| | not the system locale, and therefore, you must set, at a |
| | minimum, the ``LC_NUMERIC`` category (see the |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This isn't true, is it? I'm referring to the wording "must set": shouldn't it be "can set".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On my machine, nothing happens if I use n, without setting LC_NUMERIC, so it is "must set" for me. I'm happy to adjust the wording if there are cases where it is set, but I haven't seen it.

FWIW, I'm seeing that the C locale seems to be very minimal and doesn't have a thousands separator. Since that is what the default is, instead of the system locale, the thousands separator won't be set until the user sets LC_NUMERIC, meaning it is "must set" for everyone. I'm new to the C locale as well though, so let me know if that isn't the case.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed, LC_NUMERIC is usually C, but I think using "must" is a little too absolute.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that is a fair assessment on "must". @stefmolin I think that breaking this into two sentences. The first that "Note that the system locale is not the default locale." And the second, "Depending on your use case, you may wish to set LC_NUMERIC."

@stefmolin
Copy link
Copy Markdown
Contributor Author

I'm not sure we need to add a reference to setting the locale, since the text already says it's local aware.

While this may be obvious to a core CPython developer, it was not obvious to me or my friend, who is a Python trainer. We have each been using Python for many years. My friend routinely explores parts of the Python documentation to learn new things and share with the community. He found out about the n option and shared it on Bluesky, mentioning that the thousands separator may be nothing in your locale. I was excited to try this because I use the comma in the format specifier all the time and sometimes when I teach or show code snippets using this in places that use a different thousands separator, I get asked about this.

When I tried the n option, however, it didn't do anything on my machine, which I was shocked to see. I then read through the docs and couldn't see why it wouldn't work. The string module docs lack any reference to the locale module, which at this point I didn't even know existed. I searched on the Internet to see how I could check if my locale was correct, which led me to the locale module, and I was able to confirm it was set as I would expect. Since the locale was indeed set and the docs say that n provides locale-aware formatting, I was confused why it wasn't working.

I searched some more to discover why it wasn't using a thousands separator for the format. This time Stack Overflow led me to the real reason behind it not working: I needed to set the LC_NUMERIC category. I seriously doubt that my friend and I are the only people that don't find this obvious, so I made a PR in the hopes that the next person can figure out how to get this working while reading the docs directly, instead of having to troubleshoot it for a while (or just assuming it doesn't work).

@cmaureir
Copy link
Copy Markdown
Contributor

I can reproduce this issue as well, and can confirm that even though my system has LC_NUMERIC the formatting with :n for f-strings doesn't properly work without setting the locale like: locale.setlocale(locale.LC_NUMERIC, 'en_US')

@github-actions
Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 25, 2026
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label May 15, 2026
@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community Bot commented May 17, 2026

Copy link
Copy Markdown
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

Thanks @stefmolin for the PR. I've restarted CI as we work through the PR together.

Hi @ericvsmith and @StanFromIreland, I'm mentoring Stefanie. She had a great chat with Guido at PyCon the other night. For a bit of background, she's given a bunch of tutorials at PyCons around the world and works in Bloomberg's security group.

Unless I hear any strong objections, I am going to approve and merge this PR since it is helpful.

Comment thread Doc/library/string.rst Outdated
For a locale-aware separator, use the ``'n'`` presentation type instead.
Note that the locale setting for numeric values must first be set using
the :mod:`locale` module, for example,
``locale.setlocale(locale.LC_NUMERIC, 'en_US')``.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe clarify this is not portable? Users may end up confused when it fails with a localeError.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good point @StanFromIreland.

See also https://docs.python.org/3/library/locale.html#background-details-hints-tips-and-caveats @stefmolin:

It is generally a bad idea to call setlocale() in some library routine, since as a side effect it affects the entire program. Saving and restoring it is almost as bad: it is expensive and affects other threads that happen to run before the settings have been restored.

I think the best approach would be to remove the Note.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Are there any objections to at least including a reference to LC_NUMERIC or the locale module docs at this point? The original reason I put this here is that I had searched the entire page for "locale aware" and "locale-aware" to find the format specifier in question, and this was the only hit in the entire page. If we remove any reference to the locale here (the page's original state), then, in my opinion, we don't fully address the problem.

Perhaps, something like this?

For a locale-aware separator, use the 'n' presentation type instead. See also the :mod:locale module.

Comment thread Doc/library/string.rst Outdated
| | digit group separators. |
| | digit group separators. Note that the default locale is |
| | not the system locale, and therefore, you must set, at a |
| | minimum, the ``LC_NUMERIC`` category (see the |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Indeed, LC_NUMERIC is usually C, but I think using "must" is a little too absolute.

@StanFromIreland
Copy link
Copy Markdown
Member

I'm not sure why the CI run failed before, but it passed on a re-run.

@encukou encukou added sprint needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 18, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Sprint May 18, 2026
@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Sprint May 18, 2026
@ericvsmith
Copy link
Copy Markdown
Member

@willingc : no objections from me.

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

Labels

awaiting merge docs Documentation in the Doc dir needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes skip news sprint

Projects

Status: Todo
Status: In Progress

Development

Successfully merging this pull request may close these issues.

7 participants