Import Callable from collections.abc#474
Conversation
py4j-python/src/py4j/clientserver.py
Outdated
|
|
||
| from collections import deque, Callable | ||
| from collections import deque | ||
| from collections.abc import Callable |
There was a problem hiding this comment.
The problem was that it breaks Python 2 so I used the deprecated one. Probably we can just remove the usage of Callable, and check if it is a weakref reference.
There was a problem hiding this comment.
That makes perfect sense! Sorry I missed that. I'll to get a python 2 compatible version up tomorrow.
There was a problem hiding this comment.
Nah, thanks for addressing this issue man.
There was a problem hiding this comment.
According to the internet, it looks like the builtin callable should be a solution that works for all 2.X and >=3.2. Is that a safe range for py4j at this point?
There was a problem hiding this comment.
I've verified locally on 2.7, 3.9, and 3.10. I've also verified that in 3.10 an ImportError is raised on from collections import Callable
There was a problem hiding this comment.
Yeah it broke python 3.10 support. Or we can even just remove the assert for now.
This should be compatible with all 2.X and >= 3.2 CITE: https://bugs.python.org/issue10518
|
Hmm. GitHub seems to be struggling this morning, but may have resolved. When I pushed up the latest commit to my branch, it doesn't seem to be getting picked up by this PR. I'm going to try closing and reopening |
|
Actually #475 suggested the fix .. let me merge that one and close this one - will add you as a co-author of #475. Thanks for investigation @gabe-l-hart! |
Closes #474 Co-authored-by: Gabe Goodhart <ghart@us.ibm.com>
|
Thanks for getting the fix through! |
Description
This PR attempts to address #473