-
Notifications
You must be signed in to change notification settings - Fork 259
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 our terms of service and privacy 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?
Changes from all commits
d2a3bdf
cfefa40
cf3486f
6155dbd
a442bfe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,12 @@ | ||
conformant = "Pass" | ||
conformant = "Partial" | ||
notes = """ | ||
Does not narrow from float to int after isinstance() check | ||
""" | ||
output = """ | ||
specialtypes_promotions.py:13: error: "float" has no attribute "numerator" [attr-defined] | ||
specialtypes_promotions.py:17: error: "float" has no attribute "numerator" [attr-defined] | ||
specialtypes_promotions.py:33: error: Incompatible return value type (got "complex", expected "float") [return-value] | ||
""" | ||
conformance_automated = "Pass" | ||
conformance_automated = "Fail" | ||
errors_diff = """ | ||
Line 26: Expected 1 errors | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
version = "mypy 1.10.0" | ||
test_duration = 1.4 | ||
test_duration = 1.5 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
conformant = "Partial" | ||
notes = """ | ||
Does not reject use of attribute that is compatible only with float. | ||
Does not narrow from float to int after isinstance() check | ||
""" | ||
output = """ | ||
specialtypes_promotions.py:33:8 Incompatible return type [7]: Expected `float` but got `complex`. | ||
""" | ||
conformance_automated = "Fail" | ||
errors_diff = """ | ||
Line 13: Expected 1 errors | ||
Line 17: Expected 1 errors | ||
Line 26: Expected 1 errors | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
version = "pyre 0.9.21" | ||
test_duration = 3.4 | ||
test_duration = 2.9 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
version = "pyright 1.1.364" | ||
test_duration = 1.4 | ||
test_duration = 1.6 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,12 @@ | ||
conformant = "Pass" | ||
conformant = "Partial" | ||
notes = """ | ||
Does not narrow from float to int after isinstance() check | ||
""" | ||
output = """ | ||
File "specialtypes_promotions.py", line 13, in func1: No attribute 'numerator' on float [attribute-error] | ||
File "specialtypes_promotions.py", line 17, in func1: No attribute 'numerator' on float [attribute-error] | ||
File "specialtypes_promotions.py", line 33, in func2: bad return type [bad-return-type] | ||
""" | ||
conformance_automated = "Pass" | ||
conformance_automated = "Fail" | ||
errors_diff = """ | ||
Line 26: Expected 1 errors | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
version = "pytype 2024.04.11" | ||
test_duration = 30.1 | ||
test_duration = 49.8 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,15 +2,36 @@ | |
Tests "type promotions" for float and complex when they appear in annotations. | ||
""" | ||
|
||
from typing import assert_type | ||
|
||
# Specification: https://typing.readthedocs.io/en/latest/spec/special-types.html#special-cases-for-float-and-complex | ||
|
||
v1: float = 1 | ||
v2: complex = 1.2 | ||
v2 = 1 | ||
v1: int = 1 | ||
v2: float = 1 | ||
v3: float = v1 | ||
v4: complex = 1.2 | ||
v4 = 1 | ||
|
||
|
||
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 commentThe reason will be displayed to describe this comment to others. Learn more. I think worth having an unguarded There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 The proposed spec wording, as is, would mean that this should be an error, right? Because "member But this doesn't seem to be an error in either mypy or pyright. The latter is particularly interesting, since my understanding was that pyright already used the @erictraut What's the explanation of pyright's behavior here? Is this special-cased in some way? I think if the conformance suite shows There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In pyright's current implementation, I have a function called expandPromotionTypes that is responsible for taking the type There are three places where I initially added calls to this function:
I also 4) changed the inference logic for float literals to be inferred as I ended up backing out 3 and 4 because these two cases produced too much noise, including some false positive errors. See this 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 to 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 Here's a 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 for 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 commentThe reason will be displayed to describe this comment to others. Learn more. I see, so pyright's implementation doesn't actually expand I guess with #3 added back, pyright would also error on 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 commentThe reason will be displayed to describe this comment to others. Learn more.
That's correct. It would be confusing for pyright & pylance users to see |
||
|
||
if isinstance(f, float): | ||
f.hex() # OK (attribute exists on float but not int) | ||
return 1 | ||
else: | ||
assert_type(f, int) | ||
erictraut marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# Make sure type checkers don't treat this branch as unreachable | ||
# and skip checking it. | ||
return "x" # E | ||
|
||
def func1(f: float): | ||
f.numerator # E | ||
|
||
if not isinstance(f, float): | ||
f.numerator # OK | ||
def func2(x: int) -> float: | ||
if x == 0: | ||
return 1 | ||
elif x == 1: | ||
return 1j # E | ||
elif x > 10: | ||
return x | ||
else: | ||
return 1.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -113,11 +113,25 @@ Special cases for ``float`` and ``complex`` | |
|
||
Python's numeric types ``complex``, ``float`` and ``int`` are not | ||
subtypes of each other, but to support common use cases, the type | ||
system contains a straightforward shortcut: | ||
when an argument is annotated as having | ||
type ``float``, an argument of type ``int`` is acceptable; similar, | ||
for an argument annotated as having type ``complex``, arguments of | ||
type ``float`` or ``int`` are acceptable. | ||
system contains a special case. | ||
|
||
When a reference to the built-in type ``float`` appears in a :term:`type expression`, | ||
it is interpreted as if it were a union of the built-in types ``float`` and ``int``. | ||
Similarly, when a reference to the type ``complex`` appears, it is interpreted as | ||
a union of the built-in types ``complex``, ``float`` and ``int``. | ||
These implicit unions behave exactly like the corresponding explicit union types, | ||
but type checkers may choose to display them differently in user-visible output | ||
for clarity. | ||
|
||
Type checkers should support narrowing the type of a variable to exactly ``float`` | ||
or ``int``, without the implicit union, through a call to ``isinstance()``:: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC this is basically saying that if In that case, I wonder what should happen in other cases, e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The proposed wording says that |
||
|
||
def f(x: float) -> None: | ||
reveal_type(x) # float | int, but type checkers may display just "float" | ||
if isinstance(x, float): | ||
reveal_type(x) # float | ||
else: | ||
reveal_type(x) # int | ||
|
||
.. _`type-brackets`: | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.