beartype / plum

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

Warning about double-registration #171

Open nstarman opened 2 months ago

nstarman commented 2 months ago

In https://github.com/GalacticDynamics/coordinax/pull/128 I am trying to build with plum v2.4.3, but I am seeing many MethodRedefinitionWarning warnings about my constructor methods, but the earlier definition it compares against is the same function!

e.g.

ERROR tests/test_base.py - plum.resolver.MethodRedefinitionWarning: Method(function_name='constructor', signature=Signature(type[coordinax._base.AbstractVector], collections.abc.Mapping[str, unx, return_type=<class 'coordinax._base.AbstractVector'>, impl=<function AbstractVector.constructor at 0x7fc60876f2e0>) overwrites the earlier definition Method(function_name='constructor', signature=Signature(type[coordinax._base.AbstractVector], collections.abc.Mapping[str, unx, return_type=<class 'coordinax._base.AbstractVector'>, impl=<function AbstractVector.constructor at 0x7fc60876f2e0>).

where both functions are 0x7fc60876f2e0.

nstarman commented 2 months ago

@patrick-kidger, I managed to isolate the problem somewhat and Equinox is involved. It doesn't happen when I use a dataclasses.dataclass instead.

Here'a MWE that raises a warning

from collections.abc import Mapping
from dataclasses import dataclass

import equinox as eqx
from plum import dispatch

class Class(eqx.Module):
    a: float
    b: float

    @classmethod
    @dispatch
    def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
        return cls(**obj)

Emits MethodRedefinitionWarning: `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x177cdade0>)` overwrites the earlier definition `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x177cd80e0>)`.

nstarman commented 2 months ago

Sorry @Patrick-kidger, that may hav been a spurious finding. When I updated equinox this wasn't sourced by equinox any more.

wesselb commented 2 months ago

@nstarman, hmmm, I seem to be unable to reproduce the issue in a fresh environment with Python 3.10:

Python 3.10.13 (main, Jan 16 2024, 14:46:45) [Clang 14.0.0 (clang-1400.0.29.202)]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.26.0 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from collections.abc import Mapping
   ...: from dataclasses import dataclass
   ...:
   ...: import equinox as eqx
   ...: from plum import dispatch
   ...:
   ...: class Class(eqx.Module):
   ...:     a: float
   ...:     b: float
   ...:
   ...:     @classmethod
   ...:     @dispatch
   ...:     def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
   ...:         return cls(**obj)
   ...:

In [2]: Class.constructor._f.methods
Out[2]:
List of 1 method(s):
    [0] constructor(cls: type, obj: collections.abc.Mapping) -> __main__.Class
        <function Class.constructor at 0x10a530af0> @ /<ipython-input-1-9a739e5fe7aa>:11

Could it possibly be the case that you accidentally ran the snippet twice in the same session? That would produce a redefinition error:

In [1]: from collections.abc import Mapping
   ...: from dataclasses import dataclass
   ...:
   ...: import equinox as eqx
   ...: from plum import dispatch
   ...:
   ...: class Class(eqx.Module):
   ...:     a: float
   ...:     b: float
   ...:
   ...:     @classmethod
   ...:     @dispatch
   ...:     def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
   ...:         return cls(**obj)
   ...:

In [2]: Class.constructor._f.methods
Out[2]:
List of 1 method(s):
    [0] constructor(cls: type, obj: collections.abc.Mapping) -> __main__.Class
        <function Class.constructor at 0x10a530af0> @ /<ipython-input-1-9a739e5fe7aa>:11

In [3]: from collections.abc import Mapping
   ...: from dataclasses import dataclass
   ...:
   ...: import equinox as eqx
   ...: from plum import dispatch
   ...:
   ...: class Class(eqx.Module):
   ...:     a: float
   ...:     b: float
   ...:
   ...:     @classmethod
   ...:     @dispatch
   ...:     def constructor(cls: "type[Class]", obj: Mapping[str, float], /) -> "Class":
   ...:         return cls(**obj)
   ...:
/Users/wessel/Desktop/venv/lib/python3.10/site-packages/plum/resolver.py:269: MethodRedefinitionWarning: `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x103b94280>)` overwrites the earlier definition `Method(function_name='constructor', signature=Signature(type[__main__.Class], collections.abc.Mapping[str, float]), return_type=<class '__main__.Class'>, impl=<function Class.constructor at 0x10a530af0>)`.
  warnings.warn(
wesselb commented 2 months ago

Oops, I completely missed your last message, @nstarman! Glad that the issue seems to have gone away. :) Is this ready to be closed?

nstarman commented 2 months ago

I'm still getting the issue everywhere in https://github.com/GalacticDynamics/coordinax/pull/128. It's just not (maybe?) caused by Equinox. I'm suspicious of jaxtyping.install_import_hook ATM, but have no proof.

wesselb commented 2 months ago

Hmm, let's leave it open for now then to see if anything else comes up. I'll try to release the warn_redefinition keyword soon, which should be an easy change.

nstarman commented 2 months ago

Great. I'll probably use that in coordinax so I can unpin plum. Then I can continue trying to find a MWE.

wesselb commented 1 month ago

@nstarman v2.5.1 makes redefinition warnings opt-in by setting warn_redefinition=True in Dispatcher, Function, or Resolver.