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

[flake8-slots] Rule SLOT002 doesn't take typing.NamedTuple into account #10805

Closed
autinerd opened this issue Apr 6, 2024 · 3 comments · Fixed by #10808
Closed

[flake8-slots] Rule SLOT002 doesn't take typing.NamedTuple into account #10805

autinerd opened this issue Apr 6, 2024 · 3 comments · Fixed by #10808

Comments

@autinerd
Copy link
Contributor

autinerd commented Apr 6, 2024

Hello 😊

While adding the SLOT rules to Home Assistant, I found that the SLOT002 doesn't take subclasses of typing.NamedTuple in account, although they are the typed version of collections.namedtuple.

Thanks in advance!

@AlexWaygood
Copy link
Member

Hi, thanks for opening the issue! Can you give a minimal code snippet as an example of where you think this rule should be emitting an error but isn't currently?

@AlexWaygood
Copy link
Member

AlexWaygood commented Apr 6, 2024

Ah, do you mean something like this?

from typing import NamedTuple

class Foo(NamedTuple("Foo", [("x", int)])):
    pass

It's unusual to inherit from a call-based typing.NamedTuple rather than to do something like

from typing import NamedTuple

class Foo(NamedTuple):
    x: int

And if you create a class-based typing.NamedTuple (the second way), it will create the __slots__ for you. But I suppose if you do inherit from a call-based typing.NamedTuple, we probably should emit the diagnostic the same as if it were a collections.namedtuple (and we currently don't).

@autinerd
Copy link
Contributor Author

autinerd commented Apr 6, 2024

Oh, I didn't know that this example:

from typing import NamedTuple

class Foo(NamedTuple):
    x: int

will have the __slots__ automatically created, then it is not needed of course, thanks.

And I didn't know about the call-based NamedTuple as well 😅 (it is not used in the Home Assistant codebase), but that would be a good idea to add.

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