gh-100428: Make float documentation more accurate

[hauntsaninja]

Previously, the grammar did not accept float("10"). Also implement mdickinson's suggestion of removing the indirection.

• Issue: Float documentation doesn't allow int-like strings as valid arguments for float #100428

[mdickinson]

The approach LGTM; I've left a couple of minor comments.

With the new text, we're missing a definition of digit in the grammar production. Could we add a sentence describing what characters are valid digits? (IIRC, it's any Unicode character with Numeric_Type=Decimal - i.e., those characters c for which unicodedata.category(c) == 'Nd', but it's worth double-checking in case IDRC.)

One other suggestion: in the sentence

More precisely, the input must conform to the following grammar [...]

"conform to the following grammar" is a bit ambiguous, since it doesn't specify which part of the grammar is meant. Could we add an explicit reference to the numeric_string production rule here? (It's possible to guess that that's what's meant, but it would be better not to have to guess.)

[hauntsaninja]

Thank you for the review! I've addressed the feedback in individual commits.

It's a little tempting to do some renaming while we're here

I think we can do some renaming here! This isn't part of the official grammar, the only place where there are similarly named production rules are in Lexical Analysis which we're trying to be different from. In particular, the numeric_value and numeric_string names aren't used anywhere else.

Perhaps we could collapse numeric_string and numeric_value to a single floatstring: [sign] (floatnumber | infinity | nan) rule?

we're missing a definition of digit

I checked the implementation and you're right, _PyUnicode_ToDecimalDigit is Numeric_Type=Decimal. I'm not too familiar with Unicode, so I wasn't sure whether Numeric_Type=Decimal matches the "Nd" category. The Unicode standard says

The Numeric_Type = Decimal property value (which is correlated with the General_Category = Nd property value) is ...

within which I find the use of "correlated" a little ambiguous. But it does seem to empirically hold:

{i for i in range(0x110000) if unicodedata.decimal(chr(i), None) is not None} == {i for i in range(0x110000) if unicodedata.category(chr(i)) == "Nd"}

[mdickinson]

Thanks for the updates. This L(V)GTM!

Perhaps we could collapse numeric_string and numeric_value to a single floatstring: [sign] (floatnumber | infinity | nan) rule?

That could work; I think it's also fine as it is, though. I'll leave it up to you whether you want to tinker with this before merging.

within which I find the use of "correlated" a little ambiguous

Yes, me too, but I agree that that does seem to be the intended meaning (i.e., General_Category = Nd if and only if Numeric_Type = Decimal). If we're worried about this, we could update the text to match the exact test we're using in the code (i.e., based on Numeric_Type instead of General_Category). Either way seems fine to me - I leave it to you to decide.

Thank you for this!

[mdickinson]

(BTW, I'll review #100436, too, but it might be a couple of days before I get to it.)

[hauntsaninja]

cc @JelleZijlstra
(let me know if i shouldn't tag you before merge on either or both of a) my own PRs that other core devs have approved, b) other's PRs that other core devs have approved)

[JelleZijlstra]

Looks good. Agree, no need to ping me on PRs approved by other core devs (so both a) and b) in your message).

[miss-islington]

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

[miss-islington]

Sorry @hauntsaninja, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 2e1a9ce9890aba748a518a39d01d1ea6d623d0d9 3.11

[bedevere-bot]

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

[miss-islington]

Thanks @hauntsaninja for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

[bedevere-bot]

GH-100511 is a backport of this pull request to the 3.11 branch.