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

Attribute with the same name as a class in type annotation #1775

Closed
JukkaL opened this issue Jun 30, 2016 · 13 comments
Closed

Attribute with the same name as a class in type annotation #1775

JukkaL opened this issue Jun 30, 2016 · 13 comments
Labels

Comments

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 30, 2016

This was reported by Agustin Barto:

class A1:
    pass

class B:
    a1 = None  # type: A1  # Works fine

class C:
    A1 = None  # type: A1  # Complains about Invalid type "A1"

A1Alias = A1
class D:
    A1 = None  # type: A1Alias  # Works

I wonder if the body of C should be valid?

@gvanrossum
Copy link
Member

There's a big code smell though, using the class name as the name for a variable containing instances of that class. I'm not sure if we should encourage that.

Reminds me a bit of #1776 though.

@tharvik
Copy link
Contributor

tharvik commented Jul 18, 2016

Same issue found with a method definition

class A: pass

class B:
    def a(self) -> A: ...
    def A(self) -> A: ...
    def b(self) -> A: ...

gives

/tmp/asd.py: note: In function "A":
/tmp/asd.py:5: error: Invalid type "A"
/tmp/as3.py: note: In function "b":
/tmp/asd.py:6: error: Invalid type "A"

Also, the error is only raised after the line the name is redefined and then can't be used again.

It is clearly a code smell, but it was found in stdlib while typing datetime.datetime.tzinfo which return a tzinfo.

@JukkaL
Copy link
Collaborator Author

JukkaL commented Jul 18, 2016

I'd introduce a type alias to work around the issue. For example:

class A: pass

_A = A

class B:
    def a(self) -> _A: ...
    def A(self) -> _A: ...
    def b(self) -> _A: ...

@tharvik
Copy link
Contributor

tharvik commented Jul 18, 2016

@JukkaL that's working, I was finding a better example, but it seems that it was more related to #1637; thanks

@dmoisset
Copy link
Contributor

dmoisset commented Sep 6, 2016

I found other cases of this, even when the name is declared as an attribute. For example this:

class Sentence:
    def __init__(self):
        self.subject = "Bob"
        self.verb = "writes"
        self.object = "a letter"

    def __eq__(self, other: object) -> bool:
        return isinstance(other, Sentence) and other.verb == self.verb

This fails with Invalid type "object" on the definition of __eq__, just because there is an "self.object" attribute in the class, even if python's scope rules indicate that the annotation refers to the object builtin, not the object attribute.

(As a side note, this could get even more confusing with PEP526, you'll have a object: str declaration at the class body level)

@gvanrossum gvanrossum removed this from the 0.5 milestone Mar 29, 2017
@ahawker
Copy link

ahawker commented Jun 26, 2017

Ran into this one tonight in the ulid-py library after chasing my tail a bit.

Example:

    @property
    def bytes(self) -> bytes:
        """
        Computes the bytes value of the underlying :class:`~memoryview`.

        :return: Memory in bytes form
        :rtype: :class:`~bytes`
        """
        return self.memory.tobytes()

Output:

⇒  mypy ulid
ulid/ulid.py:113: error: Invalid type "bytes"

It may be code smell (property matching a builtin type name) but it's following the same pattern as the stdlib uuid.UUID class.

I'll likely just define some type hint aliases to work-around the problem but it would be nice to see addressed if possible, or at the very least, a more "helpful" error message if this case is detectable vs. other "Invalid type" errors.

@gvanrossum
Copy link
Member

gvanrossum commented Jun 26, 2017

I think that in this case mypy doesn't treat the scopes the same way as Python does at runtime, and the example

@property  # or without this
def bytes(self) -> bytes:

should be supported. Though note that the following will be wrong at runtime:

class C:
  def bytes(self) -> bytes: ...
  def frombytes(self, x: bytes): ...

Here, C.__annotations__['frombytes'] is a reference to the bytes method, not to the bytes type! So the problem is order-dependent, and defining an alias is probably a good defensive solution.

ahawker added a commit to ahawker/ulid that referenced this issue Jun 27, 2017
@ahawker
Copy link

ahawker commented Jun 27, 2017

@gvanrossum Thanks for the detailed explanation; defining aliases for those name clashes worked out.

@ilevkivskyi ilevkivskyi added the false-positive mypy gave an error on correct code label May 17, 2018
@JukkaL
Copy link
Collaborator Author

JukkaL commented Jan 28, 2020

This works at least a little better now since mypy switched to the new semantic analyzer. Otherwise, the alias trick seems like a reasonable workaround.

@tristanlatr
Copy link

This issue should probably be re-open.
It seem that pyright and mypy are not in agreement about the resolving order of annotation.

This code works both on mypy and pyright (with the difference that mypy needs the TypeAlias and pyright doesn't):

from __future__ import annotations
from typing import TypeAlias
class C:
  F:TypeAlias = bytes
  def bytes(self) -> F:
    return b''

But the following code causes mypy to complain about F being not a valid type.

from __future__ import annotations
class C:
  F = b'not a valid type'
  def bytes(self) -> F:
    return F()

class F:
  ...

Looks like pyright starts by looking for annotation names in the global scope, then tries the locals (where mypy does the opposite).

@JukkaL
Copy link
Collaborator Author

JukkaL commented Mar 3, 2023

The behavior of mypy matches what happens at runtime, which is usually what we want:

class C:
    F = b'foo'
    def f(self) -> F: ...
print(C.f.__annotations__)  # {'return': b'foo'}

@tristanlatr
Copy link

@JukkaL note the future import. In this case the runtime behavior is not helping.

@tristanlatr
Copy link

I've open this issue on pyright's repo, and it looks like their conclusion is that mypy is inconsistent in this particular case.

A clarification regarding pep 563 and it's implication on scoping rules of annotations would be good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants