Skip to content
This repository was archived by the owner on Nov 23, 2017. It is now read-only.

Python Issue #26654: Inspect keyword arguments of functools.partial.#328

Closed
iceboy233 wants to merge 6 commits into
python:masterfrom
iceboy233:master
Closed

Python Issue #26654: Inspect keyword arguments of functools.partial.#328
iceboy233 wants to merge 6 commits into
python:masterfrom
iceboy233:master

Conversation

@iceboy233

Copy link
Copy Markdown

No description provided.

Comment thread asyncio/events.py Outdated

def _format_callback_source(func, args):
func_repr = _format_callback(func, args)
func_repr = _format_callback(func, args, {})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pass None instead of {}, it's cheaper.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@vstinner

Copy link
Copy Markdown
Member

CPython issue: https://bugs.python.org/issue26654

@iceboy233

Copy link
Copy Markdown
Author

Gentle ping. I think the PR is ready.

@gvanrossum

Copy link
Copy Markdown
Member

@hayo can you review and merge this?

@hayo

hayo commented Apr 5, 2016

Copy link
Copy Markdown

sorry, wrong hayo, you probably mean haypo :)

@iceboy233

Copy link
Copy Markdown
Author

@Haypo

@1st1

1st1 commented May 13, 2016

Copy link
Copy Markdown
Member

@iceboy-sjtu Could you please sign CPython's contributor agreement?

@iceboy233

Copy link
Copy Markdown
Author

@1st1 Signed online. Thank you.

@1st1

1st1 commented Jun 28, 2016

Copy link
Copy Markdown
Member

@iceboy-sjtu Looking at this PR again -- a quick question: a lot of callable formatting functionality is already implemented as part of inspect.signature API. Would it be possible to just reuse that and drop all formatting logic from asyncio?

@iceboy233

Copy link
Copy Markdown
Author

@1st1 This is just a quick fix for the issue that the error message includes positional arguments but it does not include keyword arguments which may be misleading. I agree that the original code looks ugly, but let's leave it as is as it does not cause any problem.

btw There are two problems using inspect.signature. Firstly, it does not work on builtin functions such as print, len, repr. It also does not work on any composition that includes these "non-workable functions" by functools.partial.

>>> inspect.signature(print)
ValueError: no signature found for builtin <built-in function print>
>>> inspect.signature(functools.partial(print, 0))
ValueError: no signature found for builtin <built-in function print>

Secondly, it does not recognize functools.partial.

>>> str(inspect.signature(functools.partial(lambda x: x, 0)))
'()'

@1st1

1st1 commented Jun 29, 2016

Copy link
Copy Markdown
Member

Firstly, it does not work on builtin functions such as print, len, repr

asyncio can't work with them either. Some builtin functions are just not introspectable yet.

Secondly, it does not recognize functools.partial.

It does. In your example it prints (), which means your callable that does not accept any arguments. But I see the point now -- inspect.signature formats callable objects to show what arguments they can accept, whereas we want to format the function with all its bound arguments. That means we can't use the signature API here...

@iceboy233

Copy link
Copy Markdown
Author

Acknowledged for inspect.signature. Seems good to merge?

@iceboy233

Copy link
Copy Markdown
Author

Ping.

@1st1

1st1 commented Sep 9, 2016

Copy link
Copy Markdown
Member

Will merge this after beta1.

@1st1

1st1 commented Sep 15, 2016

Copy link
Copy Markdown
Member

Merged in b100957. Thanks a lot for fixing this!

@1st1 1st1 closed this Sep 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants