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

Why is Callable assumed to be a user defined function? #14392

Open
daniil-berg opened this issue Jan 4, 2023 · 10 comments
Open

Why is Callable assumed to be a user defined function? #14392

daniil-berg opened this issue Jan 4, 2023 · 10 comments

Comments

@daniil-berg
Copy link

daniil-berg commented Jan 4, 2023

The following should be an error, but it currently is not.

from collections.abc import Callable
from typing import Any


def f(call: Callable[..., Any]) -> None:
    print(call.__name__)  # why is there no error here?

IMO the last line should cause a mypy error to the effect of "Callable" has no attribute "__name__". The callable protocol generally does not define __name__.

To further drive this point home, add the following piece:

class Bar:
    def __call__(self) -> None:
        print(f"hi mom")


f(Bar())  # this is valid
print(Bar().__name__)  # this is an error

This means that

  1. Bar is correctly viewed as being a Callable subtype (structurally) and
  2. it is also correctly identified to not have the __name__ attribute by mypy.
  3. Yet when something is annotated as Callable, mypy just assumes it has the __name__ attribute. 🤨

Correct me if I am wrong, but these points cannot logically all be true.


The documentation of the Callable ABC clearly states that it is a class that defines the __call__ method. Nowhere does it mention the need for a Callable class to define __name__ (or anything else for that matter).

There is also no mention of any attributes/methods other than __call__ in the glossary entry for callable.

Lastly, the relevant section on Callable types of the standard type hierarchy chapter in the data model documentation again broadly defines them as

types to which the function call operation [...] can be applied

and further down even explicitly states the following:

Instances of arbitrary classes can be made callable by defining a __call__() method in their class.


I realize that user defined functions do always have the __name__ attribute (as well as a bunch of others listed in the aforementioned data model docs). But this should make them a subtype of Callable.

Yet it seems as though mypy treats a Callable as a user defined function. My example f above can be changed to any of the attributes of user defined functions, such as __kwdefaults__ for example and it would still pass mypy checks. This seems wildly inconsistent with all the documentation I could find, as well as logically inconsistent in itself.

I can only assume that there is some historic/pragmatic explanation for this, but I could not find it.

What is the reasoning behind this?


I found a discussion in #5958 that seems to be related, but found no answer to my question.

@JelleZijlstra
Copy link
Member

I think the main reason is that if we don't do this, we'll get a lot of false positives from people who use __name__ on Callables that do have the attribute (as the vast majority of Callables, not just user-defined functions but also types and builtin functions, have the attribute). So pragmatically, the current approach works better for users.

It does, as you say, create a minor hole in the type system. To fix it without creating unnecessary pain for users, we probably need some new type system features. For example, intersection types (python/typing#213) might allow writing a type like Callable & HasName.

@AlexWaygood
Copy link
Member

Note that if you want a type that doesn't assume all the attributes you get on standard function objects, you can just use a callback protocol:

from typing import Any, Protocol

class SaferCallable(Protocol):
    def __call__(self, *args, **kwargs) -> Any: ...

def f(call: SaferCallable) -> None:
    print(call.__name__)  # error: "SaferCallable" has no attribute "__name__"  [attr-defined]

@daniil-berg
Copy link
Author

@AlexWaygood I realize that I can define any custom protocol myself. I did not ask for workarounds or usage suggestions here. My contention is that this should not be necessary, if Callable were treated the way it is actually defined.


@JelleZijlstra Thank you for responding.

if we don't do this, we'll get a lot of false positives [...]

I guess I have a different definition of what "false positive" means. If someone wrongly assumes a Callable must have the __name__ attribute, just as with any other attribute that is not defined for a type, an error is warranted. Not sure how that is false positive. If you rely on __name__ or on foo, then yes, you should use an appropriate protocol:

from typing import Any, Protocol

class CallableWithNameAndFoo(Protocol):
    __name__: str
    foo: int
    
    def __call__(self, *args, **kwargs) -> Any: ...

def f(call: CallableWithNameAndFoo) -> None:
    ...

[...] the vast majority of Callables, not just user-defined functions but also types and builtin functions, have the attribute.

I understand this argument, but I don't think it is sound and I think the current approach is sub-optimal. It obscures the distinction between different callable subtypes and encourages sloppy typing. It treats the user as incapable of understanding core concepts of the data model.

A distinction is made with other collections ABCs between the more general types (such as Collection) that only define the bare minimum of methods and the more specific subtypes that add onto that (like Sequence and MutableSequence). Why isn't an equivalent approach followed for Callable?

Wouldn't it be prudent to follow the data model logic and keep Callable as the base type, and have inheritance chains like AsyncDefFunction <: DefFunction <: Callable, just as we already effectively have type <: Callable?

I mean you can see even with the latter relationship that it is inconsistent, if we take the __globals__ attribute for example:

from collections.abc import Callable
from typing import Any

def f(cal: Callable[..., Any], typ: type) -> None:
    print(cal.__globals__)  # no error
    print(typ.__globals__)  # error

assert issubclass(type, Callable)  # passes

I am not trying to be provocative here, but I really believe that mypy is too lenient with callables for no good reason. Thus, what you call "unnecessary pain" for users is what I would call "necessary pain" or "learning the hard way".

You are essentially saying: "Well, this is wrong, but... some people are doing it and we don't want to upset them by telling them they are doing it wrong, so... We'll just say this is correct.". I don't find that argument particularly compelling, regardless of the situation. Shouldn't correctness be priority number one?

@mjmikulski
Copy link

mjmikulski commented Jan 4, 2023

The problem that inspired this questions was type-annotating a decorator that takes a function and returns a function with an attribute added (see here) in my hobby project horology.

Mypy did not allow to inherit from Callable so I followed the suggestion from this answer to parameterize a Protocol:

F = TypeVar('F', bound=Callable)

class CallableWithInterval(Protocol[F]):
    interval: float
    __call__: F
    __name__: str

and then annotate the decorator as:

@overload
def timed(f: F) -> CallableWithInterval[F]: ...  # Bare decorator usage

...

If we then treat Callable as having also __name__ (and probably other attributes like __doc__), shouldn't we allow (in mypy) to inherit from it?

@JelleZijlstra
Copy link
Member

Shouldn't correctness be priority number one?

No, the first priority is to help users write better, more robust code. Overly pedantic type checks can hurt rather than help that goal.

If you want to pursue the change in this issue further, you could make a PR to mypy changing this behavior and see what the effects are in mypy-primer output (which shows new errors in various open-source projects).

@daniil-berg
Copy link
Author

This is a false dichotomy. "Better" and "more robust" are consequences of correctness first and foremost, among other things of course. Writing less correct code makes it worse and less robust, all else being equal.

It is very strange to even come across the term "overly pedantic" in the context of type safety checking. Now I am very curious: Are there other examples, where mypy purposefully holds onto incorrect behavior?

But fair enough. I'll look into the code and propose the change as a PR. I don't consider the number of new (correct) errors that change may cause to be of any importance to the argument at hand, but it will be an interesting exercise.

@helpmefindaname
Copy link

Is there currently a way to type a basically a "def" - a Callable that doesn't accept any class that implements __call__ ?

If not, I think adding a "def" to the typing module might be a good compromise, as then people using attributes such as __name__ could correctly type their usage.

@hauntsaninja
Copy link
Collaborator

@helpmefindaname yes there is, see Alex's comment above (about callback protocols)

@helpmefindaname
Copy link

@hauntsaninja I am not sure, if I don't get it but from my understanding Alex's comment is quite the opposite of my suggestion:
Not only does it still allow classes that implement __call__, but it also hides all the attributes a "def" usually has.
I'd expect to be able to access any of the attributes def has, but a class hasn't, while telling the caller that I intend to use them.
so referring to ['__annotations__', '__closure__', '__code__', '__defaults__', '__get__', '__globals__', '__kwdefaults__', '__name__', '__qualname__'].

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Jan 11, 2024

from __future__ import annotations
from typing import *

class HasName(Protocol):
    __named__: str  # modified, just for demonstration

def f(fn: Callable[[], None] & HasName):
    reveal_type(fn.__named__)  # str

playground
This approach works well in basedmypy, which supports intersections. So I'm happy to fix it there:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants