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

Fix dataclass_transform decorator for pydantic dataclasses #5120

Merged
merged 6 commits into from
Mar 8, 2023
Merged

Fix dataclass_transform decorator for pydantic dataclasses #5120

merged 6 commits into from
Mar 8, 2023

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Feb 27, 2023

Remove kw_only_default and add additional field_specifier for stdlib dataclass fields.

This change should be backported to 1.10.X since the next mypy release will break typing otherwise.

Ref https://peps.python.org/pep-0681/#dataclass-transform-parameters
Fixes #5117

@samuelcolvin
Copy link
Member

Not sure if this will conflict with #5009 which is a WIP implementation of dataclasses for V2 which I'm intending to complete this week?

@cdce8p
Copy link
Contributor Author

cdce8p commented Feb 27, 2023

Not sure if this will conflict with #5009 which is a WIP implementation of dataclasses for V2 which I'm intending to complete this week?

Just looked at it, as long as the public interface isn't changed (i.e. dataclasses.field and positional arguments still works) it shouldn't conflict. It might even be slightly helpful if you add a pydantic specific field method. That could / should be added to dataclass_transform then as well.

If you really want though, I could also change the target to 1.10.X.

@samuelcolvin samuelcolvin added the cherry-pick-v1 cherry-pick back on 1.10.X-fixes label Feb 27, 2023
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM.

changes/5120-cdce8p.md Outdated Show resolved Hide resolved
pydantic/dataclasses.py Show resolved Hide resolved
Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

pydantic/dataclasses.py Outdated Show resolved Hide resolved
pydantic/dataclasses.py Outdated Show resolved Hide resolved
@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 6, 2023

due to import time problems in the past, we avoid doing module level import of stuff unless it's absolutely necessary.

I guess if we remove dataclass from __init__.py, maybe this would be fine, but we'll need to change this or on v1.10.

dataclasses is already imported in other modules, e.g. networks.py. So it won't add any additional import time.

import dataclasses as _dataclasses

Should the implementation keep the dataclass_transform for runtime introspection?

As the import time is no longer a concern, I reverted the change. With that dataclass_transform will be accessible at runtime again.

@cdce8p cdce8p mentioned this pull request Mar 7, 2023
@dmontagu
Copy link
Contributor

dmontagu commented Mar 7, 2023

Closing in favor of #5111

@dmontagu dmontagu closed this Mar 7, 2023
@dmontagu
Copy link
Contributor

dmontagu commented Mar 7, 2023

Oh I realize this is for main, not 1.10.X, so I'll reopen it.

@dmontagu dmontagu reopened this Mar 7, 2023
dmontagu added a commit that referenced this pull request Mar 8, 2023
* Fix mypy plugin for 1.1.0 (#5077)

* Fix mypy plugin for 1.1.0
* Code review
* Add version key to plugin data

(cherry picked from commit 6267ae3)

* Change file name

* Add the changes from #5120

* Update changes file

* Remove additional unneeded dataclass import (from #5120)

---------

Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>
@cdce8p
Copy link
Contributor Author

cdce8p commented Mar 8, 2023

I think it would make sense to move forward with this PR as well now that the changes have already been merged into 1.10.X. (A backport is obviously no longer necessary). /CC: @dmontagu

@dmontagu dmontagu removed the cherry-pick-v1 cherry-pick back on 1.10.X-fixes label Mar 8, 2023
@dmontagu dmontagu merged commit af1c7a1 into pydantic:main Mar 8, 2023
@dmontagu
Copy link
Contributor

dmontagu commented Mar 8, 2023

Thanks @cdce8p!

@cdce8p cdce8p deleted the dataclass-transform-decorator branch March 9, 2023 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(🐞) Mypy plugin doesn't work with positional arguments on dataclasses
4 participants