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

spec: clarify the mro linearization of Any #1672

Closed
wants to merge 8 commits into from
Closed
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
103 changes: 103 additions & 0 deletions conformance/tests/specialtypes_any.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

# > Every type is consistent with Any.

from collections.abc import Iterator
from typing import Any, Callable, assert_type


Expand Down Expand Up @@ -87,4 +88,106 @@ def method1(self) -> int:
assert_type(a.method2(), Any)
assert_type(ClassA.method3(), Any)

# > When ``Any`` is present in the bases of a type,
# > it should be considered only after all other known types in the MRO.

class ClassKnown:

classvar1 = ""

def __iter__(self) -> Iterator[str]:
yield from self.attr1

def __init__(self):
self.attr1: str = ""

def method1(self) -> str:
return ""

class AnyFirst(Any, ClassKnown):

def method2(self) -> str:
return ""

class AnyLast(ClassKnown, Any):
def method2(self) -> str:
return ""

class GetattrKnown(ClassKnown):
def __getattr__(self, name: str) -> int:
return 1

class AnyFirstGetAttr(Any, GetattrKnown):
def method2(self) -> str:
return ""

class AnyLastGetAttr(GetattrKnown, Any):
def method2(self) -> str:
return ""

class AnySub(Any):
...

# primarily included to demonstrate intent that this is for the full MRO
class AnySubFirst(AnySub, ClassKnown):
def method2(self) -> str:
...



af = AnyFirst()
assert_type(af.method1(), str)
assert_type(af.method2(), str)
assert_type(af.attr1, str)
assert_type(af.non_exist_method(), Any)
assert_type(af.non_exist_attr, Any)
assert_type(af.classvar1, str)
assert_type(AnyFirst.classvar1, str)
assert_type(iter(af), Iterator[str])

al = AnyLast()
assert_type(al.method1(), str)
assert_type(al.method2(), str)
assert_type(al.attr1, str)
assert_type(al.non_exist_method(), Any)
assert_type(al.non_exist_attr, Any)
assert_type(al.classvar1, str)
assert_type(AnyLast.classvar1, str)
assert_type(iter(al), Iterator[str])


# this example has Any deeper in the inheritence than the bases, specifically as
# an ancestor of the first base, with other known methods and attributes in the second.
full_mro_checked = AnySubFirst()

assert_type(full_mro_checked.method1(), str)
assert_type(full_mro_checked.method2(), str)
assert_type(full_mro_checked.attr1, str)
assert_type(al.non_exist_method(), Any)
assert_type(al.non_exist_attr, Any)
assert_type(full_mro_checked.classvar1, str)
assert_type(AnySubFirst.classvar1, str)
assert_type(iter(full_mro_checked), Iterator[str])

# Note The next two examples check different bases, one of which provides a `__getattr__`
# ensuring the behavior is the same for unaffected cases

af_getattr = AnyFirstGetAttr()

assert_type(af_getattr.method1(), str)
assert_type(af_getattr.method2(), str)
assert_type(af_getattr.attr1, str)
assert_type(af_getattr.triggers_getattr, int)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct. The __getattr__ method is called only if the attribute isn't present in the object's dictionary. Since Any is in the MRO (even as the last item), then all attributes are effectively "found in the object's dictionary". So this will evaluate to Any.

Copy link
Author

Choose a reason for hiding this comment

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

This is correct, one of the bases provided provides a __getattr__ definition

Copy link
Collaborator

Choose a reason for hiding this comment

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

But __getattr__ isn't applicable here. This method is invoked only if the attribute is not present. But in this case, it is "present" because all attributes are present on an Any base class.

Copy link
Author

Choose a reason for hiding this comment

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

At runtime, this is definitely invoked, runtime doesn't see Any and do something special for __getattr__, I've modeled this on accurately typing runtime for a known implementation of __getattr__

Copy link
Member

@carljm carljm Apr 3, 2024

Choose a reason for hiding this comment

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

I don't think "modeling this on runtime" with the actual runtime Any type is meaningful here. The non_existent_attr examples don't "model runtime" either -- at runtime with the actual runtime Any type in the MRO, those examples raise AttributeError.

The point of having Any in the MRO is that for typing purposes it might represent any concrete type, which might have any actual attribute. If Any in this case represented a concrete type that provides the triggers_getattr attribute, then __getattr__ would not be invoked. So for the same reason that non_existent_attr works and produces Any in these examples, triggers_getattr should work and also produce Any, because checking the types in the MRO for the triggers_getattr attribute occurs before consulting __getattr__, and Any must be considered to provide all attributes.

Choose a reason for hiding this comment

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

What about going the other way on this? You could specify that a class that inherits from Any must only have a __getattr__ definition if the definition is typed to return Any

Copy link
Member

Choose a reason for hiding this comment

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

Can someone outline or link to the motivating case with mocks that would be broken if the spec just gave Any on these getattr cases?

Copy link
Author

Choose a reason for hiding this comment

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

It could be resolved that way, taking this over to discourse.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This has nothing to do with whether the unknown class implements __getattr__. It has to do with whether the unknown class has an attribute called triggers_getattr. We must assume it does, in which case the presence of a __getattr__ method is irrelevant.

Consider the following:

class A:
    foo = "foo"

class B:
    def __getattr__(self, name: str) -> int:
        print("__getattr__")
        return 1

class C(A, B):
    pass

c = C()
reveal_type(c.foo) # str
print(c.foo) # "foo"

Note that B.__getattr__ is never invoked at runtime in this case, and the type checker accordingly never simulates a call to __getattr__ when evaluating the type of expression c.foo.

Now, we replace A with Any. Here again, the type checker should assume that __getattr__ will not be invoked.

class C(Any, B):
    pass

c = C()
reveal_type(c.foo) # Any

Copy link
Author

Choose a reason for hiding this comment

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

(github isn't loading reviews in realtime right now for anyone viewing this after the fact)

assert_type(af_getattr.classvar1, str)
assert_type(AnyFirstGetAttr.classvar1, str)
assert_type(iter(af_getattr), Iterator[str])

al_getattr = AnyLastGetAttr()

assert_type(al_getattr.method1(), str)
assert_type(al_getattr.method2(), str)
assert_type(al_getattr.attr1, str)
assert_type(al_getattr.triggers_getattr, int)
mikeshardmind marked this conversation as resolved.
Show resolved Hide resolved
assert_type(al_getattr.classvar1, str)
assert_type(AnyLastGetAttr.classvar1, str)
mikeshardmind marked this conversation as resolved.
Show resolved Hide resolved
assert_type(iter(al_getattr), Iterator[str])
3 changes: 2 additions & 1 deletion docs/spec/special-types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ to ``tuple[Any, ...]``. As well, a bare

``Any`` can also be used as a base class. This can be useful for
avoiding type checker errors with classes that can duck type anywhere or
are highly dynamic.
are highly dynamic. When ``Any`` is present in the MRO of a type,
it should be considered only after all other known types in the MRO.

.. _`none`:

Expand Down