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

Protocols should not care if a type is a class or an instance #4536

Closed
chadrik opened this issue Feb 3, 2018 · 19 comments · Fixed by #13501
Closed

Protocols should not care if a type is a class or an instance #4536

chadrik opened this issue Feb 3, 2018 · 19 comments · Fixed by #13501
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-0-high topic-protocols

Comments

@chadrik
Copy link
Contributor

chadrik commented Feb 3, 2018

As a developer of a protocol, I don't care whether that protocol is met by an instance or a class, as long as the necessary functions and variables are present and correct. Currently it appears that only instances can satisfy a protocol.

For example, the code below attempts to meet a protocol using an instance and a class. It executes in python fine, but mypy complains that Type[Jumper1] is not compatible with the Jumps protocol:

from typing_extensions import Protocol

class Jumps(Protocol):
    def jump(self) -> int:
        pass

class Jumper1:
    @classmethod
    def jump(cls) -> int:
        print("class jumping")
        return 1

class Jumper2:
    def jump(self) -> int:
        print("instance jumping")
        return 2

def do_jump(j: Jumps):
    print(j.jump())

do_jump(Jumper1)
do_jump(Jumper2())
@chadrik chadrik changed the title Protocols should respect classmethods and class variables Protocols should not care if a type is a class or an instance Feb 3, 2018
@glyph
Copy link

glyph commented Jun 7, 2018

I just hit this as well. It looks like this and #5018 are the same issue, but about modules rather than classes.

I think the same thing probably should be true of instances as well. If I do, for example:

class Empty: pass
nothing = Empty()
def free_jump() -> int:
    return 3
nothing.jump = free_jump
do_jump(nothing)

I feel like that ought to work.

Zope Interface has this notion of "provides" as distinct from "implements" to draw a clean separation between the different ways a type can relate to an Interface; it seems like Protocol should behave in the same way. It has a directlyProvides function which can be used to annotate any object, including classes and modules, to indicate that it provides the interface.

@gvanrossum
Copy link
Member

I agree with the OP's initial example (and with #5018), but I think Glyph's example goes a bit too far -- it dynamically modifies an object. While in the toy example it is easy to see that nothing has a jump attribute, in realistic applications I doubt that in most cases mypy would be able to make the connection.

IIUC the provides/implements distinction is between a class whose instances can jump() vs. an instance that can jump()? (Though I'm not all that familiar with Zope Interface, and the two words don't have clearly distinct meanings to me -- I could easily have it backwards.)

@ilevkivskyi
Copy link
Member

I agree with the OP's initial example (and with #5018), but I think Glyph's example goes a bit too far

The same for me.

@ilevkivskyi ilevkivskyi added bug mypy got something wrong priority-1-normal topic-protocols false-positive mypy gave an error on correct code labels Jun 8, 2018
@ilevkivskyi ilevkivskyi self-assigned this Jun 8, 2018
@ilevkivskyi
Copy link
Member

Note to self: we need to add an OP example (simplified) to the PEP 544 (there is already a PR for modules).

@glyph
Copy link

glyph commented Jun 14, 2018

IIUC the provides/implements distinction is between a class whose instances can jump() vs. an instance that can jump()?

Yes.

To restate:

An object x is said to "provide" the interface IX if a function that "takes an IX" can take x and behave correctly; i.e. it matches the specification presented by IX.

A type X is said to "implement" the interface IX if all its instances provide IX.

The distinction is important because if IX specifies methods a, b, and c, the type X might:

  • have all three of those attributes just because it implements the interface, but this of course doesn't imply that it provides said interface,
  • implement the interface using some runtime trickery and not itself have any of those attributes,
  • have all three of those attributes because it directly provides the interface; if it does so, depending on descriptor semantics, its instances may or may not provide the interface; i.e. it may provide but not implement the interface.

@glyph
Copy link

glyph commented Jun 14, 2018

Regarding my setting-attributes-on-an-instance example, I agree that as stated it's perhaps pushing mypy a bit far. Practically I do not expect it to be able to do that automatically without help. What I want is maybe something like dynamic_cast in C++, for @runtime Protocols only.

@gvanrossum
Copy link
Member

OK, so "provides" is a property of objects while "implements" is a property of types -- with the understanding that a class is both an object and a type. And the distinction is an important once especially when you're doing runtime checking and all you can check is the presence of specific attributes, rather than the full signatures of methods.

