gh-143535: Document dispatch behavior of singledispatchmethod#143592
gh-143535: Document dispatch behavior of singledispatchmethod#143592priyanshu2282-cyber wants to merge 8 commits intopython:mainfrom
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
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 have made the requested changes; please review again. |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
Co-authored-by: Bartosz Sławecki <bartosz@ilikepython.com>
Doc/library/functools.rst
Outdated
| For every ``singledispatchmethod`` method defined in a class, the value | ||
| dispatched on by that method is *always* the first argument passed for the call. | ||
| Therefore, dispatching to regular methods can only be sensibly performed by | ||
| calling such a method from instances of the class, and not from the class object. |
There was a problem hiding this comment.
Align your paragraph with the other paragraphs, that is, use the correct indentation. You're 1 space off.
There was a problem hiding this comment.
Got it, Aligned!
|
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 |
Doc/library/functools.rst
Outdated
| The same pattern can be used for other similar decorators: | ||
| :deco:`staticmethod`, :deco:`~abc.abstractmethod`, and others. | ||
|
|
||
| For every ``singledispatchmethod`` method defined in a class, the value |
There was a problem hiding this comment.
You don't define a singledispatchmethod, you decorate one with this decorator. See the other paragraphs.
There was a problem hiding this comment.
You don't define a singledispatchmethod
It doesn't say singledispatchmethod defined in a class, but singledispatchmethod method defined in a class.
However, I agree that the current wording can be better. If it was good, you wouldn't have raised the thread.
There was a problem hiding this comment.
Thanks for your clarification, I have used the decorators as suggested, and updated the PR.
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
|
You did not address all my comments. I am going offline but I really need you to address everything before requesting a review as this makes me lose my time. |
|
Sorry for the back-and-forth. I’ve addressed all the review comments and also I have made the requested changes; please review again @picnixz |
|
Hi @johnslavik, just checking in on this PR in case it slipped through. No rush at all, would appreciate your feedback when you have time. Thanks! |
3 participants
The PR clarifies the behavior of
singledispatchmethodin the documentation. It always dispatches the first argument during call, regardless of whether the method is bound or unbound, as a resultxf(a)!=c.f(x,a). The behavior is intentional and required to support correct dispatch forstaticmethodandclassmethod.singledispatchmethoddispatches on first argument to instance-bindable method called from class level #143535📚 Documentation preview 📚: https://cpython-previews--143592.org.readthedocs.build/