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.

Sign up for GitHub

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

Jump to bottom

Proposed clarification of spec for int/float/complex promotion #1748

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Proposed clarification of spec for int/float/complex promotion #1748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d2a3bdf
Proposed clarification of spec for int/float/complex promotion
JelleZijlstra May 23, 2024
cfefa40
typo
JelleZijlstra May 23, 2024
cf3486f
remove incorrect case
JelleZijlstra May 23, 2024
6155dbd
Extend test
JelleZijlstra May 23, 2024
a442bfe
Guard against unreachability
JelleZijlstra May 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions conformance/results/mypy/specialtypes_promotions.toml
View file
Open in desktop
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
"""
2 changes: 1 addition & 1 deletion conformance/results/mypy/version.toml
View file
Open in desktop
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
5 changes: 4 additions & 1 deletion conformance/results/pyre/specialtypes_promotions.toml
View file
Open in desktop
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
"""
2 changes: 1 addition & 1 deletion conformance/results/pyre/version.toml
View file
Open in desktop
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
6 changes: 5 additions & 1 deletion conformance/results/pyright/specialtypes_promotions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
conformant = "Pass"
output = """
specialtypes_promotions.py:13:7 - error: Cannot access attribute "numerator" for class "float"
specialtypes_promotions.py:17:7 - error: Cannot access attribute "numerator" for class "float"
  Attribute "numerator" is unknown (reportAttributeAccessIssue)
specialtypes_promotions.py:26:16 - error: Expression of type "Literal['x']" is incompatible with return type "int"
  "Literal['x']" is incompatible with "int" (reportReturnType)
specialtypes_promotions.py:33:16 - error: Expression of type "complex" is incompatible with return type "float"
  "complex" is incompatible with "float" (reportReturnType)
"""
conformance_automated = "Pass"
errors_diff = """
Expand Down
2 changes: 1 addition & 1 deletion conformance/results/pyright/version.toml
View file
Open in desktop
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
11 changes: 8 additions & 3 deletions conformance/results/pytype/specialtypes_promotions.toml
View file
Open in desktop
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
"""
2 changes: 1 addition & 1 deletion conformance/results/pytype/version.toml
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions conformance/results/results.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,16 +159,16 @@ <h3>Python Type System Conformance Test Results</h3>
<div class="table_container"><table><tbody>
<tr><th class="col1">&nbsp;</th>
<th class='tc-header'><div class='tc-name'>mypy 1.10.0</div>
<div class='tc-time'>1.4sec</div>
<div class='tc-time'>1.5sec</div>
</th>
<th class='tc-header'><div class='tc-name'>pyright 1.1.364</div>
<div class='tc-time'>1.4sec</div>
<div class='tc-time'>1.6sec</div>
</th>
<th class='tc-header'><div class='tc-name'>pyre 0.9.21</div>
<div class='tc-time'>3.4sec</div>
<div class='tc-time'>2.9sec</div>
</th>
<th class='tc-header'><div class='tc-name'>pytype 2024.04.11</div>
<div class='tc-time'>30.1sec</div>
<div class='tc-time'>49.8sec</div>
</th>
</tr>
<tr><th class="column" colspan="5">
Expand Down
35 changes: 28 additions & 7 deletions conformance/tests/specialtypes_promotions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,36 @@
Tests "type promotions" for float and complex when they appear in annotations.
"""

from typing import assert_type
JelleZijlstra marked this conversation as resolved.
Show resolved Hide resolved

# 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
Copy link
Collaborator

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 unguarded f.hex() as well

Copy link
Member

@carljm carljm May 29, 2024 •
edited
Loading

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 "member int of float | int has no method hex." builtins.int in typeshed doesn't have the hex method. So if you want to call hex you have to guard it with isinstance(float).

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 float | int interpretation of float annotations.

@erictraut What's the explanation of pyright's behavior here? Is this special-cased in some way?

I think if the conformance suite shows f.hex() here to not be an error, then that needs to be justified with some additional wording in the spec.

Copy link
Collaborator

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 called expandPromotionTypes that is responsible for taking the type float and expanding it to Float | int and complex into Complex | 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:

  1. isinstance type narrowing
  2. Class pattern matching (which is the match statement equivalent of isinstance checks)
  3. Attribute access ("dot") expression forms

I also 4) changed the inference logic for float literals to be inferred as Float rather than float.

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 float() or complex() constructors needs to evaluate to Float and Complex, 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 as list[float], which probably means that 3.1 should continue to be inferred as float and not Float. I think that's OK, but it does lead to an apparent inconsistency because [float(3.1)] is evaluated as list[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'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 f.numerator in the conformance test case). The "mypy_primer" run shows one change in the dd_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.

Copy link
Member

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 expand float to float | 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 on f: float; f.hex(). The hex/fromhex case is similar to the is_integer case encountered in microsoft/pyright#6032, but I expect is_integer is more commonly used. And is_integer was added to the int 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

Copy link
Collaborator

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 see float | int in hover text, inlined type annotations, completion suggestions, reveal_type text, etc. if they use float in a type annotation. And likewise, if they specify float | int in a type annotation, it would be confusing to reduce that to just float. I try to retain the same form used in the annotation where possible.


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
24 changes: 19 additions & 5 deletions docs/spec/special-types.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()``::
Copy link

@grievejia grievejia May 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC this is basically saying that if float appears as an argument to isinstance() then we should interpret it differently from the case when float appears in function annotations?

In that case, I wonder what should happen in other cases, e.g. cast(), assert_type(), or other kinds of type forms -- should we interpret float to be the "real" float in those places as well? Does it make sense to have the spec discuss about the "scope" of this kind of special-casing interpretations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed wording says that float should be interpreted as float | int in type expressions. The first argument to cast() is a type expression, but the second argument to isinstance() is not.


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`:

Expand Down