-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
locale.atof documentation is missing func argument #58126
Comments
atof has a func argument used to instantiate the result but it is missing in the documentation. |
I don't think that argument needs to be documented. It's just there because somebody thought that copying 3 lines from atof into atoi was a bad idea. |
Indeed I find it useful to use to get a Decimal instead of a float. |
It looks like an implementation detail to me, so I tend to agree with Georg. I'm not sure if this should be noted in the code though. |
Then I think we miss a locale.atod to parse string to Decimal |
I agree with "won't fix" for the original issue. These locale functions For decimal locale specific formatting, use: format(Decimal("1729.1415927"), "n") IOW, I don't think new formatting functions should be added to the |
locale.atof is not about formatting but parsing string into float following the locale. |
Cédric Krier <report@bugs.python.org> wrote:
You're right. Sorry, I never use these locale functions. My impression is So actually I now agree that making the parameter official is one reasonable |
Here is a patch for the documentation. |
I find the idea of intentionally not documenting a public parameter and the full signature of a function somewhat strange, especially when it is already automatically partially-documented. >>> import locale
>>> help(locale.atof)
Help on function atof in module locale: atof(string, func=<class 'float'>)
Parses a string as a float according to the locale settings.
# 2.7, 3.2, 3.3 Not documenting the full signature of a function seems to me contrary to proper policy. That aside, the func parameter is, to me, a useful feature, not just an implementation detail The way to have factored out the common normalization without a func parameter is obvious: define a private normalization function. def _anormalize(string):
"remove thousands separators, make decimal dot"
ts = localeconv()['thousands_sep']
if ts:
string = string.replace(ts, '')
#next, replace the decimal point with a dot
dd = localeconv()['decimal_point']
if dd:
string = string.replace(dd, '.')
return string
def atof(string):
"Parses a string as a float according to the locale settings."
return float(_anormalize(string))
def atoi(string): # changed from str
"Converts a string to an integer according to the locale settings."
return int(_anormalize(string)) But Martin von Loewis, the original author did not do this. I would not assume that he "thought that copying 3 lines from atof into atoi was a bad idea." without asking him. Whatever his conscious intention, the func parameter *does* have the advantage of allowing alternate float string to number converters. We now have another one in the stdlib besides decimal.Decimal: fractions.Fractions. >>> locale.atof('99,999.99', F)
Fraction(9999999, 100)
# versus
>>> F(locale.atof('99,999.99'))
Fraction(6871946986405233, 68719476736) There are also 3rd party float implementations, such as indefinite precision binary floats. Does anyone still object to properly documenting this useful feature? I am willing to do the commits. As to the patch and atof docstring, I thinks 'converts' (used in atoi docstring) is better than 'parses'. So I would change both. |
The function was introduced by Guido in f5b55311e79d. I think it would have been better if atof had another name (e.g. _atof) and that atof and atoi were implemented as: def atof(str):
return _atof(str, float)
def atoi(str):
return _atof(str, int) Even better would have been to have _atof return the string without applying any function, and let e.g. atoi return int(_atof(str)). This could have been documented publicly and used to build other functions. |
The refactoring could be done if we were willing to give the normalize function a public name, so people could write Decimal(delocalize(<localized float string>)) or if we were willing to add atod and atofr (fraction). However, simply adding a few words to the doc is a lot easier. |
It's easier, but we will be exposing an API that is not too elegant IMHO. The refactoring could provide a better API, but OTOH it will make it available for 3.4+ only, and it would break backward compatibility if the old API is removed (even though it's not documented, so in theory we could do that). |
So what about this patch? |
The patch's approach looks reasonable to me. |
A new version with unittest. |
Thanks for the updated patch, Cédric. Just some remarks:
|
Add return value is string in doc |
Thank you! The patch looks good to me, I'm going to apply it. |
New changeset aee097e5a2b2 by Antoine Pitrou in branch 'default': |
Done. Thank you for your contribution! |
+ :const:'LC_NUMERIC`settings. a space is missing before "settings", no? |
Ah, right, thank you. |
And the first quote is wrong. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: