Move collect_bicolor_runs() to rustworkx-core

[ElePT]

This is a first attempt at solving #1166, the PR introduces a new collect_bicolor_runs() function in rustworx-core and modifies the python-facing collect_bicolor_runs to use the core function under the hood (no user-facing changes).

The current function returns a <Vec<Vec<G::NodeId>>, I could not find a way to implement the iterator mentioned in the original issue because I kept getting "the size cannot be known at compilation time", but I am open to implementation suggestions.

[mtreinish]

Instead of returning NodeWeight here I think it would be simpler to just return something like:

impl Iterator<Item=Vec<G::NodeId>>

(or something like that I might have made a mistake in the exact syntax) Or alternatively a Vec<Vec<G::NodeId>>

The idea is to return an iterator of node indices for each run. This isn't exactly what the python api was returning, but retunring the node ids will let us easily iterate and map that to the weights for python, but for other rust consumer (especially Qiskit in the future) this will be lighter weight to work with because it's a simple vec get to go from an index to a weight.

Then using an iterator would be a more rust native interface to return an iterator of the runs

[coveralls]

Pull Request Test Coverage Report for Build 9446628745

Details

• 96 of 97 (98.97%) changed or added relevant lines in 2 files are covered.
• 2 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.004%) to 95.837%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
rustworkx-core/src/dag_algo.rs	74	75	98.67%

Files with Coverage Reduction	New Missed Lines	%
rustworkx-core/src/generators/random_graph.rs	2	85.04%

Totals
Change from base Build 9429147177:	-0.004%
Covered Lines:	17380
Relevant Lines:	18135

---

💛 - Coveralls

[mtreinish]

This LGTM and I think it's ready to merge except for the missing assertion in the docstring example. There is an open question which I'm fine either way on which is whether the filter function and coloring functions are passed node weights or node ids, I can go either way so I'm curious your thoughts on it.

[ElePT]

I have tried out the suggestion of using ids instead of weights in c256e27, the only impact I noticed on the user side was the need to switch from one shared function for all tests to per-test closures in the unit tests, because a reference to the graph is needed to get the weights from the node/edge ids. I actually think this makes the tests more readable. I had hoped to be able to get rid of the result annotations, but the error type still needs to be determined, shame.

[mtreinish]

This LGTM now thanks for the updates. I just left some whitespace nits in the docstring. Also I think we can drop one trait bound.

[mtreinish]

Is this used anywhere?