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

Subclassing a dataclass protocol incorrectly flags not implementing all required members #6675

Closed
alwaysmpe opened this issue Dec 7, 2023 · 5 comments
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@alwaysmpe
Copy link

When subclassing a protocol class that has been annotated with @dataclass pyright flags an error that members are not implemented. These members are implicitly implemented by the subclass/dataclass annotation. Subclassing from a protocol with an __init__ that assigns these values explicitly doesn't cause an error.

I would expect that neither are errors.

Code Snippet

from dataclasses import dataclass
from typing import Protocol

class BaseProtocol(Protocol):
    a: int

    def __init__(self, a: int) -> None:
        self.a = a

class SubProtocol(BaseProtocol):
    def print_a(self):
        print(self.a)

@dataclass
class BaseDataclassProtocol(Protocol):
    a: int

@dataclass
class SubDataclassProtocol(BaseDataclassProtocol):
    def print_a(self):
        print(self.a)

a = SubDataclassProtocol(2)
b = SubProtocol(2)
a.print_a()
b.print_a()

Expected Behaviour
Both classes should pass type checking.

Actual Behaviour
Pyright flags an error on SubDataclassProtocol. Code works correctly.

/notebooks/protocol_bug.py
  /notebooks/protocol_bug.py:20:7 - error: Class derives from one or more protocol classes but does not implement all required members
    Member "a" is declared in protocol class "BaseDataclassProtocol" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations 
@alwaysmpe alwaysmpe added the bug Something isn't working label Dec 7, 2023
@erictraut
Copy link
Collaborator

I'm not sure that it makes sense to allow @dataclass to be used in conjunction with a Protocol. After all, Protocol is meant to define a structural type. It's not clear what it would mean to combine the two. The fields of the dataclass are implemented in the class (by virtue of the @dataclass decorator), so they are not structural.

My inclination is to change pyright to detect this combination and report it as an error so it is disallowed. I would want to consult with the broader typing community before doing so.

I'd also like to understand your intent in trying to combine these two concepts. Why are you using protocols here? This seems like a misuse of the concept.

@erictraut
Copy link
Collaborator

FWIW, mypy issues the same error as pyright in this case. But I'm guessing that's because the mypy authors (like me) didn't contemplate anyone every trying to use @dataclass together with Protocol.

@alwaysmpe
Copy link
Author

Python lets me do weird things, so I try to do those things and see what breaks. It's a weird combination but I make prolific use of dataclasses. I don't remember the last time I wrote a real __init__ method.

I personally prefer having pyright agree with what happens at runtime. But @dataclass optionally adds some extra methods like comparison and equality operators. I would say it's reasonable to exclude those from the protocol definition. The PEP explicitly gives the example of subclassing a Protocol, but says nothing about __init__ methods. If the decision is to not support @dataclass , ideally it would instead emit an error.

@alwaysmpe
Copy link
Author

alwaysmpe commented Dec 7, 2023

I was switching to a protocol to try to get round the issue described in #2601 with some classes directly implementing the protocol, others indirectly.

@erictraut
Copy link
Collaborator

Thanks for the additional information.

The issue goes beyond the __init__ method. That's not what pyright is complaining about here. Pyright is complaining that you haven't implemented a in your protocol, not __init__.

As I said above, I don't think it makes sense to use @dataclass with Protocol based on the intended meaning of Protocol in the type system. The fact that it "works" at runtime is a weak argument because there are many things that work at runtime that are not allowed in the type system, and that's by design.

For now, I'm going to close this issue without making any changes to pyright. I think it's arguably doing the right thing in the face of a misuse of Protocol, and I think it's unlikely that other people will attempt to use this combination as you've done. If my assumption turns out to be incorrect, I'm willing to revisit this issue and work to refine the typing spec to clarify the meaning of @dataclass used with Protocol.

@erictraut erictraut added as designed Not a bug, working as intended and removed needs decision labels Dec 7, 2023
@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
as designed Not a bug, working as intended bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants