Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-6761: Fix __call__ documentation#7987
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
andresdelfino commented Jun 28, 2018 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Jan 10, 2019
I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches |
andresdelfino commented Feb 23, 2019
@bitdancer could you take a look at this? |
vstinner left a comment
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 prefer (arg1, arg2, ...) syntax rather than (arguments) syntax: IMHO (arg1, arg2, ...) is more explicit.
bedevere-bot commented Feb 25, 2019
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 |
andresdelfino commented Feb 25, 2019
I have made the requested changes; please review again |
bedevere-bot commented Feb 25, 2019
Thanks for making the requested changes! @vstinner: please review the changes made to this pull request. |
vstinner left a comment
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.
LGTM.
vstinner commented Feb 25, 2019
@csabella@JulienPalard: Would you mind to double check and/or merge this PR? |
vstinner commented Feb 25, 2019
I'm not super excited by continuing to enhance the 2.7 doc. IMHO it's good as it is. I mean, we are close to Python 2 end of life, and I prefer to read Python 3 doc to write Python 2 code. Well, it's just a comment, I'm also fine with backport to 2.7 if you want (but I expect a conflict). |
andresdelfino commented Mar 5, 2019
I think you are right. Could you take a look at the new wording? |
csabella commented Mar 6, 2019
I'm definitely not the one who would be able to have the final say on the wording for this, but I wonder if something like this would work: |
andresdelfino commented Mar 7, 2019
@vstinner could you take a look on the new wording I wrote based on Cheryl's comments on #7987 (comment) and also Cheryl's proposed wording on #7987 (comment) and tell us what you think? |
vstinner commented Mar 7, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
For me, Usually, it's also equivalent to |
Doc/reference/datamodel.rst Outdated
| Called when the instance is "called" as a function; if this method is defined, | ||
| ``x(arg1, arg2, ...)`` is a shorthand for ``x.__call__(arg1, arg2, ...)``. | ||
| the class receives the instance and all arguments of the call. |
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 know what is supposed to be conveyed here, and I find this more confusing than the original. Diving into the internals of how calls to instance methods work is detracting from the discussion of what call does.
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.
Thanks for your insight! Before doing any changes, I'll wait for @bitdancer to give his input.
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.
This current version don't work for me, "if this method is defined, the class receives the instance and all arguments of the call." looks wrong, It's not the class that receives the instance and all arguments.
I'd propose something like:
Called when the instance is "called" as a function. Receives the instance and the function parameters.
If you want to almost keep the example you could add something like:
So ``obj(arg1, arg2, ...)`` calls ``__call__(arg1, arg2, ...)`` on ``obj``.
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 don’t think that last suggestion (from Julien) works (call on obj?).
type(x).__call__(x, arg1, ...) is closer to what happens, and the call matches the way the method is defined (with a self parameter).
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.
@merwok I agree. Made a change based on your suggestion, what do you think?
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.
@vstinner You said «I would prefer to [not] use type() in the doc, since technically the builtin function can be overriden» → that’s what we say «roughly»! Is the latest version good for you or you would prefer obj.__class__.__call__(obj, etc)?
@csabella I’m not sure I understand your review; could you comment on the latest version? Thanks!
andresdelfino commented Jun 15, 2019 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
@bitdancer Could you check the proposed wording out and comment? :) |
vstinner left a comment
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.
LGTM.
merwok commented Oct 27, 2020
I updated the backports labels to get 3.9 and 3.8, and I think this is ready to merge unless another reviewer weighs in. |
miss-islington commented Oct 27, 2020
Thanks @andresdelfino for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
(cherry picked from commit 95f710c) Co-authored-by: Andre Delfino <adelfino@gmail.com>
bedevere-bot commented Oct 27, 2020
GH-23004 is a backport of this pull request to the 3.9 branch. |
(cherry picked from commit 95f710c) Co-authored-by: Andre Delfino <adelfino@gmail.com>
bedevere-bot commented Oct 27, 2020
GH-23005 is a backport of this pull request to the 3.8 branch. |
vstinner commented Oct 27, 2020
Thanks @andresdelfino! It's a nice documentation enhancement. I'm fine with "roughly" ;-) |
…lots1 * origin/master: (365 commits) bpo-42029: Remove IRIX code (pythonGH-23023) bpo-42143: Ensure PyFunction_NewWithQualName() can't fail after creating the func object (pythonGH-22953) bpo-34204: Use pickle.DEFAULT_PROTOCOL in shelve (pythonGH-19639) bpo-41805: Documentation for PEP 585 (pythonGH-22615) bpo-42161: Micro-optimize _collections._count_elements() (pythonGH-23008) bpo-42161: Remove private _PyLong_Zero and _PyLong_One (pythonGH-23003) bpo-42099: Fix reference to ob_type in unionobject.c and ceval (pythonGH-22829) bpo-41659: Disallow curly brace directly after primary (pythonGH-22996) bpo-6761: Enhance __call__ documentation (pythonGH-7987) bpo-42161: Modules/ uses _PyLong_GetZero() and _PyLong_GetOne() (pythonGH-22998) bpo-41474, Makefile: Add dependency on cpython/frameobject.h (pythonGH-22999) bpo-42157: Rename unicodedata.ucnhash_CAPI (pythonGH-22994) bpo-42161: Use _PyLong_GetZero() and _PyLong_GetOne() (pythonGH-22995) bpo-30681: Support invalid date format or value in email Date header (pythonGH-22090) bpo-42161: Add _PyLong_GetZero() and _PyLong_GetOne() (pythonGH-22993) bpo-42123: Run the parser two times and only enable invalid rules on the second run (pythonGH-22111) bpo-42157: Convert unicodedata.UCD to heap type (pythonGH-22991) bpo-42157: unicodedata avoids references to UCD_Type (pythonGH-22990) bpo-39101: Fixes BaseException hang in IsolatedAsyncioTestCase. (pythonGH-22654) bpo-1635741: _PyUnicode_Name_CAPI moves to internal C API (pythonGH-22713) ...
https://bugs.python.org/issue6761