forge
forge copied to clipboard
Composition of `copy`/`synthetize` revision with `modify` revision does not track _interface name_ correctly
Thank you for this great library and for the huge effort that has been made on documentation.
I'm experimenting a minor problem (I surely have workaround for it, but isn't it the whole point of forge to simplify our life?). I actually think that it is a bug, as it is in my opinion not the expected behavior. It concerns composition of revisions, and more precisely composition of copy
(or synthetize
) with modify
. Doing so, it seems that the flattening process of the composition of revisions do not track the interface name of parameters correctly (at least, not as I expected).
MWE
Here is a minimal (non-)working example:
import forge
def f(a):
pass
@forge.copy(f)
@forge.modify("b", name="a")
def g(b):
pass
The expected result (in my opinion) would be that function g
is well defined with signature g(a)
, but instead I obtain a TypeError
with the message Missing requisite mapping to non-default positional or keyword parameter 'b'
. I understand that the modify
revision has not been considered when flattening.
Other tests
I did some tests and it appears that, contrary to modify
(and probably other revisions), synthetize
suffers the same issues. (This actually makes sense since, just as with copy
, the signature set by synthetise
is not based on the current signature of the function, contrary to the ones defined by modify
and other revisions that alter the current one.) Here is the example:
@forge.modify("b", name="a")
@forge.modify("c", name="b")
def g(c):
pass
#works fine: g well-defined is defined with signature g(a)
@forge.synthetize(*forge.fsignature(f))
@forge.modify("b", name="a")
def g(b):
pass
#fails similarly as in the minimal example given above
Expected feature
In my opinion, copy
should use the signature of the preceding revisions to build its correspondences with the copied signature, instead of directly look at the inner function. Hence, the above examples should work fine.
Thanks.
Workaround
Here is my best workaround:
@forge.modify("b", name="a")
@forge.copy(forge.modify("a", name="b"))
def g(b):
pass
we could also do:
@forge.synthetize(forge.arg("a", interface_name="b"), *forge.fsignature(f)[1:])
def g(b):
pass
but it is clearly less robust (requires the modified arg to be at position 0).
Suggestion
About this topic, I also have a suggestion:
Copying a signature of an existing function is one of the nicest way to set the signature of a new function. The copy
revision already offers a way to restrict the copied signature to some its parameters through its exclude
/include
keworded parameters. I think that renaming parameters of a copied signature — like I wanted to do in my original post — could be almost as usual as restricting it. So, I suggest to add the name_map
(or rename
?) and/or interface_name_map
(or interface_rename
?) keyworded parameter to copy
so that a map (Mapping[str, str]
) could be passed for modifying the name
and/or the interface_name
of the copied signature respectively. Thus (basing the examples on the example in the original post):
@forge.copy(f, rename={"a": "b"})
def g(b):
pass
@forge.copy(f, interface_rename={"a": "b"})
def h(b):
pass
would be equivalent to:
@forge.copy(forge.modify("a", name="b")(f))
def g(b):
pass
@forge.modify("b", name="a")
@forge.copy(forge.modify("a", name="b")(f))
def h(b):
pass
Though such a feature is not required, it could be useful in my opinion. I know that going on the path of adding unnecessary features is often not a good idea, since further extensions might follow and finally lead to a too cumbersome and hard-to-understand API, as well as possible hard-to-track bugs. Nevertheless, I think that the renaming action on copied signatures might be almost as usual as the restriction action that is already provided at this level through the exclude/include parameters.