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

Shortern boilerplate for singledispatch #882

Merged
merged 17 commits into from
Aug 30, 2023

Conversation

IvanIsCoding
Copy link
Collaborator

@IvanIsCoding IvanIsCoding commented May 27, 2023

Shortern boilerplate code for singledispatch. This is convenient becase we don't need repeat the function signature

note: for after 0.13 release

@IvanIsCoding IvanIsCoding changed the title [WIP] Shortern boilerplate for singledispatch Shortern boilerplate for singledispatch May 27, 2023
@coveralls
Copy link

coveralls commented May 27, 2023

Pull Request Test Coverage Report for Build 6025476956

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

Totals Coverage Status
Change from base Build 6005519045: 0.05%
Covered Lines: 15400
Relevant Lines: 15960

💛 - Coveralls

Copy link
Member

@mtreinish mtreinish 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 an excellent cleanup. It reduces the LoC and removes a layer of indirection which added like 100ns to each of the dispatch calls.

@@ -249,7 +249,9 @@ pub fn digraph_random_layout(
/// :returns: The bipartite layout of the graph.
/// :rtype: Pos2DMapping
#[pyfunction]
#[pyo3(text_signature = "(graph, first_nodes, /, horitontal=False, scale=1,
#[pyo3(
signature=(graph, first_nodes, horizontal=false, scale=1.0, center=None, aspect_ratio=1.33333333333333),
Copy link
Member

Choose a reason for hiding this comment

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

This is a good catch on the defaults not matching

@mtreinish mtreinish added the automerge Queue a approved PR for merging label Aug 30, 2023
@mtreinish mtreinish added this to the 0.14.0 milestone Aug 30, 2023
@mergify mergify bot merged commit e0af0a2 into Qiskit:main Aug 30, 2023
26 checks passed
@IvanIsCoding IvanIsCoding deleted the shorter-dispatch branch August 30, 2023 15:39
mtreinish pushed a commit that referenced this pull request Feb 22, 2024
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:

* Creates the singledispatch for func
* Automatically registers digraph_func for PyDiGraph
* Automatically registers graph_func for PyGraph

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

* Try to shorten Bellman-Ford

* More shortening

* Even more

* Almost done with shortening

* Finish shortening

* Fix layout incorrect default arguments

* Simplify distance_matrix

* Magic custom dispatcher

* Minor detail about is_isomorphic_node_match

* Black

* Use explicit package name

* Minor change

* Sync with main

* Black

* Minor fix

* Sync with new functions

* Synch with new functions

* Fix cargo lock

* Cargo.lock strikes again

* Fix black error

* Black fmt

* Do not explicitly export rustworkx_dispatch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Queue a approved PR for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants