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 containing Any #4356

Merged

Conversation

DetachHead
Copy link
Contributor

@DetachHead DetachHead commented Aug 9, 2022

Change Summary

remove Any types that contributed to usages of the dataclass decorator causing mypy errors when using the disallow-any-expr rule

Related issue number

fixes #4355

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

@DetachHead DetachHead force-pushed the remove_any_from_dataclass_type branch 8 times, most recently from 81462c2 to e90e882 Compare August 9, 2022 04:01
@DetachHead
Copy link
Contributor Author

please review

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.

looking good, but I wonder if we can reduce the size of the change somewhat?

please update.

pydantic/typing.py Outdated Show resolved Hide resolved
pydantic/config.py Show resolved Hide resolved
pydantic/dataclasses.py Show resolved Hide resolved
tests/mypy/modules/no_any.py Outdated Show resolved Hide resolved
@DetachHead DetachHead force-pushed the remove_any_from_dataclass_type branch from 9038a76 to 579a1ea Compare August 15, 2022 00:44
@samuelcolvin
Copy link
Member

This is a fix since V1.9, so really needs to be included before we release v1.10.


Notice

See twitter 🐦, I've you'd like this to be included in V1.10, please fix it and request a review TODAY.

Or if you need this in V1.10 but don't have time to complete it (or aren't the author), please comment here and on #4324.

@samuelcolvin samuelcolvin mentioned this pull request Aug 19, 2022
14 tasks
@DetachHead
Copy link
Contributor Author

@samuelcolvin all outstanding conversations are resolved, unless there's something i'm missing? i think it's ready to merge

@samuelcolvin
Copy link
Member

Okay, I'll look.

In future, best if you can say "please review" so the label changes and reviewers get assigned. I basically only look at PRs with the "ready for review" label.

@DetachHead
Copy link
Contributor Author

my bad, i didn't realize i had to comment it again after making changes, i assumed clicking "re-request review" would've done it

@samuelcolvin
Copy link
Member

samuelcolvin commented Aug 19, 2022

my bad, i didn't realize i had to comment it again after making changes, i assumed clicking "re-request review" would've done it

Totally understandable.

We might be able to somehow extend hooky to detect "re-request review" if there's a webhook for it. But it doesn't work at present.

@samuelcolvin samuelcolvin merged commit 9585ef3 into pydantic:main Aug 19, 2022
@samuelcolvin
Copy link
Member

This is great, thanks so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dataclass type contains Any
4 participants