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.

Sign up for GitHub

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

Jump to bottom

bpo-35540 dataclasses.asdict support defaultdict fields #32056

Merged
merged 6 commits into from
Oct 7, 2022
Merged

bpo-35540 dataclasses.asdict support defaultdict fields #32056

merged 6 commits into from
Oct 7, 2022

Conversation

kwsp
Copy link
Contributor

@kwsp kwsp commented Mar 22, 2022 •
edited by bedevere-bot
Loading

https://bugs.python.org/issue35540

dataclasses.asdict breaks when there are defaultdicts in the dataclass attributes because it assumes defaultdict and dict are the same thing, but in reality defaultdict takes a default_factory as the first argument in its constructor.

Since dataclasses.asdict already supports NamedTuples, I think its fair that defaultdict, which is also in the standard library, does not break dataclasses.

Test:

from dataclasses import dataclass, asdict
from typing import DefaultDict, List
from collections import defaultdict


@dataclass
class C:
    mp: DefaultDict[str, List]


dd = defaultdict(list)
dd["x"].append(12)
c = C(mp=dd)
d = asdict(c)  # throws "TypeError: first argument must be callable or None"
assert d == {"mp": {"x": [12]}}

https://bugs.python.org/issue35540

@kwsp kwsp requested a review from ericvsmith as a code owner March 22, 2022 18:23
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@kwsp

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@kwsp
bpo-35540: Fix dataclasses.asdict support for defaultdict fields
7567680 
dataclasses.asdict breaks when there are `defaultdict`s in the dataclass
attributes because it assumes `defaultdict` and `dict` are the same thing,
but in reality `defaultdict` takes a default_factory as the first argument
in its constructor. This change adds support for defaultdict fields.
@kwsp kwsp force-pushed the fix-issue-35540 branch from 82a5a4e to 7567680 Compare March 22, 2022 19:15
@MaxwellDupre
Copy link
Contributor

Could you add a test please?

@kwsp
Copy link
Contributor Author

kwsp commented May 4, 2022

Could you add a test please?

I just added a test for this

@wrmsr wrmsr mannequin mentioned this pull request Apr 10, 2022
Closed
MaxwellDupre
MaxwellDupre approved these changes Jul 4, 2022
View reviewed changes
Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests ok on Fedora.
Ref to bugs.python.org/issue35540 is not future proof, assume will be turned off at some time.

ericvsmith
ericvsmith reviewed Oct 4, 2022
View reviewed changes
Lib/dataclasses.py Outdated Show resolved Hide resolved
@ericvsmith ericvsmith self-assigned this Oct 4, 2022
@ericvsmith ericvsmith merged commit c46a423 into python:main Oct 7, 2022
carljm added a commit to carljm/cpython that referenced this pull request Oct 8, 2022
@carljm
Merge branch 'main' into typewatch
29e38df 
* main:
  bpo-35540 dataclasses.asdict now supports defaultdict fields (pythongh-32056)
  pythonGH-91052: Add C API for watching dictionaries (pythonGH-31787)
  bpo-38693: Use f-strings instead of str.format() within importlib (python#17058)
This was referenced Oct 10, 2022
Closed
Merged
This was referenced Oct 11, 2022
Closed
Merged
Oct 11, 2022
@kwsp
bpo-35540 dataclasses.asdict now supports defaultdict fields (pythong…
395bc9a 
…h-32056)
@skshetry skshetry mentioned this pull request Jun 16, 2023
Closed
@kwsp kwsp mentioned this pull request Jun 26, 2023
Closed
Closed
@akitathai94
Copy link

akitathai94 commented Jun 29, 2024 •
edited
Loading

Does this merge fix the issue yet cause it still throws error in Python 3.11

@kwsp
Copy link
Contributor Author

kwsp commented Jun 29, 2024

Does this merge fix the issue yet cause it still throws error in Python 3.11

This was merged in 3.12 and not backported to 3.11

https://docs.python.org/3.12/whatsnew/changelog.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericvsmith ericvsmith ericvsmith left review comments

@MaxwellDupre MaxwellDupre MaxwellDupre approved these changes

Assignees

@ericvsmith ericvsmith

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@kwsp @the-knights-who-say-ni @MaxwellDupre @akitathai94 @ericvsmith @ezio-melotti @bedevere-bot