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

Conformance tests and spec change for generic type erasure #1589

Merged
merged 2 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions conformance/results/mypy/generics_type_erasure.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
conformant = "Partial"
notes = """
Infers Node[Never] instead of Node[Any] when argument is not provided.
False negative on instance attribute access on type(node).
"""
output = """
generics_type_erasure.py:17: error: Expression is of type "Node[Never]", not "Node[Any]" [assert-type]
generics_type_erasure.py:20: error: Expression is of type Never, not "Any" [assert-type]
generics_type_erasure.py:36: error: Argument 1 to "Node" has incompatible type "str"; expected "int | None" [arg-type]
generics_type_erasure.py:38: error: Argument 1 to "Node" has incompatible type "int"; expected "str | None" [arg-type]
generics_type_erasure.py:40: error: Access to generic instance variables via class is ambiguous [misc]
generics_type_erasure.py:41: error: Access to generic instance variables via class is ambiguous [misc]
generics_type_erasure.py:42: error: Access to generic instance variables via class is ambiguous [misc]
generics_type_erasure.py:43: error: Access to generic instance variables via class is ambiguous [misc]
"""
11 changes: 11 additions & 0 deletions conformance/results/pyre/generics_type_erasure.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
conformant = "Partial"
notes = """
Doesn't allow using Node[Any] in assert_type expression.
False negatives on instance attribute access on the type.
"""
output = """
generics_type_erasure.py:11:0 Uninitialized attribute [13]: Attribute `label` is declared in class `Node` to have type `Variable[T]` but is never initialized.
generics_type_erasure.py:17:25 Incompatible parameter type [6]: In call `typing.GenericMeta.__getitem__`, for 1st positional argument, expected `Type[Variable[T]]` but got `object`.
generics_type_erasure.py:36:15 Incompatible parameter type [6]: In call `Node.__init__`, for 1st positional argument, expected `Optional[int]` but got `str`.
generics_type_erasure.py:38:15 Incompatible parameter type [6]: In call `Node.__init__`, for 1st positional argument, expected `Optional[str]` but got `int`.
"""
18 changes: 18 additions & 0 deletions conformance/results/pyright/generics_type_erasure.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
conformant = "Partial"
notes = """
False negatives on instance attribute access on the type.
"""
output = """
generics_type_erasure.py:36:16 - error: Argument of type "Literal['']" cannot be assigned to parameter "label" of type "int | None" in function "__init__"
  Type "Literal['']" cannot be assigned to type "int | None"
    "Literal['']" is incompatible with "int"
    "Literal['']" is incompatible with "None" (reportGeneralTypeIssues)
generics_type_erasure.py:38:16 - error: Argument of type "Literal[0]" cannot be assigned to parameter "label" of type "str | None" in function "__init__"
  Type "Literal[0]" cannot be assigned to type "str | None"
    "Literal[0]" is incompatible with "str"
    "Literal[0]" is incompatible with "None" (reportGeneralTypeIssues)
generics_type_erasure.py:42:6 - error: Cannot assign member "label" for type "type[Node[T@Node]]"
  Expression of type "Literal[1]" cannot be assigned to member "label" of class "Node[T@Node]"
    Member "__set__" is unknown
    Type "Literal[1]" cannot be assigned to type "T@Node" (reportGeneralTypeIssues)
"""
4 changes: 4 additions & 0 deletions conformance/results/pytype/generics_type_erasure.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
conformant = "Unsupported"
output = """
NotImplementedError: ParameterizedClass
"""
54 changes: 54 additions & 0 deletions conformance/tests/generics_type_erasure.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Specification: https://typing.readthedocs.io/en/latest/spec/generics.html#instantiating-generic-classes-and-type-erasure

from typing import Any, TypeVar, Generic, assert_type

T = TypeVar("T")

# > If the constructor (__init__ or __new__) uses T in its signature, and a
# > corresponding argument value is passed, the type of the corresponding
# > argument(s) is substituted. Otherwise, Any is assumed.

class Node(Generic[T]):
label: T
def __init__(self, label: T | None = None) -> None: ...

assert_type(Node(''), Node[str])
assert_type(Node(0), Node[int])
assert_type(Node(), Node[Any])

assert_type(Node(0).label, int)
assert_type(Node().label, Any)

# > In case the inferred type uses [Any] but the intended type is more specific,
# > you can use an annotation to force the type of the variable, e.g.:

n1: Node[int] = Node()
assert_type(n1, Node[int])
n2: Node[str] = Node()
assert_type(n2, Node[str])

n3 = Node[int]()
assert_type(n3, Node[int])
n4 = Node[str]()
assert_type(n4, Node[str])

n5 = Node[int](0) # OK
n6 = Node[int]("") # Type error
n7 = Node[str]("") # OK
n8 = Node[str](0) # Type error

Node[int].label = 1 # Type error
Node[int].label # Type error
Node.label = 1 # Type error
Node.label # Type error
type(n1).label # Type error
assert_type(n1.label, int)
assert_type(Node[int]().label, int)
n1.label = 1 # OK

# > [...] generic versions of concrete collections can be instantiated:

from typing import DefaultDict

data = DefaultDict[int, bytes]()
assert_type(data[0], bytes)
8 changes: 4 additions & 4 deletions docs/spec/generics.rst
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,15 @@ argument(s) is substituted. Otherwise, ``Any`` is assumed. Example::

class Node(Generic[T]):
Copy link
Collaborator Author

@hauntsaninja hauntsaninja Jan 16, 2024

Choose a reason for hiding this comment

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

The paragraph above could probably be improved. For instance, without the change I make here, pyright would infer Node() to be Node[None], and I think that should be legal.

That said, handling of default values for generics in general is underspecified (mypy handles them poorly, but in part because it wasn't clear what the behaviour should be python/mypy#3737)

Copy link
Collaborator

Choose a reason for hiding this comment

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

When this part of the spec was written, mypy defaulted to "implicit-optional", which would give you the same effect as adding | None. Since then, the spec was updated to recommend against "implicit-optional", but this code sample wasn't updated. I'd be in favor of just updating the spec to fix the sample.

x: T # Instance attribute (see below)
def __init__(self, label: T = None) -> None:
def __init__(self, label: T | None = None) -> None:
...

x = Node('') # Inferred type is Node[str]
y = Node(0) # Inferred type is Node[int]
z = Node() # Inferred type is Node[Any]

In case the inferred type uses ``[Any]`` but the intended type is more
specific, you can use a type comment (see below) to force the type of
specific, you can use an annotation (see below) to force the type of
the variable, e.g.::

# (continued from previous example)
Expand Down Expand Up @@ -373,8 +373,8 @@ class instance that does not have an instance attribute with the same name::
Node.x = 1 # Error
Node.x # Error
type(p).x # Error
p.x # Ok (evaluates to None)
Node[int]().x # Ok (evaluates to None)
p.x # Ok (evaluates to int)
Node[int]().x # Ok (evaluates to int)
p.x = 1 # Ok, but assigning to instance attribute
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These just seem straight up incorrect to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I agree. Looks like a spec bug.


Generic versions of abstract collections like ``Mapping`` or ``Sequence``
Expand Down
Loading