This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2014年02月19日 12:40 by ncoghlan, last changed 2022年04月11日 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| issue20684_ignore_wrapped_in_argspec.diff | ncoghlan, 2014年02月19日 13:29 | Patch and tests | review | |
| issue20684_ignore_wrapped_in_argspec_02.diff | yselivanov, 2014年02月19日 20:14 | review | ||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 21100 | merged | Anthony Sottile, 2020年07月30日 18:16 | |
| Messages (12) | |||
|---|---|---|---|
| msg211609 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2014年02月19日 12:40 | |
In a comment on issue 17482, Mike Bayer pointed out a backwards incompatibility resulting from changing inspect.getfullargspec (etc) to rely on inspect.signature: they now follow __wrapped__ chains, where previously they ignored them. This means that instead of reporting the exact signature of the *wrapper*, they now report the signature of the wrapped function instead. Since switching these functions from ignoring __wrapped__ to following it is an unintended backwards incompatible change, I'll tweak the code to bypass the unravelling of wrapper chains in the getfullargspec case. |
|||
| msg211614 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2014年02月19日 13:29 | |
Attached patch moves the signature to an internal helper that takes an additional flag - whether or not to follow wrapper chains. getfullargspec() now calls this with the flag turned off, and that is passed down to any recursive calls. I'll be offline again until tomorrow evening, so don't feel obliged to wait for me if the patch looks good or just needs minor tweaks before merging. I also noticed a quirk with the getfullargspec -> signature fallback - we still drop the leading arg for partialmethod and other sources of signatures that aren't special cased. That's probably OK though - previously those wouldn't report signatures at all. |
|||
| msg211649 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2014年02月19日 20:14 | |
Nick, thanks for writing this patch! > That's probably OK though - previously those wouldn't report signatures at all. I honestly don't think it is OK. I think we should stick to the behaviour of old 'getfullargspec' consistently, to make its behaviour always predictable. I further tweaked your patch, please review the new one (*_02.diff) The changes are relatively simple -- I've added a new bool flag to '_signature_internal' -- skip_bound_arg. When it is False, the logic for skipping bound args is off. It also made 'getfullargspec' implementation simpler. Now 'getfullargspec()' should behave always like it did before + it will handle more callables. Larry, if you have some time for this, I'd be glad to receive your feedback on this. This issue is quite important. |
|||
| msg211650 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2014年02月19日 20:49 | |
Your version looks good to me, Yury (and good idea to add the second flag). |
|||
| msg211651 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2014年02月19日 20:53 | |
Thanks, Nick. Do you want me to commit the patch? (Larry will x-ray the patch before including it in 3.4rc2) |
|||
| msg211652 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2014年02月19日 20:58 | |
Yeah, I think push this (with NEWS etc) for 3.4.1, then create the cherry pick patch for Larry. |
|||
| msg211653 - (view) | Author: Larry Hastings (larry) * (Python committer) | Date: 2014年02月19日 21:02 | |
My only question: do they need to be independent flags? ISTM that they're always either both True or both False. |
|||
| msg211654 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2014年02月19日 21:05 | |
> My only question: do they need to be independent flags? ISTM that they're always either both True or both False. I'd keep them independent solely to make the code easier to read/audit. As, for instance, a combined argument won't make sense for someone who just inspects the code of '_signature_from_builtin()' helper. |
|||
| msg211657 - (view) | Author: Alyssa Coghlan (ncoghlan) * (Python committer) | Date: 2014年02月19日 21:24 | |
I think in 3.5 it may actually make sense to expose these as options (perhaps with different names), so yeah, I think separate flags is reasonable. The only plausible combined name would also be something like "legacy", and that would need to be assigned to more meaningful local variables for readability reasons anyway. |
|||
| msg211659 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2014年02月19日 21:31 | |
Committed in 65fb95578f94 .. forgot to add the issue # in the commit message :( |
|||
| msg211661 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2014年02月19日 21:35 | |
Issue #20688 to track cherry-picking this into 3.4.0 |
|||
| msg211667 - (view) | Author: Yury Selivanov (yselivanov) * (Python committer) | Date: 2014年02月19日 22:18 | |
> I think in 3.5 it may actually make sense to expose these as options > (perhaps with different names), so yeah, I think separate flags is > reasonable. +1 on thinking about exposing your 'follow_wrapper_chains' option in 3.5. I'll create an issue. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:57:58 | admin | set | github: 64883 |
| 2020年07月30日 18:16:47 | Anthony Sottile | set | nosy:
+ Anthony Sottile pull_requests: + pull_request20834 |
| 2014年02月19日 22:18:35 | yselivanov | set | messages: + msg211667 |
| 2014年02月19日 21:35:43 | yselivanov | set | messages: + msg211661 |
| 2014年02月19日 21:31:30 | yselivanov | set | status: open -> closed resolution: fixed |
| 2014年02月19日 21:31:09 | yselivanov | set | messages: + msg211659 |
| 2014年02月19日 21:24:51 | ncoghlan | set | messages: + msg211657 |
| 2014年02月19日 21:05:05 | yselivanov | set | messages: + msg211654 |
| 2014年02月19日 21:02:08 | larry | set | messages: + msg211653 |
| 2014年02月19日 20:58:04 | ncoghlan | set | messages: + msg211652 |
| 2014年02月19日 20:53:34 | yselivanov | set | messages: + msg211651 |
| 2014年02月19日 20:49:44 | ncoghlan | set | messages: + msg211650 |
| 2014年02月19日 20:14:31 | yselivanov | set | files:
+ issue20684_ignore_wrapped_in_argspec_02.diff messages: + msg211649 |
| 2014年02月19日 13:29:42 | ncoghlan | set | files:
+ issue20684_ignore_wrapped_in_argspec.diff title: inspect.getfullargspec (etc) incorrectly follow __wrapped__ chains -> inspect.getfullargspec (etc) incorrectly follows __wrapped__ chains messages: + msg211614 keywords: + patch stage: test needed -> patch review |
| 2014年02月19日 12:40:21 | ncoghlan | create | |