Issue25971
Created on 2015-12-28 22:48 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| fraction_from_floating.patch | serhiy.storchaka, 2015-12-28 22:48 | review | ||
| Messages (12) | |||
|---|---|---|---|
| msg257145 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2015-12-28 22:48 | |
Proposed patch makes following things: 1. Rewrite error messages in float.as_integer_ratio() and Python implementation of Decimal.as_integer_ratio() in more general form, not mentioning as_integer_ratio(), as in C implementation of Decimal.as_integer_ratio(). 2. Use Decimal.as_integer_ratio() to convert Decimal to Fraction. 3. Get rid of additional checks in Fraction constructor that raise errors with appropriate messages, since new error messages from as_integer_ratio() are appropriate enough. This speeds up creating a Fraction from float and Decimal 2 to 3 times. |
|||
| msg257201 - (view) | Author: Mark Dickinson (mark.dickinson) * ![]() |
Date: 2015-12-29 17:46 | |
Any particular reason for the lower-casing of "Cannot" to "cannot" in the exception messages? Otherwise, LGTM. |
|||
| msg257205 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2015-12-29 18:47 | |
> Any particular reason for the lower-casing of "Cannot" to "cannot" in the exception messages? Only for matching current messages in C implementation of Decimal.as_integer_ratio(). As well as non-distinguishing positive and negative infinities, NaN and sNaN. If this is desirable, exception messages in C implementation of Decimal.as_integer_ratio() should be changed too. |
|||
| msg257206 - (view) | Author: Stefan Krah (skrah) * ![]() |
Date: 2015-12-29 18:57 | |
Both versions are fine with me. The lowercase "cannot do ..." is more pervasive in the source tree: $ grep -R '"cannot' | wc -l 293 $ grep -R '"Cannot' | wc -l 150 If we change it, let's change all occurrences in _pydecimal and _decimal/docstrings.h to uppercase (e.g. in __floor__, __round__, __ceil__). |
|||
| msg257207 - (view) | Author: Stefan Krah (skrah) * ![]() |
Date: 2015-12-29 19:13 | |
From my non-native speaker's point of view, I'd use lowercase if the sentence doesn't end with a full stop, uppercase otherwise. When I started here I was told that error messages in Python usually don't have a full stop. ;) |
|||
| msg257208 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2015-12-29 19:14 | |
Yes, this already was discussed in issue22364. The lowercase form wins with the score 4:1 (see msg226790, error messages use not only double quotes). |
|||
| msg257209 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2015-12-29 19:17 | |
The question is wherever we should distinguish different sorts of infinities and NaNs (I think this is not needed). |
|||
| msg257211 - (view) | Author: Stefan Krah (skrah) * ![]() |
Date: 2015-12-29 19:47 | |
Sorry for the bikeshedding, but I find this interesting: Poly/ML (Cambridge), ghci (Glasgow) and OCaml (INRIA) appear to use error messages without a full stop but starting in uppercase. SML/NJ (Bell Labs) uses lowercase (and no full stop). Perhaps this is a British/European vs. American issue? Regarding int/-inf: I don't think it's important to distiguish between them. |
|||
| msg257212 - (view) | Author: R. David Murray (r.david.murray) * ![]() |
Date: 2015-12-29 19:55 | |
I wonder if it is a (programming) language specific thing. On the other hand, I don't know what those langauges error messages look like, but Python's normally follow a colon (ValueError: ....) where not having the message capitalized is more natural (whether or not there is a full stop at the end), at least in American English. |
|||
| msg257213 - (view) | Author: Stefan Krah (skrah) * ![]() |
Date: 2015-12-29 20:16 | |
Serhiy: to me, the patch also looks good, we can certainly change any error messages later. Back to the bikeshedding: Poly/ML: ======== > f; Error-Value or constructor (f) has not been declared Found near f Static errors (pass2) ghci ==== Prelude> f <interactive>:2:1: Not in scope: `f' OCaml ===== # f;; Error: Unbound value f SML/NJ ====== - f; stdIn:1.2 Error: unbound variable or constructor: f Basically the European languages start in uppercase after the first relevant colon (or hyphen in the case of Poly/ML). |
|||
| msg257215 - (view) | Author: Roundup Robot (python-dev) ![]() |
Date: 2015-12-29 20:34 | |
New changeset 284026a8af9e by Serhiy Storchaka in branch 'default': Issue #25971: Optimized creating Fractions from floats by 2 times and from https://hg.python.org/cpython/rev/284026a8af9e |
|||
| msg257216 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * ![]() |
Date: 2015-12-29 21:10 | |
Thank you all for your review. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:25 | admin | set | github: 70159 |
| 2015-12-29 21:10:08 | serhiy.storchaka | set | status: open -> closed messages: + msg257216 assignee: serhiy.storchaka |
| 2015-12-29 20:34:47 | python-dev | set | nosy:
+ python-dev messages: + msg257215 |
| 2015-12-29 20:16:05 | skrah | set | messages: + msg257213 |
| 2015-12-29 19:55:38 | r.david.murray | set | nosy:
+ r.david.murray messages: + msg257212 |
| 2015-12-29 19:47:04 | skrah | set | messages: + msg257211 |
| 2015-12-29 19:17:42 | serhiy.storchaka | set | messages: + msg257209 |
| 2015-12-29 19:14:32 | serhiy.storchaka | set | messages: + msg257208 |
| 2015-12-29 19:13:38 | skrah | set | messages: + msg257207 |
| 2015-12-29 18:57:29 | skrah | set | messages: + msg257206 |
| 2015-12-29 18:47:19 | serhiy.storchaka | set | messages: + msg257205 |
| 2015-12-29 17:46:30 | mark.dickinson | set | messages: + msg257201 |
| 2015-12-28 22:48:17 | serhiy.storchaka | create | |

