Skip to content
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

Corrections to format precision description. #31291

Merged
merged 2 commits into from Feb 14, 2022

Conversation

@belm0
Copy link
Contributor

@belm0 belm0 commented Feb 12, 2022

  • precision field is a decimal integer
  • clarify that stated limitations are on presentation type
    rather than input value type. Especially misleading is
    "precision is not allowed for integer values", since integer
    value input to a format like .1f is fine.
  • regarding max field size, replace "non-number" with "string",
    which is the only non-numeric presentation type

Automerge-Triggered-By: GH:ericvsmith

  * `precision` field is an integer, not decimal
  * clarify that stated limitations are on presentation type
    rather than input value type.  Especially misleading is
    "precision is not allowed for integer values", since integer
    value input to a format like `.1f` is fine.
  * regarding max field size, replace "non-number" with "string",
    which is the only non-numeric presentation type
@belm0
Copy link
Contributor Author

@belm0 belm0 commented Feb 13, 2022

@ericvsmith would you mind reviewing this doc correction?

Copy link
Member

@ericvsmith ericvsmith left a comment

The change looks really good. Thanks. @mdickinson, any comments?

As long as we're changing this section, should 'e' and 'E' be mentioned?

@mdickinson
Copy link
Member

@mdickinson mdickinson commented Feb 13, 2022

Changes look good to me. How about "decimal integer" in place of "integer"? (Strictly speaking, the precision isn't an integer - it's a decimal string representation of an integer.)

FWIW, "decimal integer" is the language that the C specification uses here. From C99:

The precision takes the form of a period (.) followed either by an asterisk * (described later) or by an optional decimal integer [...]

@mdickinson
Copy link
Member

@mdickinson mdickinson commented Feb 13, 2022

As long as we're changing this section, should 'e' and 'E' be mentioned?

Probably, yes, though it would be fine to change it in a follow-up PR. Technically, the same description as for 'f' and 'F' applies, I think. (Though this feels a bit misleading, given that the precision has quite a different meaning.)

@ericvsmith
Copy link
Member

@ericvsmith ericvsmith commented Feb 13, 2022

As long as we're changing this section, should 'e' and 'E' be mentioned?

Probably, yes, though it would be fine to change it in a follow-up PR.

I'm okay with it being a separate PR.

So other than "decimal integer", this is ready to go. Note that for width it already says: "width is a decimal integer".

Copy link
Member

@ericvsmith ericvsmith left a comment

Thanks!

@miss-islington miss-islington merged commit 1d6ce67 into python:main Feb 14, 2022
12 checks passed
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Feb 14, 2022

Thanks @belm0 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒🤖

miss-islington added a commit to miss-islington/cpython that referenced this issue Feb 14, 2022
* `precision` field is a decimal integer
  * clarify that stated limitations are on presentation type
    rather than input value type.  Especially misleading is
    "precision is not allowed for integer values", since integer
    value input to a format like `.1f` is fine.
  * regarding max field size, replace "non-number" with "string",
    which is the only non-numeric presentation type

Automerge-Triggered-By: GH:ericvsmith
(cherry picked from commit 1d6ce67)

Co-authored-by: John Belmonte <john@neggie.net>
@miss-islington
Copy link
Contributor

@miss-islington miss-islington commented Feb 14, 2022

Sorry, @belm0, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 1d6ce67c29aa2166ef326952cb605b908fb4f987 3.9

@miss-islington miss-islington self-assigned this Feb 14, 2022
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Feb 14, 2022

GH-31320 is a backport of this pull request to the 3.10 branch.

@belm0 belm0 deleted the format_precision_docs branch Feb 14, 2022
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Feb 14, 2022

GH-31321 is a backport of this pull request to the 3.9 branch.

belm0 added a commit to belm0/cpython that referenced this issue Feb 14, 2022
* `precision` field is a decimal integer
  * clarify that stated limitations are on presentation type
    rather than input value type.  Especially misleading is
    "precision is not allowed for integer values", since integer
    value input to a format like `.1f` is fine.
  * regarding max field size, replace "non-number" with "string",
    which is the only non-numeric presentation type

Automerge-Triggered-By: GH:ericvsmith.
(cherry picked from commit 1d6ce67)

Co-authored-by: John Belmonte <john@neggie.net>
miss-islington added a commit that referenced this issue Feb 14, 2022
* `precision` field is a decimal integer
  * clarify that stated limitations are on presentation type
    rather than input value type.  Especially misleading is
    "precision is not allowed for integer values", since integer
    value input to a format like `.1f` is fine.
  * regarding max field size, replace "non-number" with "string",
    which is the only non-numeric presentation type

Automerge-Triggered-By: GH:ericvsmith
(cherry picked from commit 1d6ce67)

Co-authored-by: John Belmonte <john@neggie.net>
ericvsmith pushed a commit that referenced this issue Feb 14, 2022
* `precision` field is a decimal integer
  * clarify that stated limitations are on presentation type
    rather than input value type.  Especially misleading is
    "precision is not allowed for integer values", since integer
    value input to a format like `.1f` is fine.
  * regarding max field size, replace "non-number" with "string",
    which is the only non-numeric presentation type

Automerge-Triggered-By: GH:ericvsmith.
(cherry picked from commit 1d6ce67)

Co-authored-by: John Belmonte <john@neggie.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment