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
singledispatch register should typecheck its argument #72171
Comments
from functools import singledispatch
from enum import Enum
IS = Enum("IS", "a, b")
@singledispatch
def foo(x):
print(foo.dispatch(x))
print("foo")
@foo.register(IS.a)
def bar(x):
print(foo.dispatch(x))
print("bar")
@foo.register(int)
def baz(x):
print("baz")
>>> foo(IS.a)
<function bar at 0x7fa92c319d90>
foo I think the result is self-explaining. The foo.dispatch() correctly says function bar should be handling the call when IS.a is passed, but foo is handling the call anyway. If an int is passed, baz works as expected. |
functools is meant to work with types, but the enum member "IS.a" is not a type -- however, the enum "IS" is a type. Use "foo.register(IS)" instead. |
The register() method should raise a TypeError if it's called with an object that's not a type. Consider the following: from functools import singledispatch
class C:
pass
obj = C()
@singledispatch
def foo(x):
print(foo.dispatch(x))
print("foo")
@foo.register(obj)
def bar(x):
print(foo.dispatch(x))
print("bar") >>> foo(obj)
<function bar at 0x7f16f2adc9d8>
foo |
Catching the erroneous registration rather than silently ignoring it sounds like the right thing to do here to me as well. I'm actually surprised that code isn't already throwing an exception later on, as "isinstance" itself does fail with non-types: >>> from enum import Enum
>>>
>>> IS = Enum("IS", "a, b")
>>> isinstance(IS.a, IS)
True
>>> isinstance(IS.a, IS.a)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: isinstance() arg 2 must be a type or tuple of types |
It's also better to add the typecheck to dispatch. Otherwise sometimes it can generate not obvious exception message. >>> from enum import IntEnum
>>> from functools import singledispatch
>>> IS = IntEnum("IS", "a, b")
>>> @singledispatch
... def foo(x):
... pass
...
>>> foo.dispatch(IS.a)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/angwer/cpython/Lib/functools.py", line 718, in dispatch
impl = dispatch_cache[cls]
File "/home/angwer/cpython/Lib/weakref.py", line 365, in __getitem__
return self.data[ref(key)]
TypeError: cannot create weak reference to 'IS' object
>>> |
@xiang.zhang, would you be able to make a PR for your patch? Thanks! |
Thanks for the reminding, I already forget this. I'll do it night. |
Revising the patch, register() is fixed by feature in bpo-32227. So I will only fix dispatch() in 3.7 and 3.8, the whole patch to 3.6. |
I'm sorry that you wasted your time on those pull requests, Xiang but:
The wrapper returned by the We should not slow down calling of all generic functions on the basis that somebody might pass a non-type straigh to Cheryl, thanks for your help triaging the bug tracker! It might be a good idea to confirm with people on old issues whether moving the patch to a pull request is indeed the missing part of it getting merged. In this case it wasn't, I hope Xiang won't feel bad about me rejecting his pull requests after you pinged him for them. |
It's all right. |
Łukasz, Sorry about that. Since Xiang is a core developer, I thought he would be in the position to make the decision about moving forward with the patch. I apologize for causing unnecessary work. |
Cheryl, It's a good remind for me. I totally forget about this issue.
Yes, I made the decision. But seems not a wise one, sorry. :-( But fortunately Łukasz noticed it. :-) I am happy to see finally this issue is solved, instead of hanging with no reply. :-) Thanks for Łukasz and Cheryl. I am sorry for the PRs making noise here. Again, it's quite all right for me to close them. :-) |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: