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

Rethink what i2.Sig instances should do, or not do. #16

Open
thorwhalen opened this issue Mar 12, 2022 · 0 comments
Open

Rethink what i2.Sig instances should do, or not do. #16

thorwhalen opened this issue Mar 12, 2022 · 0 comments
Assignees
Labels
invalid This doesn't seem right

Comments

@thorwhalen
Copy link
Member

thorwhalen commented Mar 12, 2022

Compatible signatures

The __signature__ of just changes what inspect.signature will show us (as well as what some IDEs (e.g. using Jedi) will show us. But there is another signature. The internal one. The one that apparently can't be changed dynamically at runtime (though __signature__ can!).

from i2 import Sig
from inspect import signature

@Sig(lambda x, y: None)
def foo(x, *, y):
    return x + y

# foo has the signature we want (with POSITIONAL_OR_KEYWORD y)
assert str(signature(foo)) == '(x, y)'
assert foo(1, y=2) == 3
# But the function still doesn't work with POSITIONAL_OR_KEYWORD
assert foo(1, 2) == 3  # TypeError: foo() takes 1 positional argument but 2 were given

So in order to keep things sane when changing signatures, we had better not allow the user (unless they explicitly ask us to) change the signature of a function to one that is not compatible with the "actual/real/internal/unchangeable" signature that will be used at runtime.

What does compatible signature mean?

One that makes function call behavior consistent with what the user sees through inspect.signature.
For instance

  • if the signature says y is POSITIONAL_OR_KEYWORD, we should be able to call the function with either position or keyword argument specification
  • if the signature says the default of y is 2, we should have the same effect we have by calling the function with y=2 as we have by not specifying y at all
  • on the other hand, we should be able to permute any of the KEYWORD_ONLY parameters and still have a consistent signature, since this doesn't effect the function's behavior at all
  • all signatures are consistent with the (*args, **kwargs) signature
  • all signatures that have only POSITIONAL_ONLY and VAR_POSITIONAL are consistent with (*args)
  • all signatures that have only KEYWORD_ONLY and VAR_KEYWORD are consistent with (**kwargs)

So what we need is a signatures_are_compatible(outer_sig, inner_sig) -> bool such that, for any func with (real/actual) signature inner_sig, signatures_are_compatible(outer_sig, inner_sig) == True if, and only if, the function wrapped_func defined as:

@outer_sig
def wrapped_func(*args, **kwargs):
    return func(*args, **kwargs)    

is such that, any (args, kwargs) inputs that are valid inputs of func are also valid inputs of wrapped_func.
Though note that func(*args, **kwargs) and wrapped_func(*args, **kwargs) could return different outputs (the defaults could be different.

image

Appendix: More materials and rumination that lead to the above design

One often expects signatures to have an effect of the computation of a function, but it doesn't (see this and other references listed below).

Right now, when wrapping with a Sig instance, we change the __signature__, but also tamper with the __defaults__ and __kwdefaults__ of the function to keep them aligned with the signature. Seems like the right thing to do, but it's not enough. It doesn't have the desired effect anyway.

So either we should make Sig:

  • do more work (which will involve wrapping the function and wiring the interface params to the wrapped ones),
  • or make it just assign itself to the __signature__ of the input function -- possibly with a warning or error if it's not a sane thing to do (though maybe some extras could be considered, such as assigning a __name__ if the wrapped doesn't have one.)

Here are a few examples of unexpected things current Sig does (or doesn't do).
Even more examples of signature and wraps weirdness in the references below.

@Sig(lambda x, y, z: None)
def foo(*args, **kwargs):
    return args, kwargs

assert str(Sig(foo)) == '(x, y, z)'
assert foo(1,2,s=3) == ((1, 2), {'s': 3})  # but want to disallow s, since not signature compliant


@Sig(lambda x, y, z=3: None)
def foo(*args, **kwargs):
    return args, kwargs

assert foo(1,2) == ((1, 2), {})  # where's z=3?

More:

def foo(x, y, z=0):
    return x + y * z

assert foo.__defaults__ == (0,)
assert foo(1, 2) == 1


@Sig(lambda x, y, z=3: None)
def foo(x, y, z=0):
    return x + y * z
assert foo(1, 2) == 7
#    works because Sig also changed __defaults__:
assert foo.__defaults__ == (3,)



@Sig(lambda x, y, *, z=3: None)
def foo(x, y, *, z=0):
    return x + y * z

assert foo(1, 2) == 7
#    works because Sig also changed __defaults__ and __kwdefaults__:
assert foo.__defaults__ == ()
assert foo.__kwdefaults__ == {'z': 3}


# But these don't work:

@Sig(lambda x, y, *, z=3: None)
def foo(x, y, z=0):
    return x + y * z

foo(1, 2)  # TypeError: foo() missing 1 required positional argument: 'z'


@Sig(lambda x, y, z=3: None)
def foo(x, y, *, z=0):
    return x + y * z

foo(1, 2)  # TypeError: foo() missing 1 required keyword-only argument: 'z'

References

Relevant questions I've asked on stackoverflow:

Also relevant, a bpo and a pull request I made about functools.wraps.

Possible solutions

I say "solutions", but really the main issue here is to decide what i2.Sig should or should not do.
Should it just write to __signature__, or should it also write on __defaults__ and __kwdefaults__, or should it go all the way to wrapping the function and rewire the inputs?

The solution I'm leaning towards right now is leave Sig be back-compatible (so continue doing the extra __defaults__ and __kwdefaults__ stuff, but return an error (or warning?) when defaults AND kinds change, since the user will only know that there's a problem at run time.

Elements of solutions

Following are some things to consider.

If kind and signature change we can implement the effect we want with i2.wrapper.

The following use of partial to take care of defaults is notable:

from functools import partial

@partial(partial, z=3)
def foo(x, y, z=0):
    return x + y * z

assert str(signature(foo)) == '(x, y, *, z=3)'  # note that z is keyword-only now
assert foo(1, 2) == 7

But it's not really a solution since this changes the signature will change and we can't just do this:

from functools import partial

@Sig(lambda x, y, z=3: None)
@partial(partial, z=3)
def foo(x, y, z=0):
    return x + y * z

assert str(signature(foo)) == '(x, y, z=3)'  # okay
assert foo(1, 2) == 7  # okay
# ... but what's not okay is:
foo(1, 2, 3)  # TypeError: foo() got multiple values for argument 'z'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

4 participants