-
Notifications
You must be signed in to change notification settings - Fork 1
Description
We did a lot of work on signature compatibility, but don't see it reflected in signature wrapping.
There's arguments for and against restricting signature wrapping to compatible signatures.
The conclusion was: We need to be able to be flexible with it.
We introduced a ignore_incompatible_signatures
(set to True by default -- not sure that's the best choice).
But setting it to False
doesn't seem to have any effect.
from i2 import Sig
def foo(x=1, y=2):
return 10 * (x + y)
assert foo() == 30
sig3 = Sig(lambda a, b, c: None)
# The wrap method should complain about incompatible signatures (different name), but it doesn't.
sig3.wrap(foo, ignore_incompatible_signatures=False)
def bar(x=1, *, y=2):
return 10 * (x + y)
# The wrap method should complain about incompatible signatures (different kind), but it doesn't.
sig3.wrap(bar, ignore_incompatible_signatures=False)
Better error messages
Also, would be good to give less confusing error messages.
Consider this:
from i2 import Sig
def foo(x=1, y=2):
return 10 * (x + y)
assert foo() == 30
sig1 = Sig(lambda a=2, b=3: None)
sig1(foo) # endow foo with the sig1 signature
assert str(Sig(foo)) == str(sig1) == '(a=2, b=3)'
foo() == 50 # the new defaults took effect!
sig2 = Sig(lambda a, b=3: None)
sig2(foo) # endow foo with the sig2 signature
assert str(Sig(foo)) == str(sig2) == '(a, b=3)'
foo()
# TypeError: foo() missing 1 required positional argument: 'x'
Wrapping with a signature that didn't have a default had the effect of forcing the user to specify the default.
That's good! But the message mentions the required argument x
, not a
.
This is a message directly from the python interpreter. Is there even a way for us to inform it that the new name is a
, or would we be obliged to check/catch errors and raise our own?