-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Docs update and warning frozen defined both #10082
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
Conversation
|
Hi @sydney-runkle, |
CodSpeed Performance ReportMerging #10082 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like a good start. We've left a few requests for changes. Thanks for your contribution!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe a small test (with pytest.warns) would be great. We also need to specify the correct stacklevel argument, as per my comment.
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested 2 other changes. Looks good otherwise, thanks!
|
As @Viicos mentioned, I'd love to see a test (and the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - can merge once we add a test. Thanks!
|
I'll go ahead and merge this and add a test in a separate PR |
Change Summary
This PR addresses an issue where defining
frozenboth in the@dataclassdecorator andconfig=ConfigDict(frozen=True)led to confusing and inconsistent behavior.The changes made in this PR aim to clarify the expected behavior and provide better guidance to users.
In the dataclasses.md file, there was no specific explanation about frozen, so I added an explanation about the frozen setting in ConfigDict(config.py).
Related issue number
fix #9850
Checklist
please review: @sydney-runkle