beartype / plum

Multiple dispatch in Python
https://beartype.github.io/plum
MIT License
497 stars 22 forks source link

Incorrect dispatching according to varargs #117

Open PhilipVinc opened 7 months ago

PhilipVinc commented 7 months ago

See the following example where I create two signatures, where the latter is more specialised than the former but also accepts varargs.

Plum interprets this as a dispatch ambiguity, but I think it should not and should resolve to the second one (Julia does the same).

from plum import dispatch
from numbers import Number

@dispatch
def test(x: int, y: Number):
    return 1

@dispatch
def test(x: int, y: int, *args):
    return 2

test(1, 1)
---------------------------------------------------------------------------
AmbiguousLookupError                      Traceback (most recent call last)
Cell In[6], line 1
----> 1 test(1, 1)

    [... skipping hidden 2 frame]

File ~/Dropbox/Ricerca/Codes/Python/plum/plum/function.py:325, in Function.resolve_method(self, target)
    323 except AmbiguousLookupError as e:
    324     __tracebackhide__ = True
--> 325     raise self._enhance_exception(e) from None  # Specify this function.
    327 except NotFoundLookupError as e:
    328     __tracebackhide__ = True

AmbiguousLookupError: For function `test`, `msg(1, 1)` is ambiguous among the following:

 [0] Method(function_name='test', signature=Signature(int, numbers.Number), return_type=typing.Any, impl=<function test at 0x10742e020>)
 [1] Method(function_name='test', signature=Signature(int, int, varargs=typing.Any), return_type=typing.Any, impl=<function test at 0x107429620>)
PhilipVinc commented 7 months ago

I think a major problem comes from this line


    def __le__(self, other) -> bool:
        # If this signature has variable arguments, but the other does not, then this
        # signature cannot be possibly smaller.
        if self.has_varargs and not other.has_varargs:
            return False
        ...

because in the case above the signature of the varagrg method is automatically deemed more general aka less specific than the one of the first method, but (following Julia's approach) we should compare the signature obtained by expanding the varargs.

(This might have side effects)

wesselb commented 7 months ago

@PhilipVinc, whoa, you might have caught onto a pretty serious bug.

I'm wondering what the right formalism is to view this.

One perspective is to make a distinction between concrete types (int, float, et cetera) and abstract types (Number, et cetera) and to say that methods are unions of signatures of concrete types. For example, in a universe where only int, float, and Number exist,

Method(int, Number) = {(int, int), (int, float)}

Method(int, int, *int) = {(int, int), (int, int, int), (int, int, int, ...)}

Then a partial order on signatures is introduced by checking whether one union is a subset of the other. This is what Signature currently implements. In the example above, the union corresponding to Method(int, Number) is not a subset of the union corresponding to Method(int, int, *int).

Where I think this goes wrong is that Method(int, int, *int) should not be expanded into a big union. Instead, Method(int, int, *int) should be interpreted as many methods Method(int, int), Method(int, int, int), Method(int, int, int, int) et cetera, which in turn correspond to unions with just one element.

One possible approach to implementing this would be to simply remove the highlighted if-statement. However, this now has the consequence that e.g.

In [1]: from plum import Signature

In [2]: Signature(int) == Signature(int, varargs=int)
Out[2]: True

This is not right either.

What might need to happen is that, at the appropriate moment, Signature(int, varargs=int) should be split into e.g. {Signature(int), Signature(varargs=int)}, both having the same implementation. Hmmm, this might require some thought.

Also sorry for not yet reviewing your PRs! Will get to that soon.

PhilipVinc commented 7 months ago

One perspective is to make a distinction between concrete types (int, float, et cetera) and abstract types (Number, et cetera) and to say that methods are unions of signatures of concrete types.

(I'm unsure where you want to go with this but...) I don't think we can make this distinction in Python. If we forget for a moment abc.ABC abstract classes, all types are 'concrete' and can be instantiated, and 'concrete' types can be subclassesed (class MyInt(int): pass).

Instead, Method(int, int, *int) should be interpreted as many methods

I perfectly agree. From a Julia point of view this is because the varargs are encoded as a type parameter and type parameters are more akin to 'different methods' than a general method. Actually, if we had proper support for type parameters this could be solved easily with type pars.

However, this now has the consequence that e.g.

This suggests me that the correct way to handle dispatch is not through a total ordering interpretation as we are currently doing... For example, is our current infrastructure capable of handling type parameters? I suspect not..