coady / multimethod

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

Maximum recursion with Union of iterables #13

Closed vnmabus closed 4 years ago

vnmabus commented 4 years ago

First, let me thank you for this small but incredibly useful package.

I have a problem when the register method tries to infer the parameters from the typing annotations. If I register a function as

@inner_product.register
def inner_product_fdatabasis(arg1: Union[FDataBasis, Basis],
                             arg2: Union[FDataBasis, Basis]):

then, upon calling the multimethod it will fail with

  ...
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\site-packages\multimethod.py", line 171, in __call__
    return self[tuple(map(self.get_type, args))](*args, **kwargs)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\site-packages\multimethod.py", line 209, in get_type
    return subtype(type(arg), *map(get_type, itertools.islice(arg, 1)))
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\site-packages\multimethod.py", line 171, in __call__
    return self[tuple(map(self.get_type, args))](*args, **kwargs)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\site-packages\multimethod.py", line 209, in get_type
    return subtype(type(arg), *map(get_type, itertools.islice(arg, 1)))
  File "C:\Users\Carlos\git\scikit-fda\skfda\representation\_functional_data.py", line 661, in __iter__
    yield self[i]
  File "C:\Users\Carlos\git\scikit-fda\skfda\representation\basis\_fdatabasis.py", line 756, in __getitem__
    return self.copy(coefficients=self.coefficients[key:key + 1])
  File "C:\Users\Carlos\git\scikit-fda\skfda\representation\basis\_fdatabasis.py", line 518, in copy
    basis = copy.deepcopy(self.basis)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\copy.py", line 180, in deepcopy
    y = _reconstruct(x, memo, *rv)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\copy.py", line 275, in _reconstruct
    y = func(*args)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\copy.py", line 274, in <genexpr>
    args = (deepcopy(arg, memo) for arg in args)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\copy.py", line 157, in deepcopy
    y = _deepcopy_atomic(x, memo)
  File "E:\Programas\Utilidades\Lenguajes\Miniconda\envs\fda\lib\copy.py", line 190, in _deepcopy_atomic
    def _deepcopy_atomic(x, memo):
RecursionError: maximum recursion depth exceeded while calling a Python object

I see from the call stack that __iter__ is being called in my objects without reason, so maybe the problem is related with iterable objects.

Registering the types manually, however, works well:

@inner_product.register(FDataBasis, FDataBasis)
@inner_product.register(FDataBasis, Basis)
@inner_product.register(Basis, FDataBasis)
@inner_product.register(Basis, Basis)
def inner_product_fdatabasis(arg1: Union[FDataBasis, Basis],
                             arg2: Union[FDataBasis, Basis]):

Thanks in advance.

coady commented 4 years ago

Thanks. Fixed in 1bba115.

What happened is when a multimethod detects that a subscripted type is in use, it switches to a custom type getter to support checking items in iterables, e.g., List[int]. In this case, it didn't need to do that (yet), because Union is more of a special case. So the check is stricter now.

But keep in mind there's still probably a bug in FDataBasis. From the traceback it looks likes that object is supposed to be iterable but is entering a infinite loop on iteration. Meaning you would see the same error if you tried to register something like FDataBasis[int], in which case a multimethod would have to try iterating the instance to work correctly.

vnmabus commented 4 years ago

Thank you for looking at this. FDataBasis is not a type which should be iterable at the class level, only instances are supposed to be iterated. Also, maybe related with the infinite recursion, is that obtaining a FDataBasis element returns another FDataBasis object (like with the str class), which maybe is confusing your methods.

Thanks again for your help and your package!