This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: dataclasses should allow frozendict default value
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: CCLDArjun, eric.smith, gianni, rhettinger
Priority: normal Keywords: patch

Created on 2021-07-19 14:06 by gianni, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 29867 merged eric.smith, 2021-11-30 14:55
Messages (10)
msg397799 - (view) Author: Gianni Mariani (gianni) Date: 2021-07-19 14:06
Using a frozendict as a default value should not cause an error in dataclasses. The check for mutability is:

   isinstance(f.default, (list, dict, set))

It appears frozendict has been changed to have a dict base class and it now raises an exception.

There should be a way to indicate object mutability as the purpose of the isinstance(f.default, (list, dict, set)) check is for mutable default values.

Using default_factory to work around this issue is cumbersome.
msg397800 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-07-19 14:18
I agree that would be an improvement.
msg398098 - (view) Author: Arjun (CCLDArjun) * Date: 2021-07-23 21:28
Which frozendict does your message concern? I found this in some test: https://github.com/python/cpython/blob/bb3e0c240bc60fe08d332ff5955d54197f79751c/Lib/test/test_builtin.py#L741 or are you suggesting any user defined immutable object?

I'm not sure indicating that an object should be immutable to dataclasses will be less cumbersome than default_factory.

Currently, you have to do this:
> x: frozendict = field(default_factory=frozendict)

But, indicating mutability would be something like this:
> x: frozendict = field(mutable=false)
msg398105 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-07-24 00:00
When I originally read this, I read it as frozenset. I agree that there's no good way of telling if an arbitrary class is immutable, so I'm not sure we can do anything here.

So I think we should close this as a rejected idea.
msg398118 - (view) Author: Gianni Mariani (gianni) Date: 2021-07-24 06:45
@Arjun - this is about default values (See the bug description - Using a frozendict as a default value)

Try this:

from frozendict import frozendict
from dataclasses import dataclass

@dataclass
class A:
    a: frozendict = frozendict(a=1)

This used to work until frozendict became a subclass of dict.

Perhaps another fix is to convert any dict to a frozendict? Maybe not.

How would you handle this case? The only thing I figured was:
from frozendict import frozendict, field
from dataclasses import dataclass

AD=frozendict(a=1)
@dataclass
class A:
    a: frozendict = field(default_factory=lambda:AD)

Which imho is cumbersome.
msg403103 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2021-10-03 17:50
[Eric]
> I agree that there's no good way of telling if an
> arbitrary class is immutable, so I'm not sure we can 
> do anything here.

Consider using non-hashability as a proxy indicator for immutability.

    -  isinstance(f.default, (list, dict, set))
    +  f.default.__hash__ is None

While this is imperfect, it would be more reliable than what we have now.
msg403108 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-10-03 19:16
That's a good idea, Raymond.

>>> [x.__hash__ is None for x in (list, dict, set, frozenset)]
[True, True, True, False]

I don't think this change would cause any backward compatibility issues, except it would now allow a default of something bad like:

>>> class BadList(list):
...   def __hash__(self): return 0
...
>>> isinstance(BadList(), list), BadList.__hash__ is None
(True, False)

I can't say I care too much about now allowing things that didn't used to be allowed, especially if they're poorly designed like BadList.

I'll put together a PR.
msg408335 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-12-11 21:12
New changeset e029c53e1a408b89a4e3edf30a9b38b094f9c880 by Eric V. Smith in branch 'main':
bpo-44674: Use unhashability as a proxy for mutability for default dataclass __init__ arguments. (GH-29867)
https://github.com/python/cpython/commit/e029c53e1a408b89a4e3edf30a9b38b094f9c880
msg408365 - (view) Author: Gianni Mariani (gianni) Date: 2021-12-12 06:31
Excellent. Thanks!
msg408370 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2021-12-12 09:27
@gianni: can you verify that your use case works in 3.11?
History
Date User Action Args
2022-04-11 14:59:47adminsetgithub: 88840
2021-12-12 09:27:52eric.smithsetmessages: + msg408370
2021-12-12 06:31:09giannisetmessages: + msg408365
2021-12-11 21:13:04eric.smithsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-12-11 21:12:25eric.smithsetmessages: + msg408335
2021-11-30 14:55:29eric.smithsetkeywords: + patch
stage: patch review
pull_requests: + pull_request28093
2021-10-03 19:17:08eric.smithsettype: compile error -> behavior
2021-10-03 19:16:57eric.smithsetmessages: + msg403108
2021-10-03 17:50:13rhettingersetnosy: + rhettinger
messages: + msg403103
2021-07-24 06:45:59giannisetmessages: + msg398118
2021-07-24 00:03:17eric.smithsetassignee: eric.smith
2021-07-24 00:00:42eric.smithsetmessages: + msg398105
2021-07-23 21:28:26CCLDArjunsetnosy: + CCLDArjun
messages: + msg398098
2021-07-19 14:18:38eric.smithsetnosy: + eric.smith
messages: + msg397800
2021-07-19 14:06:32giannicreate