Graph6 DiGaph6 Sparse6 support by hsunwenfang · Pull Request #1500 · Qiskit/rustworkx

hsunwenfang · 2025-09-06T08:25:04Z

https://github.com/Qiskit/rustworkx/issues/1496

I ran rustfmt locally
I have added the tests to cover my changes.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.

git push

…commit 826a8e4 baseline)

…er consolidation

CLAassistant · 2025-09-06T08:25:10Z

All committers have signed the CLA.

CLAassistant · 2025-09-06T08:25:10Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

IvanIsCoding

I left many comments:

We control all the code, please don't panic! or unwrap. Return Result<T, E>.
Python tests live in tests/. We only use #[cfg(test)] in rustworkx-core. Either migrate the tests or remove them
For universal functions, please use _rustworkx_dispatch. We don't need Python modules for graph6, digraph6, and sparse6.

Please take a look at https://github.com/Qiskit/rustworkx/blob/main/CONTRIBUTING.md, especially for formatting, tests, and adding type annotations.

IvanIsCoding · 2025-09-07T17:10:36Z

tests/test_graph6.py

+
+    def test_read_graph6_file(self):
+        """Test reading a graph from a graph6 file."""
+        with tempfile.NamedTemporaryFile(mode="w", delete=False) as fd:


I don't see why the whole test couldn't exist within the with statement. I'd rather not have a os.remove statement and let tempfile deal with it

Example:

rustworkx/tests/digraph/test_node_link_json.py

Line 116 in 7318a80

with tempfile.NamedTemporaryFile() as fd:

Use tempfile now

IvanIsCoding · 2025-09-07T17:11:29Z

tests/test_graph6.py

+                content = f.read()
+            self.assertEqual(content, "C~")
+        finally:
+            os.remove(path)


Same comment for all occurrences

IvanIsCoding · 2025-09-07T17:12:52Z

tests/test_graph6.py

+    def test_write_and_read_triangle(self):
+        g = rx.PyGraph()
+        g.add_nodes_from([None, None, None])
+        g.add_edges_from([(0, 1, None), (1, 2, None), (0, 2, None)])


Use https://www.rustworkx.org/dev/apiref/rustworkx.generators.cycle_graph.html#rustworkx.generators.cycle_graph

or the directed equivalent.

https://www.rustworkx.org/dev/apiref/rustworkx.generators.complete_graph.html works as well for n=3

IvanIsCoding · 2025-09-07T17:13:39Z

tests/test_graph6.py

+        with self.assertRaises(rx.Graph6ParseError):
+            rx.parse_graph6_size(bad_hdr)
+
+    def test_overflow(self):


A comment explaining this test would be great, especially bitwise manipulations

IvanIsCoding · 2025-09-07T17:14:28Z

tests/test_sparse6.py

+        self.assertEqual(g2.num_edges(), g.num_edges())
+
+
+if __name__ == '__main__':  # pragma: no cover


this is not necessary, you can remove it from all files. we use stestr python -m unittest also handles it AFAIK in our release

IvanIsCoding · 2025-09-07T17:29:22Z

releasenotes/notes/add-graph6-support.yaml

@@ -0,0 +1,48 @@
+features:


Please read https://docs.openstack.org/reno/latest/user/usage.html#editing-a-release-note for the correct format of release notes

used reno to generate one with nox -e docs compatible doc

IvanIsCoding · 2025-09-07T17:31:07Z

src/graph6.rs

+        DiGraph(DiGraph),
+    }
+
+    let wrapped = std::panic::catch_unwind(|| {


Do not use std::panic::catch_unwind. If you need to return an error use, Result as a return type with Err. Not panic! or unwrap.

This will likely break with WASM.

src/digraph6.rs

IvanIsCoding · 2025-09-07T17:38:57Z

rustworkx/digraph6.py

@@ -0,0 +1,30 @@
+"""digraph6 format helpers.


Right now, this wrapper is pointless:

For writing, see

rustworkx/rustworkx/__init__.py

Line 2285 in 7318a80

def write_graphml(graph, path, /, keys=None, compression=None):

for the correct pattern

For reading, if the file format contains the hint: use the same pattern from

rustworkx/src/graphml.rs

Line 1248 in 7318a80

pub fn read_graphml<'py>(

. If it doesn't, then just have two functions digraph_read_graph6 and graph_read_graph6

Removed graph6.py, digraph6.py, sparse6.py and use a write_graph6 function in init with _rustworkx_dispatch

IvanIsCoding · 2025-09-07T17:41:09Z

src/digraph6.rs

+
+#[pyfunction]
+#[pyo3(signature=(digraph, path))]
+pub fn digraph_write_graph6_file(digraph: Py<crate::digraph::PyDiGraph>, path: &str) -> PyResult<()> {


Just call the methods that write to file *_write_graph6, *_write_sparse6, etc. For methods that write to string, use to_str or from_str.

The naming is now graph_write_graph6 digraph_write_graph6

hsunwenfang · 2025-09-24T09:18:06Z

Thanks @IvanIsCoding for all the suggestion
Please feel free to point out any further things~

hsunwenfang added 12 commits September 1, 2025 20:25

feat: Add support for graph6 format

ac1afb8

git commit -m "chore: trigger self-hosted CI"

2a09e57

git push

remove gh action yaml

c6e5e7c

tidy up tests/test_graph6_py.py

826a8e4

Remove noxfile.py before PR as requested

efab5f1

restore: revert graph6.rs to pre-refactor monolithic implementation (…

d6ed94f

…commit 826a8e4 baseline)

pytests passed

f47a0ff

remove excessive test files

1f00fd1

release note

d9a2c6b

clean up tests

42b158b

delete unused github action yaml

f8241fe

tests: remove duplicate graph6 and sparse6 size/format test files aft…

d6be2f2

…er consolidation

IvanIsCoding reviewed Sep 7, 2025

View reviewed changes

hsunwenfang added 11 commits September 22, 2025 14:38

removed unwrap and panic

ffd809d

update naming

a5c6a90

update naming

ed32f8a

tidyup python

5bd2a65

Remove let wrapped = std::panic::catch_unwind(|| {

8a2ae09

fix the namings

84f202c

use tempfile.NamedTemporaryFile()

cd3d88f

Use generator to help tests

6ecd347

centralize byte string checking in graph6.rs/ SizeCodec

886b893

i/o naming align to @_rustworkx_dispatch

aca2137

cargo fmt, nox test, nox -e docs

d8b13d1

		self.assertEqual(g2.num_edges(), g.num_edges())


		if __name__ == '__main__': # pragma: no cover

Conversation

hsunwenfang commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Sep 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Sep 6, 2025

Uh oh!

IvanIsCoding left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hsunwenfang commented Sep 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hsunwenfang commented Sep 6, 2025 •

edited

Loading

CLAassistant commented Sep 6, 2025 •

edited

Loading

IvanIsCoding left a comment •

edited

Loading