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

Mypy rejects valid nested class inside of a NamedTuple class definition #5362

Open
gwk opened this issue Jul 15, 2018 · 7 comments · May be fixed by #15064
Open

Mypy rejects valid nested class inside of a NamedTuple class definition #5362

gwk opened this issue Jul 15, 2018 · 7 comments · May be fixed by #15064
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-2-low topic-named-tuple

Comments

@gwk
Copy link

gwk commented Jul 15, 2018

I think I've found a bug in NamedTuple handling:

from enum import Enum
from typing import NamedTuple

class T(NamedTuple):
  class State(Enum):
    A =1
  state:State

print(T.State.A)
print(T(state=T.State.A))

This runs fine in Python3.7 but mypy master head issues the following error:

Invalid statement in NamedTuple definition; expected "field_name: field_type [= default]"

setup.cfg:

[mypy]
python_version = 3.6
cache_dir = _build/mypy_cache
mypy_path = ~/work/pithy:./lambda
check_untyped_defs = True
disallow_subclassing_any = True
disallow_untyped_calls = True
disallow_untyped_defs = False
ignore_missing_imports = False
show_column_numbers = True
show_none_errors = True
strict_boolean = False
strict_optional = True
warn_incomplete_stub = True
warn_no_return = True
warn_redundant_casts = True
warn_return_any = True
warn_unused_configs = True
warn_unused_ignores = True
@ethanhs
Copy link
Collaborator

ethanhs commented Jul 15, 2018

It seems we miss this case in check_namedtuple_classdef here: https://github.com/python/mypy/blob/master/mypy/semanal_namedtuple.py#L57. I believe we would also need to handle adding this class to the generated typeinfo. @gwk would you be interested in trying to fix this?

@ethanhs ethanhs added bug mypy got something wrong topic-named-tuple priority-2-low false-positive mypy gave an error on correct code labels Jul 15, 2018
@JelleZijlstra
Copy link
Member

I'm not sure we should support this. Defining a class inside a NamedTuple class doesn't seem like the intended usage of NamedTuple.

@ethanhs
Copy link
Collaborator

ethanhs commented Jul 16, 2018

I'm not sure we should support this.

I'm okay with that too. I agree it doesn't make a lot of sense in this context.

@gwk
Copy link
Author

gwk commented Jul 16, 2018

I'm a little surprised that you would deem legal usage undesirable so quickly.

Nested classes are a feature of the language that can be used in a variety of ways. NamedTuple is a way of conveniently generating classes with certain very desirable properties, namely immutability and sequential access. These two features compose well in Python, and for Mypy to prohibit that composition just makes Mypy less useful.

In my own humble opinion, "Intended usage" is perhaps not the only way that Mypy should consider the language. The language designers have not and cannot predict all of the ways that various features will be productively used or can interact with each other. I would think that the goal of the type checker is to prohibit pathological cases, and not ones that users are reporting as useful!

In any case, thank you for considering this. @ethanhs if I'm really the only one who sees value in this, I could take a stab at a patch.

@asottile
Copy link
Contributor

If you're looking for a concrete use case, here's where I just hit this:

class PR(NamedTuple):
    date: datetime.date
    link_text: str
    link_url: str
    title: str

    class PRDisplay(NamedTuple):
        date: str
        link: str
        title: str

    @property
    def display(self) -> 'PR.PRDisplay':
        return PR.PRDisplay(
            self.date.isoformat(), f'[{self.link_text}]', self.title,
        )

@NMertsch
Copy link

Regarding

Defining a class inside a NamedTuple class doesn't seem like the intended usage of NamedTuple.

-- @JelleZijlstra

I agree. The docs say that typing.NamedTuple is a "Typed version of collections.namedtuple()". collections.namedtuple does not allow nested classes, so I guess neither should typing.NamedTuple (at least in a type-checking context).

(Sorry for being late to the party, but I stumbled across a similar use case.)

@hmc-cs-mdrissi
Copy link
Contributor

hmc-cs-mdrissi commented Apr 16, 2023

Nested collections.namedtuple are possible and do happen in some libraries like here. That's main reason I hit this issue and made a pr to support.

A secondary reason to support is consistency with other type checkers. pyright supports nested namedtuples and pytype also seems fine with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong false-positive mypy gave an error on correct code priority-2-low topic-named-tuple
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants