GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-143535: Document dispatch behavior of singledispatchmethod by priyanshu2282-cyber · Pull Request #143592 · python/cpython Conversation
|
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I have made the requested changes; please review again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Bartosz Sławecki <[email protected]>
| 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.
The reason will be displayed to describe this comment to others. Learn more.
Align your paragraph with the other paragraphs, that is, use the correct indentation. You're 1 space off.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Got it, Aligned!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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.
The reason will be displayed to describe this comment to others. Learn more.
You don't define a singledispatchmethod, you decorate one with this decorator. See the other paragraphs.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your clarification, I have used the decorators as suggested, and updated the PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I have made the requested changes; please review again |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
#144615 demonstrates that this behavior can be fairly easily accounted for. I'd prefer to elevate the capability rather than document the deficiency. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Agreed. Let us close this one then. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.