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

Automatically use Self type even if it doesn't appear in method annotation #14075

Open
ilevkivskyi opened this issue Nov 12, 2022 · 3 comments
Open
Labels

Comments

@ilevkivskyi
Copy link
Member

This is a follow up for #14041

This example gives an error:

from typing import Self

class C:
    x: Self
    def meth(self) -> None:
        self.x = self  # E: Incompatible types in assignment (expression has type "C", variable has type "Self")

Although it is unfortunate, technically this is required by PEP 484, that says that if no annotation is given, self has a type of current class (note also we can't bind self type for self.x expression giving it type C, as this will open another unsafety). This can be avoided by using e.g. self: Self (or generally Self type anywhere in the signature).

It looks like there can be only two solutions:

  • Always infer Self type for self. This will cause a big performance penalty and may be surprising for users who are not familiar with Self (if they use reveal_type(self)).
  • Use some heuristic and only infer Self where otherwise we will have this problem. For example, infer Self if an attribute with type containing Self is used anywhere in a method.

I am leaning towards the latter, but didn't want to do this until we get some more experience.

@erictraut
Copy link

erictraut commented Nov 13, 2022

For what it's worth, pyre and pyright both adopted the former approach. (PEP 673, which introduces the Self type, was authored by the pyre team, and pyre was the reference implementation for it.)

Pyright has had support for Self for about a year now, and we haven't received any questions or complaints about inferring the type Self for self parameters. We likewise infer typeType[Self] for cls parameters.

Screenshot 2022-11-12 at 4 09 00 PM

Screenshot 2022-11-12 at 4 13 12 PM

@DevilXD
Copy link

DevilXD commented Nov 24, 2022

I think PEP 484 has not expected to ever be a Self type available at the time of it's writing. I'm also leaning towards just inferring Self by default. If a lack of user experience is factor here, this addition can be delayed until 1-2 Python versions pass, so that the Self type has a chance to "appear in the wild" (as it's fairly new) and for users to gain said experience. Although on the other hand, there's a high chance that someone who's following the typing news, even not regularly, has already heard and learned about the Self type being a thing, so there's not much reasons to even wait. At least that's how I see it in my opinion.

@AlexWaygood
Copy link
Member

I just closed #14708 as a duplicate of this issue

ilevkivskyi added a commit that referenced this issue Jun 29, 2023
Fixes #15529

The fix is straightforward, hopefully there will be no fallout. (Note
that #14075 would also fix this, but I am still not sure we should do
that)

---------

Co-authored-by: Shantanu <12621235+hauntsaninja@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants