bpo-41747: Ensure all dataclass methods uses their parents' qualname #22155
Conversation
Misc/NEWS.d/next/Library/2020-09-08-23-41-29.bpo-41747.M6wLKv.rst
Outdated
Show resolved
Hide resolved
pablogsal
left a comment
pablogsal
left a comment
There was a problem hiding this comment.
This LGTM but it would be great if @ericvsmith could take a final look
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
serhiy-storchaka
left a comment
serhiy-storchaka
left a comment
There was a problem hiding this comment.
Seems this change also makes these methods pickleable.
Lib/dataclasses.py
Outdated
| # Never overwrites an existing attribute. Returns True if the | ||
| # attribute already exists. | ||
| if isinstance(value, FunctionType): | ||
| value.__qualname__ = f"{cls.__qualname__}.{value.__name__}" |
There was a problem hiding this comment.
Why value.__name__ and not name? Are they always the same?
There was a problem hiding this comment.
For all use cases, but since it got extracted to another function which only takes cls and value, I guess there is no need to complicate that function.
Co-authored-by: Serhiy Storchaka <[email protected]>
ericvsmith
left a comment
ericvsmith
left a comment
There was a problem hiding this comment.
With a few tiny issues, this looks good to me.
@serhiy-storchaka: Should we also add tests for pickling these methods? Or should that be another PR?
Misc/NEWS.d/next/Library/2020-09-08-23-41-29.bpo-41747.M6wLKv.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Library/2020-09-08-23-41-29.bpo-41747.M6wLKv.rst
Outdated
Show resolved
Hide resolved
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I do not think that it is necessary to add tests for pickling these methods. |
|
@ericvsmith a gentle ping. |
|
Sorry for the delays. This is on my list of things to look at next week during the development sprints. |
…ythonGH-22155) * bpo-41747: Ensure all dataclass methods uses their parents' qualname Co-authored-by: Serhiy Storchaka <[email protected]>
6 participants
https://bugs.python.org/issue41747