-
Notifications
You must be signed in to change notification settings - Fork 151
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
Automagic dispatch for universal functions #883
Conversation
Pull Request Test Coverage Report for Build 7997454293Details
💛 - Coveralls |
b5a818e
to
08aefbb
Compare
This is ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I scanned through all this and all the re-registrations look right to me, including the is_isomorphic_node_match
rewrite to not be a direct dispatcher itself.
I personally avoid magic name inference like this, but this will fail loudly and immediately at import time if something's not available, so I'd agree it should be maintainable enough (and the line-count reduction is nice for sure).
My ticks don't count on rustworkx, but you can have one in principle (with or without the importlib
change).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, we end up doing this a lot so abstract away the boilerplate makes a lot of sense here.
Follow up of #882, albeit more agressive on shortening the boilerplate amount of code for universal functions
This PR introduces a new
_rustworkx_dispatch
decorator that:func
digraph_func
forPyDiGraph
graph_func
forPyGraph
The decorator is based on the original
functools.singledispatch
. It should simplify the amount of code we need to write an universal function. It does add a little bit of "magic" with functools and importlib, but overall it should be maintainable