Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX/API: use inspect.signature for python3 #8795

Merged
merged 3 commits into from
Sep 12, 2015

Conversation

tacaswell
Copy link
Contributor

This allows functions with keyword-only arguments to have their signatures introspected.

Also makes this bit of code 3.6 compatible (@Carreau I see your 3.5 and raise you a 3.6 馃槣 )

Found this via working on matplotlib/matplotlib#4829 where for python 3.3+ we mark the data kwarg as keyword-only.

This is a slight change in behavior as it makes all arguments, not just the optional ones. This can easily be changed back, but leaving it for now as I would propose that including all arguments is a better behavior.

This allows functions with keyword-only arguments to have their
signatures introspected.

Also makes this bit of code 3.6 compatible.

This slightly changes the API in python 3 as all of arguments are
available for completion, not just the optional ones.
pass
else:
try:
args, _, _1, defaults = inspect.getargspec(call_obj)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder what would happen if on py2, this could a) unwrap (signature does unwrap until a "signature" is found, getargspec does not) and b) check for a simple get_additional_completions(...) function which we could then set in out decorator. getdoc() already does something similar...

so:

addons = []
while true:
    if hasattr(call_obj, "get_additional_completions"):
        addons.extend(call_obj.get_additional_completitions())
        break
    if hasattr(call_obj, "__wrapped__"):
        call_obj = call_obj.__wrapped__
    else:
        break
[...]
ret.extend(addons)

@minrk
Copy link
Member

minrk commented Sep 8, 2015

We ship a backport of inspect.signature that should work on Python 2 in IPython.utils.signatures.signature, if you want to try to just use that instead.

@jankatins
Copy link
Contributor

Then mpl must ship that as well or needs a dependency on ipython...

@minrk
Copy link
Member

minrk commented Sep 8, 2015

@JanSchulz I'm not sure I understand. This is a PR to IPython, so why would IPython using its own utility affect MPL?

@jankatins
Copy link
Contributor

This PR is part of the "unpack labeled data for plotting": make it possible to use a call like this plot("var1", "var2", data=df) instead of plot(df["var1"], df["var2"]). This is currently implemented in matplotlib/matplotlib#4829 with a decorator, which makes all signatures (= code completions and inspections in python <3.4) look like plot(*args, **kwargs). @tacaswell added a __wrapped__ attribute and a __signature__ attribute (see these two parts of the diff: https://github.com/matplotlib/matplotlib/pull/4829/files#diff-9ffb0d1db51a67ee4f37528e00715b3cR1590 https://github.com/matplotlib/matplotlib/pull/4829/files#diff-9ffb0d1db51a67ee4f37528e00715b3cR1795) in case python understands it so that newer python versions get the right signature and therefore the right code completions.

The problem is that to construct the signature object, mpl needs inspect.signature so adding that in py2.x even if IPython understands that, we would need inspect.signature included in mpl as well. Using a workaround like get_addon_completions() would not.

@tacaswell
Copy link
Contributor Author

@JanSchulz Just moving that file over is probably a non-starter for now as mpl (still) supports py2.6 so we don't have OrderedDict.

@tacaswell
Copy link
Contributor Author

Probably the best route on the mpl side is to try to import IPython and use it if available. I think this is ok because the only reason we want to tack the signatures on to legacy python is for nice introspection in IPython.

@jankatins
Copy link
Contributor

But then your PR here should also try to use the internal ipython copy oft inspect (if python doesn't have it...) in all versions...

edit: Ah, you already did it :-)

@tacaswell
Copy link
Contributor Author

no, mpl will only fall back to the IPython version in versions of python which don't already have signature.

@tacaswell
Copy link
Contributor Author

@tacaswell
Copy link
Contributor Author

@minrk This look good?

Can this also be back-ported to what ever branch you are going to release off of next? This is needed to make a feature in matplotlib 1.5 that @fperez pushed for to work well.

@minrk
Copy link
Member

minrk commented Sep 12, 2015

@tacaswell the next release will be 4.1, so this will get in. We haven't started making breaking changes on IPython to push master to 5.x and start backporting, yet.

minrk added a commit that referenced this pull request Sep 12, 2015
FIX/API: use inspect.signature for python3
@minrk minrk merged commit 3b2b82f into ipython:master Sep 12, 2015
@minrk minrk added this to the 4.1 milestone Sep 12, 2015
@tacaswell tacaswell deleted the fix_kwonly_arg_complete branch September 12, 2015 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants