-
Notifications
You must be signed in to change notification settings - Fork 24
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
Does not work with future annotations #41
Comments
Thanks, @gasparyanartur. This is something that will have to be fixed. I imagine that it shouldn't be too complicated to get right. |
Hi @wesselb 👋 I've been looking at replacing Unfortunately this issue would be a blocker, but I'm happy to try implement a fix. If you could perhaps provide some pointers it would be super helpful and very much appreciated! |
Hey @gasparyanartur and @seeM, Since there is increased interest in this issue, I've taken a stab at supporting Currently, there is just one gotcha. On Python 3.7 and 3.8 (not 3.9), if you're using Yes: from plum import dispatch
class A:
@dispatch
def test(self) -> "A":
return self Also yes: from __future__ import annotations
from plum import dispatch
class A:
@dispatch
def test(self) -> A:
return self But not this: from __future__ import annotations
from plum import dispatch
class A:
@dispatch
def test(self) -> "A":
return self |
Ok, actually, it's actually pretty easy to specifically deal with patterns of the form from __future__ import annotations
from plum import dispatch
class A:
@dispatch
def test(self) -> "A":
return self I've commit a change to from __future__ import annotations
from typing import Union
from plum import dispatch
class A:
@dispatch
def test(self) -> Union[None, "A"]:
return self won't work. In that case, you really must not use quotes (on Python 3.7 and 3.8—things are fine on 3.9): from __future__ import annotations
from typing import Union
from plum import dispatch
class A:
@dispatch
def test(self) -> Union[None, A]:
return self |
Awesome, I'll take a look at it a bit later |
Hey @wesselb, I tried out the lastest version and it works for the use-case I specified! However, it still breaks when you try dispatching with multiple classes. You mention something in the README about using plum.type.PromisedType instead, so I'm not sure if this is intended behavior or not. The following code breaks:
I've added this test along some other in my local fork. If you want, I could push it and make a pull request. |
Hey @gasparyanartur! Thanks for checking this! Would you be able to post the full example here? E.g., the following works for me locally: from __future__ import annotations
from plum import dispatch
class A:
@dispatch
def do_a(self, a: A) -> A:
return a
@dispatch
def do_b(self, a: B) -> B:
return a
class B:
@dispatch
def do_a(self, a: A) -> A:
return a
@dispatch
def do_b(self, a: B) -> B:
return a
a = A()
b = B()
a.do_a(a)
a.do_b(b)
b.do_a(a)
b.do_b(b) I think that should be roughly along the lines of your example, right? |
Did you mean to write do_a instead of do_b? In that case it is the same as my example. Regardless, it turns out there was a bug in my test. This code does work on my machine as well. However, it does not work in my project for some reason. Here is the code gist:
from __future__ import annotations
from abc import ABC
from plum import dispatch
from dataclasses import dataclass
@dataclass(frozen=True, order=True, unsafe_hash=False, match_args=True, eq=True)
class Vector(ABC):
x: int
y: int
@property
def tuple(self):
return self.x, self.y
class Position(Vector):
@property
def algebraic(self):
return f"{chr(ord('a') + self.x)}{self.y + 1}"
@property
def i(self) -> int:
return self.y * 9 + self.x
@staticmethod
def from_index(i: int) -> Position:
y: int = i // 9
x: int = i - y
return Position(x, y)
def in_board(self) -> bool:
return (0 <= self.x < 9) and (0 <= self.y < 9)
def __eq__(self, other: Vector) -> bool:
if not isinstance(other, Vector):
raise TypeError(f"Could not compare {type(self)} with {type(other)}")
elif not isinstance(other, Position):
return False
return self.x == other.x and self.y == other.y
def __add__(self, other: Displacement) -> Position:
return Position(self.x + other.x, self.y + other.y)
@dispatch
def __sub__(self, other: Position) -> Displacement:
return Displacement(self.x - other.x, self.y - other.y)
@dispatch
def __sub__(self, other: Displacement) -> Position:
return Position(self.x - other.x, self.y - other.y)
class Displacement(Vector):
def __eq__(self, other: Vector) -> bool:
if not isinstance(other, Vector):
raise TypeError(f"Could not compare {type(self)} with {type(other)}")
elif not isinstance(other, Displacement):
return False
return self.x == other.x and self.y == other.y
def __add__(self, other: Position | Displacement) -> Position | Displacement:
if isinstance(other, Position):
return Position(self.x + other.x, self.y + other.y)
elif isinstance(other, Displacement):
return Displacement(self.x + other.x, self.y + other.y)
else:
raise TypeError(f"Could not perform subtraction between classes {type(self)} and {type(other)}.")
def __sub__(self, other: Displacement) -> Displacement:
return Displacement(self.x - other.x, self.y - other.y)
def __mul__(self, other: Displacement | int) -> Displacement:
if isinstance(other, Displacement):
return Displacement(self.x * other.x, self.y * other.y)
elif isinstance(other, int):
return Displacement(self.x * other, self.y * other) I've included the failing test in the comments. I've added dispatching to the __sub__ function in Position, but it does not seem to work. When I try replicating the error using a different class, it seems to work though: from __future__ import annotations
from abc import ABC
from dataclasses import dataclass
from plum import dispatch
@dataclass(frozen=True, order=True, unsafe_hash=False, match_args=True, eq=True)
class X(ABC):
x: int
y: int
...
class A(X):
@dispatch
def __add__(self, a: A) -> B:
return B(self.x - a.x, self.y - a.y)
@dispatch
def __add__(self, a: B) -> A:
return A(self.x - a.x, self.y - a.y)
class B(X):
@dispatch
def __add__(self, a: A) -> A:
return a
def test():
a = A(1, 2)
b = B(1, 2)
x = a + b
assert isinstance(x, A)
y = a + a
assert isinstance(y, B) as seen in this gist. However, given the fact that I cannot replicate the bug, the error is most likely in my code. |
@gasparyanartur I've left a comment on the Gist. (The Gist seems to run for me.) |
You were right, it seemed that an old version of plum-dispatch was cached, which made that specific test crash. I tried re-cloning the repository and that solved the issue for me. The dispatching works flawlessly now. Thank you for the help! |
@gasparyanartur Absolutely no problem at all. :) I'm glad that things work for you now! I'm just reopening this issue until @seeM has also confirmed that things work on their side. |
It seems I was too hasty as well. Turns out I was running my old branch, and that's why it was working. Reinstalling everything from scratch, I still get the same error. I used Pipenv with the following file:
|
Could it be that the latest official version is not the one that was pushed to master? |
Note that I've not yet pushed a new version with the latest commit on |
Indeed, it looks like that was the error all along. Installing the latest v1.5.11 does fix the issue, and this time I double checked to make sure I was running the right branch 😅. Thank you for the quick response! I'll let you close the thread when you feel it has been resolved. |
Nice! :) I'm glad to hear that resolves the issue. Thanks for working with me to confirm that everything is running smoothly! |
Thanks so much for looking into this @wesselb 😄! Unfortunately our use-case is still hitting the error when using from plum import dispatch
# from __future__ import annotations # uncommenting breaks this snippet
@dispatch
def f(x): return 'object'
def g(x: int): return 'int'
f.dispatch(g)
assert f('a') == 'object'
assert f(0) == 'int' We're using this functionality to allow users of our Sticking to the from plum import dispatch
from __future__ import annotations
@dispatch
def f(x): return 'object'
@dispatch
def f(x: int): return 'int'
assert f('a') == 'object'
assert f(0) == 'int' |
Hey @seeM! No problem at all. Unfortunately, adding support for I've released a new version |
That's understandable! :) Hmm it still doesn't seem to be working: from __future__ import annotations
from plum import dispatch, __version__
assert __version__ == '1.5.12'
@dispatch
def f(x): return 'object'
def g(x: int): return 'int'
f.dispatch(g)
assert f('a') == 'object'
assert f(0) == 'int' ResolutionError: Promise `ForwardReferencedType(name='int')` was not kept. |
Hmm, this is very strange. The snippet runs for me locally. Let me look into it. Which Python version are you running? |
Apparently the compiled extension breaks the check for whether future annotations are used or not. That's odd. Looking into fixing this now. |
@seeM, alright, attempt two 😅: I've pushed a new version |
@wesselb it worked 😄 🎉 Again, thanks so much for the support! I have one more issue 🙈 one of these commits have broken functions with no from __future__ import annotations
import math
from plum import dispatch
f = dispatch(math.sqrt)
f(3) AttributeError: 'builtin_function_or_method' object has no attribute '__annotations__' (BTW I'm not sure why some functions don't have |
@seeM Nice! :)
We'll get there! I've pushed another release |
@wesselb That's fixed, hit another one :S Reproducible example: from __future__ import annotations
from plum import dispatch
class A: pass
def f(x: A): pass
g = dispatch(f)
h = dispatch(f)
a = A()
g(a) # h(a) also errors File ~/.pyenv/versions/3.8.9/lib/python3.8/site-packages/plum/function.py:546, in plum.function.Function.__call__()
File ~/.pyenv/versions/3.8.9/lib/python3.8/site-packages/plum/function.py:402, in plum.function.Function._resolve_pending_registrations()
File ~/.pyenv/versions/3.8.9/lib/python3.8/site-packages/plum/function.py:60, in plum.function.extract_signature()
File ~/.pyenv/versions/3.8.9/lib/python3.8/site-packages/plum/util.py:145, in unquote(x)
136 def unquote(x):
137 """Remove quotes from a string at the outermost level.
138
139 Args:
(...)
143 str: `x` but without quotes.
144 """
--> 145 while len(x) >= 2 and x[0] == x[-1] and x[0] in {'"', "'"}:
146 x = x[1:-1]
147 return x
TypeError: object of type 'type' has no len() |
Ah, that's a silly one on my part... Fixed in |
That's fixed but hit another. This one's very strange: from plum.function import extract_signature
class A:
def f(self): pass
extract_signature(A().f, True) AttributeError Traceback (most recent call last)
Input In [14], in <cell line: 4>()
2 class A:
3 def f(self): pass
----> 4 extract_signature(A().f, True)
File ~/.pyenv/versions/3.8.9/lib/python3.8/site-packages/plum/function.py:60, in plum.function.extract_signature()
AttributeError: 'method' object has no attribute '__annotations__' |
Hmm sorry, that might not be the best example for this issue... Trying to find a better one |
Ignore my last few messages, this last issue seems to be on our side. I think this is all resolved now. Thanks so much! 😄 |
Ah, are you sure there's nothing weird going on with I'm glad that everything seems to be working on your side now, though! :) I think we can close the issue for now. If you come across any other weird behaviour or bugs, please reopen the issue and I'll be on it. |
@wesselb i didn't check if you did that, but maybe it would be a good idea to add all those cases above to the test suite? |
@PhilipVinc Definitely! I indeed did that along the way of fixing all the bugs. The tests are here. |
Great! Sorry for bothering you with that! |
Not a problem at all! It's good to double check these things. |
@wesselb my bad, there is something going on with from __future__ import annotations
from plum import Function
class A:
@classmethod
def create(cls): pass
pf = Function(lambda x: x)
pf.dispatch(A.create)
pf() AttributeError: 'method' object has no attribute '__annotations__' |
@seeM Thanks! Will be solved in the next release. |
Thanks so much, this is working well too! |
I might've come across another bug on dispatched
results in:
using py 3.11.1 and plum-dispatch 2.1.0 I'm assuming 3.11 should also be fully supported..? |
@bariod, you're right! You caught a subtle bug. The bug has the do with the behaviour of |
A resolution error is raised when you make a promise using annotations.
It works if you stop importing the annotations package and use quotation marks.
The following code breaks.
If you comment out the first line, the code works.
The text was updated successfully, but these errors were encountered: