-
Notifications
You must be signed in to change notification settings - Fork 159
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
Include most library functionality in rustworkx-core #1121
Comments
I think porting our core functionality from One opportunity I would like to explore for us to have yet-another layer of tests on top of the library. Like a fuzzing test suite at the Rust level would be way less expensive than on the Python counterpart but find equally as many logical bugs. However, I don't think we should require contributions to always include Another point is that some contributions like #788 would have been harder to implement in pure Rust first, not in the sense of being hard but getting the interface right first. And I think we'd need to veto our dependencies more carefully if we add everything to |
I think as we build out I've created #1124 to capture discussion on the error handling interface we may want to expose from core. Separately, I think there may be value in introducing the "extension trait" pattern to With the extension trait pattern, we'd define traits for each of the graph methods ported from the Python API, along with "blanket" implementations of each for a generic graph I was thinking that if we do go with an approach like this one, it'd make sense to put such extension traits into a I've got a prototype for this in the same branch containing the error handling interface. I may work this into a PR to make it more visible, but I'm very happy to change any or all of this—the prototype exists to communicate how this idea would manifest. |
I agree, we'll need to rethink traits, error types, maybe even introduce crate features to stop pulling all the dependencies we currently add. I do think you should open the PR with the improvements and we can start the discussion from there. Overall it will be a positive addition, it is just that we will need to think more about code we merge. |
What is the expected enhancement?
Right now we have a divide between what's available in rustworkx and rustworkx-core. The rustworkx python API has far more functionality than rustworkx-core. This makes sense because the library started with just rustworkx and there was no rust api. But I've been finding an increasing number of times where I would like to access things we have implemented in rustworkx from other rust code to find it missing from rustworkx-core. I think we really should move towards trying to close this divide between the crates and ensure rustworkx-core and rustworkx are at feature parity.
I'm proposing that we migrate as much of the functionality as possible to
rustworkx-core
so that everything we expose to Python users are also available to rust users. We have some one off issues for higher priority items like #769, #741, and #602.To start I think maybe we should at least have a policy of all new algorithms are written for rustworkx-core and the python interface is built off of that.
Tasks
lexicographical_topological_sort()
functionality to rustworkx-core #1165collect_bicolor_runs()
functionality to rustworkx-core #1166layers
functionality to rustworkx-core #1167The text was updated successfully, but these errors were encountered: