python / cpython

The Python programming language
https://www.python.org
Other
63.15k stars 30.24k forks source link

typing.NamedTuple with default arguments without type annotations is falsy #90130

Open 9d440abf-3ac8-4ed7-8860-bff675b85c27 opened 2 years ago

9d440abf-3ac8-4ed7-8860-bff675b85c27 commented 2 years ago
BPO 45972
Nosy @gvanrossum, @ericvsmith, @TeamSpen210, @Fidget-Spinner, @AlexWaygood, @emontnemery

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.8', 'type-bug', 'library', '3.9'] title = 'typing.NamedTuple with default arguments without type annotations is falsy' updated_at = user = 'https://github.com/emontnemery' ``` bugs.python.org fields: ```python activity = actor = 'emontnemery' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'emontnemery' dependencies = [] files = [] hgrepos = [] issue_num = 45972 keywords = [] message_count = 7.0 messages = ['407570', '407571', '407572', '407573', '407575', '407576', '407579'] nosy_count = 6.0 nosy_names = ['gvanrossum', 'eric.smith', 'Spencer Brown', 'kj', 'AlexWaygood', 'emontnemery'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue45972' versions = ['Python 3.8', 'Python 3.9'] ```

9d440abf-3ac8-4ed7-8860-bff675b85c27 commented 2 years ago

typing.NamedTuple behaves in surprising ways when it has default arguments which lack type annotations:

>>> from typing import NamedTuple
>>> class MyTuple(NamedTuple):
...     a = 1000
...
>>> tmp = MyTuple()
>>> tmp.a
1000
>>> len(tmp)
0
>>> bool(tmp)
False

Tested in Python 3.8 and 3.9

8620fd7d-208a-4c5d-884a-ece73071c881 commented 2 years ago

What's happening is that typing.NamedTuple ignores non-annotated attributes entirely when computing the names it passes along to namedtuple(), so here "a" is just a class attribute. You're accessing it from there, but the tuple itself is entirely empty. Perhaps it should error out if no names at all are found?

ericvsmith commented 2 years ago

I don't think we'd want to prohibit zero-length namedtuples (or NamedTuples). I've used them, especially when I'm dynamically creating them.

This is just a side effect of how Python works. I don't think there's anything to do here, except maybe mention it in the docs, and I'm not even sure that's a good idea.

AlexWaygood commented 2 years ago

I agree that prohibiting zero-length NamedTuples seems like a bad idea, and also agree that this probably doesn't need to be documented.

The behaviour here definitely looks weird at first glance, but it's probably not a good idea to tamper with the __bool__() methods of NamedTuple classes either: an empty NamedTuple probably *should* be falsey by default, even if it has a class attribute.

9d440abf-3ac8-4ed7-8860-bff675b85c27 commented 2 years ago

I think elaborating in the documentation that only annotated attributes make it to the underlying namedtuple() would be helpful, it's not obvious that they are instead just class attributes.

AlexWaygood commented 2 years ago

To me, the fact that NamedTuple uses class attributes to provide field defaults feels like an implementation detail that is only relevant to an unusual edge case.

Where do you think the documentation should be improved, and what is your suggested wording? :)

9d440abf-3ac8-4ed7-8860-bff675b85c27 commented 2 years ago

Maybe something like this:

diff --git a/Doc/library/typing.rst b/Doc/library/typing.rst
index 735d477db4..8de913d8db 100644
--- a/Doc/library/typing.rst
+++ b/Doc/library/typing.rst
@@ -1291,7 +1291,8 @@ These are not used in annotations. They are building blocks for declaring types.

 .. class:: NamedTuple

-   Typed version of :func:\`collections.namedtuple\`.
+   Typed version of :func:\`collections.namedtuple\`, annotated fields are passed
+   to an underlying \`collections.namedtuple\`.

    Usage::

@@ -1311,9 +1312,20 @@ These are not used in annotations. They are building blocks for declaring types.

\```py
       employee = Employee('Guido')
       assert employee.id == 3
+      assert employee == ('Guido', 3)
\```

    Fields with a default value must come after any fields without a default.

+   Non-annotated fields will not be part of the `collections.namedtuple`::
+
+      class Employee(NamedTuple):
+          name = 'Guido'
+          id = 3
+
+      employee = Employee()
+      assert employee.id == 3
+      assert not employee  # Passes because the collections.namedtuple is empty
+
    The resulting class has an extra attribute ``__annotations__`` giving a
    dict that maps the field names to the field types.  (The field names are in
JelleZijlstra commented 2 years ago

Agree that it's good to document that typing.NamedTuple only picks up annotated attributes. Would you be willing to submit a PR?

gvanrossum commented 2 years ago

To me, the fact that NamedTuple uses class attributes to provide field defaults feels like an implementation detail that is only relevant to an unusual edge case.

This implementation detail would have to be part of the specification of NamedTuple though, right? It doesn't feel like something that we should leave unspecified.

>>> class C(NamedTuple):
...   a: int = 0
...   b: int
...   c = 42
...
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\gvanrossum\AppData\Local\Programs\Python\Python310\lib\typing.py", line 2274, in __new__
    raise TypeError(f"Non-default namedtuple field {field_name} "
TypeError: Non-default namedtuple field b cannot follow default field a
>>> class C(NamedTuple):
...   a: int
...   b: int = 0
...   c = 42
...
>>> C(1000)
C(a=1000, b=0)
>>>