gh-134887: Add references to locale module for locale-aware number formatting references in string module docs#134888
gh-134887: Add references to locale module for locale-aware number formatting references in string module docs#134888stefmolin wants to merge 11 commits into
locale module for locale-aware number formatting references in string module docs#134888Conversation
locale module for locale-aware number formatting references in string module docslocale module for locale-aware number formatting references in string module docs
locale module for locale-aware number formatting references in string module docslocale module for locale-aware number formatting references in string module docs
ericvsmith
left a comment
There was a problem hiding this comment.
I'm not sure we need to add a reference to setting the locale, since the text already says it's local aware.
| | | 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 | |
There was a problem hiding this comment.
This isn't true, is it? I'm referring to the wording "must set": shouldn't it be "can set".
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Indeed, LC_NUMERIC is usually C, but I think using "must" is a little too absolute.
There was a problem hiding this comment.
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."
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 When I tried the 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 |
|
I can reproduce this issue as well, and can confirm that even though my system has |
|
This PR is stale because it has been open for 30 days with no activity. |
Documentation build overview
115 files changed ·
|
willingc
left a comment
There was a problem hiding this comment.
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.
| 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')``. |
There was a problem hiding this comment.
Maybe clarify this is not portable? Users may end up confused when it fails with a localeError.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:localemodule.
| | | 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 | |
There was a problem hiding this comment.
Indeed, LC_NUMERIC is usually C, but I think using "must" is a little too absolute.
|
I'm not sure why the CI run failed before, but it passed on a re-run. |
|
@willingc : no objections from me. |
Add references to
localemodule for locale-aware number formatting references instringmodule docs to avoid users thinking that the locale doesn't have a thousands separator for example.nin the format-specifier should reference locale-setting #134887