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

typing.Protocol classes should probably be ignored #130

Closed
ItsDrike opened this issue Dec 29, 2022 · 4 comments
Closed

typing.Protocol classes should probably be ignored #130

ItsDrike opened this issue Dec 29, 2022 · 4 comments

Comments

@ItsDrike
Copy link

Currently, slotscheck is marking classes that inherit from typing.Protocol as regular classes, and reports missing slots. However protocol classes are usually not used for actual inheritance, and defining __slots__ on them would mean a type-checker would be checking for presence of __slots__ when determining whether a type can be downcasted to given protocol class.

Here's an example of this:

from typing import Protocol, Callable, TypeVar, ParamSpec
from functools import wraps

P = ParamSpec("P")
R = TypeVar("R")

class DecoratorFunction(Protocol):
    def __call__(self, __x: Callable[P, R]) -> Callable[P, R]:
        ...
        
def log_with_markers(x: int, y: int) -> DecoratorFunction:
    def inner(func: Callable[P, R]) -> Callable[P, R]:

        @wraps(func)
        def wrapper(*args: P.args, **kwargs: P.kwargs) -> R:
            print(f"Function called, markers: {x=}, {y=}")
            return func(*args, **kwargs)

        return wrapper
    return inner

The DecoratorFunction class shouldn't be slotted, and slotscheck probably shouldn't enforce the presence of __slots__ there.

@ariebovenberg
Copy link
Owner

ariebovenberg commented Dec 29, 2022

Good find, I agree! A special case was needed for TypedDict instances as well (#120).

A nuance here is that it is possible to inherit from protocols, and then we do want to enforce them (at least, with strict-subclass option):

class A(Protocol):
    foo: int

class B(A):  # using A as explicit base class requires slots there
    __slots__ = ...
    foo: int
    bar: str

@ariebovenberg
Copy link
Owner

ariebovenberg commented Dec 29, 2022

version 0.16.2 is out, which should fix this. Feel free to re-open if it doesn't work for you.

@ItsDrike
Copy link
Author

Hmm, well it does work when Protocol is imported directly from typing or from typing_extensions, though the annoying thing is that my library doesn't have typing_extensions dependency available on runtime, so instead what we're doing is something like:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing_extensions import Protocol
else:
    Protocol = object
    
    
class MyProtocol(Protocol):
    ...

Though it's probably impossible for slotscheck to handle something like this without introducing some complex static analysis. Oh well, I suppose we can just make typing_extensions a runtime dependency, it will make a few other things easier anyways.

Thanks for addressing this so quickly though!

@ariebovenberg
Copy link
Owner

ariebovenberg commented Dec 30, 2022

You can always add a specific ignore regex, but that would be quite cumbersome. Something like #71 would be useful in your case, I think.

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

2 participants