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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support partials and tz.curry #316

Merged
merged 4 commits into from
Nov 14, 2021
Merged

Support partials and tz.curry #316

merged 4 commits into from
Nov 14, 2021

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Nov 11, 2021

This implements support for functions wrapped in functools.partial. closes #315

There is a slight "catch" ... which is really due to the way that partial handles signatures when providing positional vs keyword arguments. Take this example

    def some_func(x: int, y: str) -> str:
        return y + str(x)

    wdg1 = magicgui(partial(some_func, 1))  # first position
    wdg2 = magicgui(partial(some_func, y='hi'))  # second kwarg

wdg1 will look like this:

Untitled

and wdg2 will look like this

Untitled2

they both behave like the corresponding partials. but wdg2 shows the widget for y, whereas wdg doesn't show x

To avoid that behavior automatically, we'd need to do a little special casing. It's possible, but prefer to leave to another PR

@tlambert03 tlambert03 requested a review from jni November 11, 2021 18:41
@codecov
Copy link

codecov bot commented Nov 11, 2021

Codecov Report

Merging #316 (f8b1f42) into main (aa226dc) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #316      +/-   ##
==========================================
+ Coverage   90.24%   90.30%   +0.06%     
==========================================
  Files          29       29              
  Lines        3301     3302       +1     
==========================================
+ Hits         2979     2982       +3     
+ Misses        322      320       -2     
Impacted Files Coverage Δ
magicgui/widgets/_function_gui.py 97.76% <ø> (+1.11%) ⬆️
magicgui/signature.py 96.62% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa226dc...f8b1f42. Read the comment docs.

@tlambert03 tlambert03 changed the title Support partials Support partials and tz.curry Nov 11, 2021
@tlambert03
Copy link
Member Author

added support for tz.curry too... the main difference there was that toolz uses the string __no__default__ as a sentinel, instead of Parameter.empty, to indicate the lack of a value.

In [1]: from functools import partial

In [2]: from toolz import curry

In [4]: def f(x: int, y: str): ...

In [5]: a = partial(f, y='hi')

In [6]: b = curry(f)(y='hi')

In [7]: import inspect

In [8]: inspect.signature(a)
Out[8]: <Signature (x: int, *, y: str = 'hi')>

In [9]: inspect.signature(b)
Out[9]: <Signature (x: int = '__no__default__', *, y: str = 'hi')>

so we need to special case that here. @jni if you wanted to upstream anything, it would be to see whether they can use Parameter.empty as a sentinel instead...

@jni
Copy link
Contributor

jni commented Nov 11, 2021

@tlambert03 where does parameter.empty come from exactly?

@jni
Copy link
Contributor

jni commented Nov 11, 2021

(Also this is incredible, I thought my issue might eventually be fixed in a few months... 😂)

@tlambert03
Copy link
Member Author

@tlambert03 where does parameter.empty come from exactly?

In [1]: import inspect

In [2]: inspect.Parameter.empty
Out[2]: <class 'inspect._empty'>

@tlambert03
Copy link
Member Author

and...

In [3]: def f(x): ...

In [4]: inspect.signature(f).parameters
Out[4]: mappingproxy({'x': <Parameter "x">})

In [5]: inspect.signature(f).parameters['x'].default
Out[5]: <class 'inspect._empty'>

@tlambert03
Copy link
Member Author

(Also this is incredible, I thought my issue might eventually be fixed in a few months... 😂)

turned out to be relatively trivial!

@tlambert03
Copy link
Member Author

(Also this is incredible, I thought my issue might eventually be fixed in a few months... 😂)

can I assume this is approved? wanna hit the button? :)

@jni jni merged commit ef899e1 into pyapp-kit:main Nov 14, 2021
@tlambert03 tlambert03 added the enhancement New feature or request label Jan 1, 2022
@tlambert03 tlambert03 deleted the partial branch November 13, 2022 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for partial/curried functions
2 participants