(I've got to admit that even after writing that down the choice of terms still feels a bit arbitrary. Especially "implements" could go either way in my intuition.)

In general the checks that mypy performs are about "provides" -- e.g. we have a function foo:

class P(Protocol): ...
def foo(arg: P):
    ...

and we're calling it with an expression:

foo(expr)

then mypy checks whether expr provides P. In the usual case, this boils down to checking whether the type of expr implements P.

But when expr is a class object, checking whether that class provides P roughly requires looking for matches of P's methods among that class's class methods and static methods. (We should first check whether its metaclass implements P, given that classes are objects.)

Finally when expr is a module object we should again check whether the module provides P by matching the functions it defines against the methods of P (again after checking whether ModuleType might implement P).

I suppose in the wonderful world of Zope Interfaces there are other cases to consider, since providing an interface could be done dynamically. But mypy is no help here, and I think the examples of dynamically providing an interface that I've seen all deserve the term "hack". :-)

ilevkivskyi added a commit to python/peps that referenced this issue May 13, 2019
This PR contains mostly minor wording tweaks plus a paragraph explicitly allowing class objects as implementations of protocols, previously there were questions whether it is actually allowed, see python/mypy#4536.
@ilevkivskyi
Copy link
Member

A small note about this, mypy should accept not only Type[X] as an implementation, but also a Callable, if the fallback is appropriate. The latter actually currently works but only partially, in particular __call__ is lost when switching to the fallback instance.

Also I raise priority to high, since this is a required part of PEP 544 in its accepted version.

@the-vindicar
Copy link

Seeing how the issue is still open, and how the described behaviour is still observed, I have to ask: are there any known workarounds for this outside of completely disabling typechecks for the relevant code parts?
Total redesign involving getting rid of the classmethods is not always an option.

@dam5h
Copy link

dam5h commented Sep 15, 2021

Bumping this as I just ran into it as well, any workarounds?

ilevkivskyi added a commit that referenced this issue Aug 24, 2022
Fixes #4536

This use case is specified by PEP 544 but was not implemented. Only instances (not class objects) were allowed as protocol implementations. The PR is quite straightforward, essentially I just pass a `class_obj` flag everywhere to know whether we need or not to bind self in a method.
@gvanrossum
Copy link
Member

Cool! What about modules? Can they implement protocols?

@ilevkivskyi
Copy link
Member

Sure, tomorrow.

@hauntsaninja
Copy link
Collaborator

#5018 :-)

@hauntsaninja
Copy link
Collaborator

@matejcik (or anyone else on this issue) apologies for the somewhat random ping, but I'd be interested in knowing if you have any open source code that makes use of this feature

@matejcik
Copy link

@hauntsaninja sure, two off the top of my head
one: the PathSchemaType is a very simple protocol built off the PathSchema class, but also implemented by "static" versions
AlwaysMatchingSchema and NeverMatchingSchema could be singletons, but there's also zero reason to do it that way. (esp. given that this is in a constrained environment, I don't want to have to have useless instances around)
also in this case I could technically replace the protocol with a Callable[[arg], bool], but then I'd have to pre-filter my PathSchema objects when passing them around, or implement __call__ or do something similarly distasteful in the name of satisfying the typechecker.

two: this protocol is semantically a protocol in that it is just a namespace providing some functions.
One implementation is a class whose instances have some state.
The other is basically just a namespace; it would be perfectly good as a module except that we wanted to have it in the same file.

@hauntsaninja
Copy link
Collaborator

Awesome, thank you! I'll add https://github.com/trezor/trezor-firmware to https://github.com/hauntsaninja/mypy_primer once the next release is out

@lundybernard
Copy link

This is causing a new issue with dataclass protocols. In my case, I need to verify that a ConfigurationClass is a dataclass, or has a dataclass_fields attribute. This changes gets us a lot closer, but It looks like there is a problem when both the Class and Instances provide the same interface

from dataclasses import dataclass, Field
from typing import Union, Protocol, Dict, runtime_checkable

ConfigValue = Union['ConfigProtocol', str]

class FieldProtocol(Protocol):
    type: ConfigValue
    name: str

@runtime_checkable
class ConfigProtocol(Protocol):
    __dataclass_fields__: Dict[str, Field[ConfigValue]]
    __module__: str
    
class Configuration:
    def __init__(self, config_class: ConfigProtocol):
        self._config_class = config_class

@dataclass
class ConfigClass:
    key: str = 'v_1'

@dataclass
class GlobalConfig:
    TestModule: ConfigClass
    config_file: str = 'GlobalConfig.yaml'

conf = Configuration(GlobalConfig)

Results in a new mypy error:

error: Argument 2 to "Configuration" has incompatible type "Type[GlobalConfig]"; expected "ConfigProtocol"
note: Only class variables allowed for class object access on protocols, __dataclass_fields__ is an instance variable of "GlobalConfig"
note: Only class variables allowed for class object access on protocols, __module__ is an instance variable of "GlobalConfig"

In this particular case, the values are identical for both the Class and Instance objects:

In []: GlobalConfig.__dataclass_fields__ == gc.__dataclass_fields__
Out[]: True

In []: GlobalConfig.__module__ == gc.__module__
Out[]: True

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Sep 23, 2022

@lundybernard thanks for testing! I think we just need to mark __dataclass_fields__ as a ClassVar, so #13721 should fix.

We'd need a similar change in typeshed for __module__ python/typeshed#8787, although you should probably just remove it from the Protocol because it's not going to do anything (since object is typed as having __module__).

@lundybernard
Copy link

@lundybernard thanks for testing! I think we just need to mark __dataclass_fields__ as a ClassVar, so #13721 should fix.

We'd need a similar change in typeshed for __module__ python/typeshed#8787, although you should probably just remove it from the Protocol because it's not going to do anything (since object is typed as having __module__).

Thanks! I'll test out #13721 when it gets merged, or I have some extra time.

module is technically a requirement, as our code needs to read it, but I suppose its safe to assume it will always exist on any object capable of satisfying the Protocol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-0-high topic-protocols
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants