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

returning an enum variant from a classmethod returning Self produces an error. #7798

Closed
ItsDrike opened this issue Apr 29, 2024 · 4 comments
Closed
Labels
as designed Not a bug, working as intended bug Something isn't working

Comments

@ItsDrike
Copy link

Describe the bug
As the title says, when returning a specific enum variant from a classmethod in that enum class, which is annotated with Self as the return type, pyright complains with:

Expression of type "Literal[Foo.X]" is incompatible with return type "Self@Foo"
  Type "Literal[Foo.X]" is incompatible with type "Self@Foo"  (reportReturnType)

However, if the classmethod is instead annotated as returning the class type (-> Foo for enum class Foo), no errors are reported.

Note that this seems to also be a regression, I found this when upgrading a fairly old project, I'm not sure since which exact version of pyright this started to fail, but I know it wasn't reported as an error on 1.1.326.

Code or Screenshots

Code sample in pyright playground

from enum import Enum, auto
from typing import Self

class Foo(Enum):
    X = auto()
    Y = auto()

    @classmethod
    def foo(cls) -> Self:
        return cls.X  # error
        
    @classmethod
    def bar(cls) -> "Foo":
       return cls.X  # works
@erictraut
Copy link
Collaborator

The current behavior is correct. If your intent is for foo to return an instance of Foo, it should be annotated as such. The Self annotation means that the method should return an instance of type[Self]. Previous versions of pyright had a bug where this error was not reported, but it has since been fixed. FWIW, mypy also generates an error for this code.

@erictraut erictraut closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
@erictraut erictraut added the as designed Not a bug, working as intended label Apr 29, 2024
@ItsDrike
Copy link
Author

ItsDrike commented Apr 29, 2024

Huh, but annotating it as Foo just means it returns an instance of Foo, which is in this case the same as an instance of type[Self] and I don't think you can even make a class that inherits from an existing enum, so why is Self incorrect?

I can see how annotating it as Foo might perhaps make more sense in that Self is usually meant to be used precisely for inheritance, but is there a reason why it would literally be incorrect to annotate this as Self? I was looking for an older issue which reported the bug you mentioned, but I didn't find it, could you perhaps share a link to it, if it explains the reasoning behind this? I'd love to check it out.

@erictraut
Copy link
Collaborator

Let's ignore the fact that it's an Enum for a moment and look at a more typical class. Do you understand why this would be a type error?

class Foo:
    @classmethod
    def bar(cls) -> Self:
        return Foo()

One could argue that if Foo were marked @final, then there's no way that the return value of bar could be anything other than a Foo instance, but special casing for final classes is not currently recognized or allowed by the typing spec with respect to Self type consistency. I don't think there's a good argument to be made for such a special case. As you said, Self is intended to be used to support inheritance. If a class doesn't support subtyping, then there's no good reason to use Self.

There has been an ongoing effort to clarify the typing spec and increase conformance with the spec. I've been diligently fixing bugs in pyright so it passes the conformance test suite. This change was part of that effort.

@ItsDrike
Copy link
Author

Let's ignore the fact that it's an Enum for a moment and look at a more typical class. Do you understand why this would be a type error?

class Foo:
    @classmethod
    def bar(cls) -> Self:
        return Foo()

One could argue that if Foo were marked @final, then there's no way that the return value of bar could be anything other than a Foo instance, but special casing for final classes is not currently recognized or allowed by the typing spec with respect to Self type consistency.

Yeah, this makes perfect sense. What's surprising that even if we assumed for a second that enums would support inheritance, in the example, I don't use Foo.VARIANT, I use cls.VARIANT, so the VARIANT should theoretically just be inherited anyways, and be an instance of Self, since cls is type[Self]. Which is why it's surprising to see that this produces an error.

Now that I'm thinking about that, is it because of the way the variants are created, where they're basically class variables holding instances of the Foo class specifically, which means even though they're accessed through cls, their type is Foo rather than Self?

I don't think there's a good argument to be made for such a special case. As you said, Self is intended to be used to support inheritance. If a class doesn't support subtyping, then there's no good reason to use Self.

The reason I used Self here is mainly to avoid having to potentially rename all of the occurences of -> Foo if the Foo class gets renamed, I know it's not really much of a hasle, pyright LSP even has the rename feature haha, but it still just felt cleaner for some reason.


Note that I don't want to bother you too much with this, so it's perfectly fine if you're too busy / have more important things to do, I completely understand, I'm just curious about this.

ItsDrike added a commit to py-mine/mcproto that referenced this issue Apr 29, 2024
This mainly addresses two types of issues:

- <microsoft/pyright#7798>: Using `Self` as a
  return type from `Enum` classmethods isn't supported, the class (name)
  should be used instead.
- <microsoft/pyright#5933>: Mutable field
  invariance - subclasses shouldn't be able to contravariantly narrow an
  inherited variable.
ItsDrike added a commit to py-mine/mcproto that referenced this issue Apr 29, 2024
This mainly addresses two types of issues:

- <microsoft/pyright#7798>: Using `Self` as a
  return type from `Enum` classmethods isn't supported, the class (name)
  should be used instead.
- <microsoft/pyright#5933>: Mutable field
  invariance - subclasses shouldn't be able to contravariantly narrow an
  inherited variable.
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