- Notifications
You must be signed in to change notification settings - Fork263
Proposed clarification of spec for int/float/complex promotion#1748
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me. One minor suggested improvement in the test case, but it's fine without this change.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
def func1(f: float) -> int: | ||
f.numerator # E: attribute exists on int but not float |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think worth having an unguardedf.hex()
as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This one is interesting, because it's not totally clear to me what behavior we should specify for an unguarded.hex()
call.
The proposed spec wording, as is, would mean that this should be an error, right? Because "memberint
offloat | int
has no methodhex
."builtins.int
in typeshed doesn't have thehex
method. So if you want to callhex
you have to guard it withisinstance(float)
.
But this doesn't seem to be an error in eithermypy orpyright. The latter is particularly interesting, since my understanding was that pyright already used thefloat | int
interpretation offloat
annotations.
@erictraut What's the explanation of pyright's behavior here? Is this special-cased in some way?
I think if the conformance suite showsf.hex()
here to not be an error, then that needs to be justified with some additional wording in the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In pyright's current implementation, I have a function calledexpandPromotionTypes that is responsible for taking the typefloat
and expanding it toFloat | int
andcomplex
intoComplex | Float | int
. (I use a capitalized name here to indicate the "real" type.)
There are three places where I initially added calls to this function:
isinstance
type narrowing- Class pattern matching (which is the
match
statement equivalent ofisinstance
checks) - Attribute access ("dot") expression forms
I also 4) changed the inference logic for float literals to be inferred asFloat
rather thanfloat
.
I ended up backing out 3 and 4 because these two cases produced too much noise, including some false positive errors. Seethis issue, which shows some of the pain this caused pyright users.
I think it's reasonable to add 3 back, but doing so requires an additional change to avoid some of the noise. Namely, a call tofloat()
orcomplex()
constructors needs to evaluate toFloat
andComplex
, respectively.
I think that adding 4 back would be problematic, especially in situations where float literals are used in list, set and dict expressions. These are problematic because the types are invariant. Consider the following code:
x= [3.1]# Should the type of `[3.1]` be inferred as `list[float]` or `list[Float]`?x.append(1)# Should this be allowed? Most devs would expect it to be!
I think the expression[3.1]
should continue to be inferred aslist[float]
, which probably means that3.1
should continue to be inferred asfloat
and notFloat
. I think that's OK, but it does lead to an apparent inconsistency because[float(3.1)]
is evaluated aslist[Float]
. The typing spec doesn't dictate type inference rules (currently), so these are not in scope for the spec, but this issue is something that type checker maintainers / authors will need to consider.
Here'sa PR (and "mypy_primer" run) that adds back 3 from my list above. This makes pyright conformant with the proposed language in this typing spec update (i.e. it now generates an error forf.numerator
in the conformance test case). The "mypy_primer" run shows one change in thedd_trace
library. It's in code that looks like this:
x= {"a":float(0)}# Now evaluates to `dict[str, Float]` rather than `dict[str, float]`x["b"]=1# Now a type error because `int` cannot be assigned to this `dict`
This change is not without consequences, but I think the impact is relatively minimal and new type errors are straightforward to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I see, so pyright's implementation doesn't actually expandfloat
tofloat | int
when it's encountered in an annotation, but rather at certain points where the type is used.
I guess with#3 added back, pyright would also error onf: float; f.hex()
. Thehex/fromhex
case is similar to theis_integer
case encountered inmicrosoft/pyright#6032, but I expectis_integer
is more commonly used. Andis_integer
was added to theint
type in Python 3.12 to avoid this problem. It doesn't seem like your mypy primer run encountered any issues with calls to.hex
or.fromhex
, which doesn't surprise me.
One thing that does surprise me on that mypy primer run is that the two new errors don't appear to involve attribute access (your (3)), but rather inferred types for containers (your (4)). E.g. one of the errors is on this line:https://github.com/DataDog/dd-trace-py/blob/main/ddtrace/llmobs/_integrations/bedrock.py#L43
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
so pyright's implementation doesn't actually expand float to float | int when it's encountered in an annotation
That's correct. It would be confusing for pyright & pylance users to seefloat | int
in hover text, inlined type annotations, completion suggestions,reveal_type
text, etc. if they usefloat
in a type annotation. And likewise, if they specifyfloat | int
in a type annotation, it would be confusing to reduce that to justfloat
. I try to retain the same form used in the annotation where possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good to me.
Uh oh!
There was an error while loading.Please reload this page.
DiscordLiz left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
IF this is actually going to be pushed through as a "Clarification", I'd appreciate if it was clear on what the behavior withTypeIs[float]
, and Type[T], where the parameter passed isfloat
.
Both of these areruntime use of float, but are in an annotation context.TypeIs
has a case where it either contradicts the guidance to treatTypeIs
with the same narrowing rules as isinstance, or conflicts with the statement thatfloat
meansfloat | int
in an annotation context.
DiscordLiz commentedJun 29, 2025
There's also a case that should have an explicit ruling with overload compatability, shown here:https://discuss.python.org/t/clarifying-the-float-int-complex-special-case/54018/44 as the changes do have other effects that break existing cases. |
I'm still in the process of figuring out the implications to mypy (which uses a differerent approach currently), in case this would surface some additional edge cases worth documenting. I'm generally positive about clarifying the spec here though. |
Yes, this approach would imply changes to how mypy currently implements the special case. An alternative could be to specify mypy's behavior (which is essentially to pretend that int is a subclass of float), but I think that behavior is harder to understand and harder to specify precisely. |
Fixes#1746.