Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

f(**dict) does not work #153

Open
Wouter1 opened this issue May 30, 2024 · 20 comments
Open

f(**dict) does not work #153

Wouter1 opened this issue May 30, 2024 · 20 comments

Comments

@Wouter1
Copy link

Wouter1 commented May 30, 2024

How do I call an overloaded constructor if I have a dict with the call args ?

If the constructor is not overloaded like this it works fine using **

class B:
  def __init__(self, x):
    self.b=x

B(1)
<main.B object at 0x7faebc3e7880>
B(**{'x':1})
<main.B object at 0x7faebc3d04f0>

but when the constructor is overloaded this doesn't. The error message does not make any sense either.

class B:
  @dispatch
  def __init__(self, x):
    self.b=x

B(1)
<main.B object at 0x7faebc3e7be0>
B(**{'x':1})

Traceback (most recent call last):
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 421, in _resolve_method_with_cache
return self._cache[types]
KeyError: (<class 'main.B'>,)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 341, in resolve_method
signature = self._resolver.resolve(target)
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/resolver.py", line 168, in resolve
raise NotFoundLookupError(f"{target} could not be resolved.")
plum.resolver.NotFoundLookupError: (<__main__.B object at 0x7faebc3e7820>,) could not be resolved.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
File "", line 1, in
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 489, in call
return self._f(self._instance, *args, **kw_args)
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 398, in call
method, return_type = self._resolve_method_with_cache(args=args)
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 427, in _resolve_method_with_cache
method, return_type = self.resolve_method(args)
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 350, in resolve_method
method, return_type = self._handle_not_found_lookup_error(e)
File "/documents/Utilities/pyson/venv/lib/python3.8/site-packages/plum/function.py", line 394, in _handle_not_found_lookup_error
raise ex
plum.resolver.NotFoundLookupError: For function __init__ of __main__.B, (<__main__.B object at 0x7faebc3e7820>,) could not be resolved.

@Wouter1
Copy link
Author

Wouter1 commented May 30, 2024

note, /documents/Utilities/pyson/venv/ is just the location of my virtual env to test this.

pip list gives
pip list
Package Version


beartype 0.18.5
mypy 1.3.0
mypy-extensions 1.0.0
pip 20.0.2
pkg-resources 0.0.0
plum-dispatch 2.2.2
setuptools 44.0.0
tomli 2.0.1
typing-extensions 4.6.3
uri 2.0.0
wheel 0.40.0

@nstarman
Copy link
Contributor

nstarman commented Jun 2, 2024

dispatch happens on positional arguments.

@wesselb
Copy link
Member

wesselb commented Jun 2, 2024

@nstarman is right! :)

@Wouter1 to use dispatch, your arguments must be given as positional arguments. B(**{'x':1}) becomes B(x=1), which breaks dispatch, which requires B(1) instead of B(x=1).

Plum's design of multiple dispatch closely mimics how it works in the Julia programming language. The Julia docs are super good resource. :)

@Wouter1
Copy link
Author

Wouter1 commented Jun 3, 2024

@nstarman @wesselb thanks for the quick response!

But this is disappointing. So I now have to figure out myself which method applies and then sort the dict arguments into a list before making the call? This seems like writing a dispatch alternative myself :-(

@Wouter1
Copy link
Author

Wouter1 commented Jun 3, 2024

@nstarman @wesselb I'm looking to make a workaround, maybe you can give me some suggestions?

I'm trying to use introspection with get_type_hints but it seems not working properly.

class B:
  def __init__(self, x:int):
    self.b=x

f=getattr(B,'__init__')
get_type_hints(f)

gives {'x': <class 'int'>}

but when I use dispatch it doesn't

class B:
  @dispatch
  def __init__(self, x:int):
    self.b=x

f=getattr(B,'__init__')
get_type_hints(f)

gives [] instead

Am I missing something?

@wesselb
Copy link
Member

wesselb commented Jun 3, 2024

Hey @Wouter1! Could you give some more context about what you're trying to achieve? One alternative is to splat using only a single *:

from plum import dispatch

class B:
    @dispatch
    def __init__(self, x: int):
        self.b = x

arguments = (1,)
b = B(*arguments)

Another alternative is to avoid splatting all-together and just directly write B(argument) or B(1).

@Wouter1
Copy link
Author

Wouter1 commented Jun 4, 2024

@wesselb

Thanks, yes I understood that I need to make a list instead of a dict.

What I need to do is construct an clazz instance using the arguments I have in a dict. So I need to call clazz(**dict). Except that it won't work if the clazz has overloaded constructors using plum.

So yes I need to convert the dict to a list as that's the only available route with plum.

However the problem is that get_type_hints also isn't working when the __init__ functions are wrapped by plum, so I was unable to get the argument order and types.

After a day of searching however I found a maybe-workaround.

It appears that inspect.signature works both with normal and with @dispatch-ed methods.
However it's behaving weird, but maybe just good enough:

  • inspect.signature gives the signature of the FIRST method. So if multiple clazz.__init__ methods exists, it gives the signature of the first one in clazz.
  • it offers no way to access the other methods. For now I will inform users of my code that the first __init__ has to be the one matching their dict contents. This might still be a deal breaker for me later, I can't oversee all the consequences of this limitation
  • inspect.signature seems to have a bug somewhere. It keeps giving the same method, even if you replace clazz entirely with a new version that does not have that __init__ function anymore. I guess there's someone caching the signature in a global storage for some (marginal?) speed gain but failing to refresh.

@wesselb
Copy link
Member

wesselb commented Jun 4, 2024

If you really need to splat argument from a dictionary, a simpler alternative is to convert the dictionary to positional arguments by using a wrapper method:

from plum import dispatch


class B:
    def __init__(self, x: int):
        self._init(x)

    @dispatch
    def _init(self, x: int):
        self.b = x


b = B(**{"x": 1})

@Wouter1
Copy link
Author

Wouter1 commented Jun 4, 2024

@wesselb I'm not sure if I understand.

You now have only 1 __init__ so @dispatch is not needed.
How would you do this if there were multiple __init__ constructor methods?

@Wouter1
Copy link
Author

Wouter1 commented Jun 4, 2024

Unfortunately inspect.signature is sometimes returning just a string instead of a real class for the argument types.

After a lot more searching it shows that inspect.signature is affected by the use of from __future__ import annotations.

https://docs.python.org/3/library/inspect.html

I can not prevent users of my library from importing that, and they may need it for good reasons.

I can not quite comprehend why python makes what looks like a trivial task lead you into a maze of partially-functioning alternatives. Do I fundamentally misunderstand something? How do I get a proper signature of a method/function, even if it is @dispatch'ed or if someone imported annotations from __future__?

@wesselb
Copy link
Member

wesselb commented Jun 4, 2024

@Wouter1 here is an example with two initialisation methods:

from plum import dispatch


class B:
    def __init__(self, x: int | str):
        self._init(x)

    @dispatch
    def _init(self, x: int):
        self.b = x

    @dispatch
    def _init(self, x: str):
        self.b = int(x)


b1 = B(**{"x": 1})
b2 = B(**{"x": "1"})

You can extend this pattern to multiple arguments too.

@Wouter1
Copy link
Author

Wouter1 commented Jun 4, 2024

@wesselb Thanks for the explanation.

But this is not "two initialization methods". It's just one with a catch-all argument. This is not overloading. And it assumes I can rewrite the classes that I need to create from the dict.

For instance, what if you have init/1 and init/2 for instance? Like init(int) and init(str,str) ?

The next step would be using a general vararg. And then we're exactly where we are now: you can not infer the types anymore and I can't build the list from the dict.

@wesselb
Copy link
Member

wesselb commented Jun 4, 2024

For instance, what if you have init/1 and init/2 for instance? Like init(int) and init(str,str)?

You can use default arguments:

from plum import dispatch


class B:
    def __init__(self, x = None, y = None):
        self._init(x, y)

    @dispatch
    def _init(self, x: int, y: None):
        self.b = x

    @dispatch
    def _init(self, x: str, y: str):
        self.b = int(x) + int(y)


b1 = B(**{"x": 1})
b2 = B(**{"x": "1", "y": "2"})

I agree that it's not an ideal solution, but dispatch currently requires positional arguments, so you will require a workaround of this sort. I don't think this particular workaround is so bad.

@wesselb
Copy link
Member

wesselb commented Jun 4, 2024

The next step would be using a general vararg. And then we're exactly where we are now: you can not infer the types anymore and I can't build the list from the dict.

General variable arguments like __init__(self, *xs: int) should actually work fine! It are keyword arguments (which includes splatted dictionaries) in particular that are troublesome.

@Wouter1
Copy link
Author

Wouter1 commented Jun 4, 2024

@wesselb Thanks for the suggestions and thinking along!.
But again "you can not infer the types anymore and I can't build the list from the dict.". I can't change the __init__ of an existing class either.
So these are useless as workaround..

@wesselb
Copy link
Member

wesselb commented Jun 5, 2024

But again "you can not infer the types anymore and I can't build the list from the dict.".

Could you elaborate on what you mean by not being able to infer the types anymore? The idea of dispatch is that you specify types for every function argument and then choose the method based on the types of the given arguments (in this case, keys in the dictionary). This in particular means that you have to name and specify the types of all keys in the dict.

I can't change the init of an existing class either.

Technically, you could do something like this:

from plum import dispatch


class B:
    def __init__(self, x = None, y = None):
        print("Old init!")


old_init = B.__init__


def new_init(self, x = None, y = None):
    old_init(x, y)
    new_init_inner(self, x, y)
    

@dispatch
def new_init_inner(self: B, x: int, y: None):
    self.b = x


@dispatch
def new_init_inner(self: B, x: str, y: str):
    self.b = int(x) + int(y)


B.__init__ = new_init

Though of course this might not be desirable depending on your use case.

@Wouter1
Copy link
Author

Wouter1 commented Jun 6, 2024

Could you elaborate on what you mean by not being able to infer the types anymore? The idea of dispatch is that you specify types for every function argument and then choose the method based on the types of the given arguments (in this case, keys in the dictionary). This in particular means that you have to name and specify the types of all keys in the dict.

I think the confusion stems from what you mean by "you" in "you specify types".

Let me try to explain in another way. Let's define my software as a method create( List[class names:str], description:dict) -> class_instance

What happens is that the software searches the list of classes for one matching the description. Then it takes the constructor arguments from the description and creates an instance of that class

I am NOT writing the classes, nor the description. That's done by the users of my library.

My code needs to search the actual classes, check their constructors, and match them with the data in the description.

I would like to support @dispatch so that my users can overload their constructors for more flexibility. But that is only possible if I can determine the signatures of the classes provided by my users. Also I should not put a lot of extra requirements on my users, like writing new __init__ functions, this is exactly what @dispatch should be for

@wesselb
Copy link
Member

wesselb commented Jun 13, 2024

Hmm, one possible solution would to not pass the description as a dictionary but as plain arguments, and pass these to the class:

from plum import dispatch


def instantiate(cls, *args, **kw_args):
    return cls(*args, **kw_args)


class MyClass:
    @dispatch
    def __init__(self, x: int):
        self.x = x


a = instantiate(MyClass, 1)

Would something like this be acceptable?

@Moosems
Copy link

Moosems commented Aug 17, 2024

If you really need to splat argument from a dictionary, a simpler alternative is to convert the dictionary to positional arguments by using a wrapper method:

from plum import dispatch





class B:

    def __init__(self, x: int):

        self._init(x)



    @dispatch

    def _init(self, x: int):

        self.b = x





b = B(**{"x": 1})

@wesselb You may consider adding this to the doc in the keyword arguments section as this is a fairly reasonable workaround (but not as good as it could be).

@wesselb
Copy link
Member

wesselb commented Aug 18, 2024

@Moosems Thanks! Good suggestion. I've added this to the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants