rustworkx icon indicating copy to clipboard operation
rustworkx copied to clipboard

Published 20 hours ago • verified publisher icon Qiskit

Move more connected components functions to retworkx-core

Open kris524 opened this issue 2 years ago • 11 comments

kris524 avatar Jul 03 '22 22:07 kris524

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 03 '22 22:07 CLAassistant

Hello, I get this error no method named 'index' found for reference &<G as GraphBase>::NodeId and I am not sure what other method is allowed. If anyone has any suggestions or has an idea about which files to look at for reference/examples let me know, otherwise I will keep looking.

I am also not sure how to import NullGraph and InvalidNode in the conn_components.rs file.

kris524 avatar Aug 25 '22 07:08 kris524

I was planning on doing a review of this in the next couple of days. The v.index() issue is that petgraph using Graph , or the derived PyGraph, is different from using the GraphBase trait as it is in rustworkx-core. For instance, NodeIndex for Graph is NodeId for GraphBase. To get the index with GraphBase, you would do graph.to_index(v). You can find to_index here, https://docs.rs/petgraph/0.4.13/petgraph/visit/trait.NodeIndexable.html#tymethod.to_index.

enavarro51 avatar Aug 26 '22 14:08 enavarro51

Oh one more important thing I forgot. You need to transition to using the rustworkx name. Steps are

  • Rename your repository on Github.
  • Rename your local directory to rustworkx.
  • Change any remote urls to the rustworkx repository.
  • Checkout main on your local.
  • Do a git pull upstream main
  • Move any files you have changed in retworkx-core to the corresponding location in rustworkx-core. I think this will just be in connectivity for conn_components.rs and mod.rs.
  • Once you have moved the files over, you can remove the retworkx-core directory.
  • Finally globally replace references to retworkx in all your changed files to rustworkx.

enavarro51 avatar Sep 05 '22 16:09 enavarro51

Apologies for the late implementation of the feedback. I have started making the corresponding naming changes and will commit any relevant changes. Thank you for the help!

kris524 avatar Sep 06 '22 18:09 kris524

@IvanIsCoding or @mtreinish Can you turn on the workflow testing here? Thanks.

enavarro51 avatar Sep 07 '22 19:09 enavarro51

Hi, @kris524. Just checking to see if you are still working on this. Thanks.

enavarro51 avatar Oct 11 '22 17:10 enavarro51

Hi @enavarro51 apologies for being slow, I was just busy. Is it ok if I have a look at it this weekend? I think I am close to finishing it.

kris524 avatar Oct 11 '22 21:10 kris524

@kris524. That's fine. Once you get a good build, you can add a test for each of the 2 new functions at the end of rustworkx/rustworkx-core/src/connectivity/conn_components.rs, and add a features release note like rustworkx/releasenotes/notes/move-conn_components-functions-1be43732ec6058ea.yaml. Let me know if you have any questions.

enavarro51 avatar Oct 11 '22 23:10 enavarro51

Tomorrow I will write a test for node_connected_component

kris524 avatar Nov 16 '22 00:11 kris524

@enavarro51 Let me know if you are happy with the tests.

kris524 avatar Nov 18 '22 18:11 kris524