[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Ivan Levkivskyi]

Ivan Levkivskyi  added the comment:

> [..] but I think it's the best we can do. It's consistent with any other 
> class derived from tuple or list: [...]

I agree with this argument. Sorry for delay with my response and thank you Eric 
for taking care about this issue!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Change by Eric V. Smith :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:


New changeset 78aa3d8f5204bc856d7b2eb67288cf90c6a30660 by Eric V. Smith (Miss 
Islington (bot)) in branch '3.7':
bpo-34363: dataclasses.asdict() and .astuple() now handle fields which are 
namedtuples. (GH-9151) (GH-9304)
https://github.com/python/cpython/commit/78aa3d8f5204bc856d7b2eb67288cf90c6a30660


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[miss-islington]

Change by miss-islington :


--
pull_requests: +8733

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:


New changeset 9b9d97dd139a799d28ff8bc90d118b1cac190b03 by Eric V. Smith in 
branch 'master':
bpo-34363: dataclasses.asdict() and .astuple() now handle fields which are 
namedtuples. (GH-9151)
https://github.com/python/cpython/commit/9b9d97dd139a799d28ff8bc90d118b1cac190b03


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

The question here is: what should asdict() return if the dataclass contains a 
namedtuple? What the code is trying to do (but currently failing!) is to return 
another namedtuple, but with the values returned by recursively calling in to 
asdict() (or rather, its helper function) for each field in the namedtuple.

I think this is the correct behavior. Specifically, I do not want to call the 
namedtuple's _asdict() method. There are two problems with _asdict():

1. It doesn't recurse in to the fields, like asdict() normally does with a 
dict, list, or tuple.

2. It returns a dict! This is a problem because if a dataclass field contains a 
dict which has a key which is a namedtuple, then asdict() would fail.

Here's an example of #2 above, if asdict() on a namedtuple field returns a dict:

@dataclass
class C:
f: 'Any'

T = namedtuple('T', 'a')

c = C({T('an a'): 0})
print('c:', c)
print(asdict(c))   # <- error here

prints:

c: C(f={T(a='an a'): 0})
Traceback (most recent call last):
...
  File "/home/eric/python/lib/dataclasses.py", line 1019, in asdict
return _asdict_inner(obj, dict_factory)
  File "/home/eric/python/lib/dataclasses.py", line 1026, in _asdict_inner
value = _asdict_inner(getattr(obj, f.name), dict_factory)
  File "/home/eric/python/lib/dataclasses.py", line 1059, in _asdict_inner
for k, v in obj.items())
TypeError: unhashable type: 'collections.OrderedDict'

So, although it's unfortunate, I think the only reasonable thing to do in this 
case is to have asdict() on a namedtuple return another namedtuple. Here's how 
that looks using the above code:

c: C(f={T(a='an a'): 0})
{'f': {T(a='an a'): 0}}

Admittedly, this can't be used with json.dumps() (you get "TypeError: keys must 
be str, int, float, bool or None, not T"), but I think it's the best we can do. 
It's consistent with any other class derived from tuple or list: asdict() will 
convert it to a copy of the class, recursing into each item that's in the tuple 
or list.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

I like GH-9151 better because it changes only the namedtuple case, not other 
classes that are derived from list or tuple.

But, I might copy some of the tests from 9151 before I'm done with this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Change by Eric V. Smith :


--
pull_requests: +8598

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

For the record, I don't disagree with namedtuples not having a base class.

Maybe it's best to let copy.deepcopy() deal with namedtuples, instead of trying 
to detect them here. So just special case exact tuples and lists, and pass 
everything else to copy.deepcopy().

>>> C = namedtuple('C', 'a b c')
>>> c = C(1, 2, 3)
>>> b=copy.deepcopy(c)
>>> b
C(a=1, b=2, c=3)
>>> hasattr(b, '_fields')
True
>>> hasattr(c, '_fields')
True
>>> b is c
False
>>>

