Composition of copy/synthetize revision with modify revision does not track _interface name_ correctly

[guillonb]

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.

[guillonb]

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).

[guillonb]

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.