Skip to content
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

Add universal methods #241

Merged
merged 8 commits into from
Feb 3, 2021
Merged

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Jan 25, 2021

In the rust generated Python API we need to have fixed class inpurts to
satisfy the traits used by the pyo3 macro generated FFI functions. This
results in duplicate methods like digraph_dfs_edges and graph_dfs_edges
with the same implementation just differing input types. To simplify the
API for users this commit adds universal functions to the python side of
the retworkx package to take in any retworkx graph object and dispatch to
the proper function in the rust generated api that relies on strict
input types.

Fixes #215

  • Add release notes

In the rust generated Python API we need to have fixed class inpurts to
satisfy the traits used by the pyo3 macro generated FFI functions. This
results in duplicate methods like digraph_dfs_edges and graph_dfs_edges
with the same implementation just differing input types. To simplify the
API for users this commit adds universal functions to the python side of
the retworkx package to take in any retworkx graph object and dispatch to
the proper function in the rust generated api that relies on strict
input types.

Fixes Qiskit#215
@mtreinish mtreinish added this to the 0.8.0 milestone Jan 25, 2021
@coveralls
Copy link

coveralls commented Jan 25, 2021

Pull Request Test Coverage Report for Build 528817992

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 95.684%

Totals Coverage Status
Change from base Build 528817235: 0.02%
Covered Lines: 4456
Relevant Lines: 4657

💛 - Coveralls

@mtreinish mtreinish changed the title [WIP] Add universal methods Add universal methods Jan 27, 2021
@mtreinish mtreinish removed the on hold label Jan 27, 2021
retworkx/__init__.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@lcapelluto lcapelluto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so awesome!! 🎉
The only "oof" I have is: if you have to fix a typo in a docstring, or if you want to change a default value, you have to change it now in three places 😬

@mtreinish
Copy link
Member Author

This is so awesome!! tada
The only "oof" I have is: if you have to fix a typo in a docstring, or if you want to change a default value, you have to change it now in three places grimacing

Yeah I couldn't figure out a way around that. :( We still need to export the per type functions both so they're accessible for the dispatch functions and for backwards compat. If we're going to be exporting the functions we'll want them to be documented.

@mtreinish mtreinish merged commit 00c9584 into Qiskit:master Feb 3, 2021
@mtreinish mtreinish deleted the add-dispatch-functions branch February 3, 2021 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make API uniform
3 participants