Although by doing that, you lose dict_factory or tuple_factory on nested data 
structures, and namedtuples that contain dataclasses would be handled 
differently, I think. I'll do some investigating.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Neil Girdhar]

Neil Girdhar  added the comment:

> The code generated by oollections.namedtuple was based on patterns already in 
> widespread use at the time.

That's fair enough.  However, it seems like there is one important difference: 
unlike any other Sequence, namedtuples cannot be initialized with an iterable.

For that reason, I like Eric's option of checking for the _fields member rather 
than special-casing list and tuple since it seems like namedtuple is the 
special case.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Raymond Hettinger]

Raymond Hettinger  added the comment:

> Raymond has resisted adding a base class to namedtuple. I believe the
> preferred way to identify a namedtuple is to look for a _fields member.

FWIW, that was also Guido's opinion as well.  A named tuple is generic concept 
defined in the glossary as "Any tuple-like class whose indexable elements are 
also accessible using a named attribute".  This includes user-defined classes, 
classes created by collections.namedtuple, and instances of structseq.  The 
code generated by oollections.namedtuple was based on patterns already in 
widespread use at the time.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

Hmm, for some reason I'm not getting mail from this issue, so I made my PR 
before I saw the comments since my last comment.

Raymond has resisted adding a base class to namedtuple. I believe the preferred 
way to identify a namedtuple is to look for a _fields member. We could do that 
instead of use the (*[]) trick for all classes derived from tuple or list.

Maybe it's worthwhile bringing up the differences in how tuple and namedtuple 
handle creation with iterables on python-dev.

I'm still not certain of the right approach, but PR 8728 adds some tests and 
fixes the problem identified in this issue. I probably won't commit it until I 
can discuss with some other people.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Change by Eric V. Smith :


--
keywords: +patch
pull_requests: +8212
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Neil Girdhar]

Neil Girdhar  added the comment:

Why can't we add an ABC into a NamedTuple instance's MRO?  Because while I like 
Eric's solution, it seems to be backwards:  tuple and list are not the special 
cases—NamedTuple is.

All sequences accept an iterable in their constructor, and NamedTuple doesn't.  
So it should be NamedTuple that is marked as being "weird".  Right now, 
NamedTuple instances claim to be tuples, but don't accept iterables to 
initialize themselves.  That seems wrong.

This problem that we have with dataclass can easily pop up somewhere else, and 
it will require the same restriction to list and tuple types to fix it 
(breaking user-defined types).

Is it imposible to add to the MRO of NamedTuple instances?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Ivan Levkivskyi]

Ivan Levkivskyi  added the comment:

Eric, I like your solution. It is probably not perfect, but at least it solves 
the existing problem without introducing obvious problems.

Neil, your way will not work since named tuples don't have NamedTuple in their 
MROs:

CustomNT.mro == (CustomNT, tuple, object)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Neil Girdhar]

Neil Girdhar  added the comment:

Sorry if I'm intruding here, but I really dislike that we are doing isinstance 
versus list, tuple, and dict.  And I dislike even more type(x) in (list, 
tuple).  I think the ideal thing to do would have been to check versus abstract 
base classes like isinstance(x, Sequence) and isinstance(x, Mapping).  The 
advantage to this is flexibility with user-defined types.

The problem seems to be that the abstract base classes don't promise anything 
about the constructor.  It seems like some Sequence types accept an Iterable 
(e.g. tuple), but others like NamedTuple want positional arguments.  I think 
this promise should be encoded in some way.

Maybe a third solution is to have NamedTuple special-cased out:

isinstance(x, Sequence) and not isinstance(x, NamedTuple)

If there are other Sequence types that do this (are there?) then an abstract 
base class could be created.

This solution has the benefit of playing the most nicely with user-defined 
classes.

--
nosy: +NeilGirdhar

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

Maybe do both, then. Also, it's probably better for performance reasons (I 
know: premature optimization and all that ...):

