refactor: simplified and updated Python/PyZX bindings #106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akissinger
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a WiP, where I've been working on revamping the Python bindings. Currently, the bindings are a bit outdated and incomplete. They also "double wrap" the main rust classes. For example, VecGraph is wrapped in a PyO3 class written in rust, which in turn is wrapped in a subclass of
BaseGraphwritten in python.I originally did this to overcome a limitation in PyO3 that you can't extend classes written in python directly with rust bindings. However, this means you have to write twice as much boilerplate to extend the bindings, and you have to deal with this "double-wrapping" not only for graphs but any other rust functions or classes that consume graphs.
I think the simplest way around this is to just re-implement the helper methods provided by
BaseGraphdirectly in rust (or throw "Not implemented" exceptions for those we don't care too much about), then "pretend" that theVecGraphwrapper class provided by the rust bindings extendsBaseGraph, just by saying so in the mypy type stubs.While I haven't tested this approach extensively, this will certainly do the right thing in code that relies on duck-typing, and should satisfy any python type checker.