coady / multimethod

Multiple argument dispatching.
https://coady.github.io/multimethod
Other
277 stars 24 forks source link

Fix partial ordering, kwargs behavior and associated tests #35

Closed cjalmeida closed 3 years ago

cjalmeida commented 3 years ago

Scenario giving dispatch error:

def test_nargs_precedence():
    class AbstractFoo:
        pass

    class Foo(AbstractFoo):
        pass

    @multimethod
    def temp(a):
        return "fallback"

    @multimethod
    def temp(a: AbstractFoo, b: bool):
        return "2 args"

    @multimethod
    def temp(a: Foo):
        return "1 arg"

    assert temp(1) == "fallback"
    assert temp(Foo(), False) == "2 args"
    assert temp(Foo()) == "1 arg"

The equivalent Julia dispatch works as stated in the scenario above.

The actual fix changed the semantic of partial ordering in signature.__le__ (line: 115); ie. not should never dispatch on a function with len(signature_args) != len(call_positional_args). From Julia dev docs, this is the actual behavior of the dispatch logic.

The side-effect was no longer handling POSITIONAL_OR_KEYWORD and VAR_POSITIONAL properly. I added a bunch of tests to further stress the scenarios.

I adapted get_types returning an iterable of signatures, eg. both (object, object) and (object,) when it encounters a POSITIONAL_OR_KEYWORD. This solution [mimics the Julia solution (https://docs.julialang.org/en/v1/manual/functions/#Optional-Arguments). For variadic positional args, the solution was to return a variadic argument marker, then expand the signature arguments, padding with objects as needed.

Fixed tests for internal checks where needed.

codecov[bot] commented 3 years ago

Codecov Report

Merging #35 (6a85a5e) into main (68ad1b7) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main       #35   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          202       234   +32     
  Branches        36        42    +6     
=========================================
+ Hits           202       234   +32     
Impacted Files Coverage Δ
multimethod/__init__.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 68ad1b7...6a85a5e. Read the comment docs.

coady commented 3 years ago

Thanks. I'd like to try out the idea described in issue #36 first. Attempting to bind the arguments first handles the case of too many positional arguments.