@@ -1018,8 +1018,10 @@ def _asdict_inner(obj, dict_factory):
 value = _asdict_inner(getattr(obj, f.name), dict_factory)
 result.append((f.name, value))
 return dict_factory(result)
-elif isinstance(obj, (list, tuple)):
+elif type(obj) in (list, tuple):
 return type(obj)(_asdict_inner(v, dict_factory) for v in obj)
+elif isinstance(obj, (list, tuple)):
+return type(obj)(*[_asdict_inner(v, dict_factory) for v in obj])
 elif isinstance(obj, dict):
 return type(obj)((_asdict_inner(k, dict_factory), _asdict_inner(v, 
dict_factory))
   for k, v in obj.items())

I guess we could argue it's a bug in nametuples, but I don't see that getting 
us very far.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Ivan Levkivskyi]

Ivan Levkivskyi  added the comment:

The problem with this solution is that it may brea (relatively common) use case 
of tuple valued fields, e.g.:

>>> tuple(*[x for x in a])
Traceback (most recent call last):
  File "", line 1, in 
TypeError: tuple expected at most 1 arguments, got 2

Essentially, named tuples are all subclasses of `tuple` but they override 
__new__ in an incompatible way.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

Thanks for the pointer, Ivan.

I haven't really thought it through yet, but this fixes the problem at hand:

diff --git a/dataclasses.py b/dataclasses.py
index ba34f6b..54916ee 100644
--- a/dataclasses.py
+++ b/dataclasses.py
@@ -1019,7 +1019,7 @@ def _asdict_inner(obj, dict_factory):
 result.append((f.name, value))
 return dict_factory(result)
 elif isinstance(obj, (list, tuple)):
-return type(obj)(_asdict_inner(v, dict_factory) for v in obj)
+return type(obj)(*[_asdict_inner(v, dict_factory) for v in obj])
 elif isinstance(obj, dict):
 return type(obj)((_asdict_inner(k, dict_factory), _asdict_inner(v, 
dict_factory))
   for k, v in obj.items())

There are plenty more tests needed for this, plus I need to think it through 
some more. astuple() has the same issue. I'll also have to think about the dict 
subclass case, too.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Eric V. Smith  added the comment:

Oops, didn't see that you commented on this, Ivan.

I'll do some analysis tomorrow.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Change by Eric V. Smith :


--
assignee:  -> eric.smith
components: +Library (Lib) -Interpreter Core
versions: +Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Ivan Levkivskyi]

Ivan Levkivskyi  added the comment:

OK, so the crux of the bug is this difference:

>>> a = (1, 2)
>>> tuple(x for x in a)
(1, 2)
>>> NamedTupleAttribute(x for x in a)
NamedTupleAttribute(example= at 0x10e2e52a0>)

A potential solution would be to either use `type(obj) in (list, tuple)` 
instead of `isinstance(obj, (list, tuple))` (and thus cause using copy.deepcopy 
for everything else), but this might break some use cases (IMO quite unlikely).

Any other thoughts?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Ivan Levkivskyi]

Change by Ivan Levkivskyi :


--
nosy: +levkivskyi

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Eric V. Smith]

Change by Eric V. Smith :


--
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue34363] dataclasses.asdict() mishandles dataclass instance attributes that are instances of subclassed typing.NamedTuple

[Alex DeLorenzo]

New submission from Alex DeLorenzo :

Example:

from typing import NamedTuple
from dataclasses import dataclass, asdict

class NamedTupleAttribute(NamedTuple):
example: bool = True

@dataclass
class Data:
attr1: bool
attr2: NamedTupleAttribute

data = Data(True, NamedTupleAttribute(example=True))
namedtuple_attr = asdict(data)['attr2']
print(type(namedtuple_attr.example))
>>> generator

One would expect that the printed type would be of type bool.

--
components: Interpreter Core
messages: 323298
nosy: alexdelorenzo
priority: normal
severity: normal
status: open
title: dataclasses.asdict() mishandles dataclass instance attributes that are 
instances of subclassed typing.NamedTuple
type: behavior
versions: Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com