Skip to content

Conversation

mtreinish
Copy link
Member

This commit adds the missing functionality to parse node link json and generate rustworkx graph objects from it. This adds two new functions parse_node_link_json_str() to parse a node link json string and parse_node_link_json_file() to parse a node link json file from a path.

Partial #840

This commit adds the missing functionality to parse node link json and
generate rustworkx graph objects from it. This adds two new functions
parse_node_link_json_str() to parse a node link json string and
parse_node_link_json_file() to parse a node link json file from a path.

Partial Qiskit#840
@coveralls
Copy link

coveralls commented Feb 16, 2024

Pull Request Test Coverage Report for Build 9620134374

Details

  • 117 of 129 (90.7%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 95.88%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/json/node_link_data.rs 27 30 90.0%
src/json/mod.rs 84 93 90.32%
Totals Coverage Status
Change from base Build 9620123262: 0.007%
Covered Lines: 17592
Relevant Lines: 18348

💛 - Coveralls

@mtreinish mtreinish added this to the 0.15.0 milestone Feb 21, 2024
Copy link
Collaborator

@IvanIsCoding IvanIsCoding left a comment

Choose a reason for hiding this comment

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

I think this is a great addition. The code is very succint and I think we leverage serde well.

My main concern is with the function naming. Maybe we could have rustworkx.parse_node_link_json for the string case and rustworkx.node_link_json_from_file for the file case? I just found parse_node_link_json_str a name that leaves a bad taste in the mouth

I also left a comment about the NetworkX round-trip test. If the test passes, I wonder how fast it will be compared to the current NetworkX.

@mtreinish mtreinish requested a review from IvanIsCoding June 21, 2024 21:42
Copy link
Collaborator

@IvanIsCoding IvanIsCoding left a comment

Choose a reason for hiding this comment

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

LGTM, I am glad we can parse the files with NetworkX and vice-versa

@IvanIsCoding IvanIsCoding added the automerge Queue a approved PR for merging label Jun 21, 2024
@mtreinish mtreinish merged commit a916638 into Qiskit:main Jun 21, 2024
@mtreinish mtreinish deleted the node-link-json-parse branch June 21, 2024 22:11
thierry-martinez added a commit to thierry-martinez/rustworkx that referenced this pull request Jun 9, 2025
This commit adds a GraphML serializer:

```python
def write_graphml(
    graphs: list[PyGraph | PyDiGraph],
    keys: list[tuple[str, Domain, str, Type, Any]],
    path: str,
    /,
    compression: str | None = ...,
) -> None: ...
```

`keys` is a list of tuples: id, domain, name of the key, type, and
default value. This commit also introduces the
`read_graphml_with_keys` function, which returns the key definitions
in the same format, along with the list of parsed graphs.

The implementation preserves the ids of graphs, nodes, and edges when
possible. If some ids conflict, fresh ids are generated in the written
GraphML file. The `read_graphml` function has also been updated to
store the graph id in the graph attributes, just like node and edge
ids are stored in the corresponding attributes.

The `write_graphml` function supports gzip compression, as does
`read_graphml`.

Note that the JSON node-link serializer (the other part of Qiskit#840) was
already implemented in Qiskit#1091.

Compared to Qiskit#1462:

- Keys are passed explicitly instead of being inferred (which allows
to use the types `float` and `int`, and to use default values);

- Attributes for graphs, nodes, and edges are taken from the weight of elements, instead of relying on callbacks. This allows write_graphml to act as a proper reciprocal of read_graphml. Round-trip tests have been added.

- IDs are taken from attributes when possible, instead of being generated from indices.

- Multiple graphs can be written to the same file.

- Gzip compression is supported.

- Tests have been added.

Regarding @IvanIsCoding's
comment (Qiskit#1462 (comment)),
about using https://github.com/jonasbb/petgraph-graphml:

- Rustworkx's `graphml.rs` introduces an internal `Graph` data
structure, which is used for `read_graphml`. It is natural to
have `write_graphml` rely on the same data structure.

- `petgraph-graphml` generates ids from indices, which prevents us
from preserving ids accross the `read_graphml`/`write_graphml` round
trip.
github-merge-queue bot pushed a commit that referenced this pull request Jun 17, 2025
* Fix #840: Add GraphML serializer

This commit adds a GraphML serializer:

```python
def write_graphml(
    graphs: list[PyGraph | PyDiGraph],
    keys: list[tuple[str, Domain, str, Type, Any]],
    path: str,
    /,
    compression: str | None = ...,
) -> None: ...
```

`keys` is a list of tuples: id, domain, name of the key, type, and
default value. This commit also introduces the
`read_graphml_with_keys` function, which returns the key definitions
in the same format, along with the list of parsed graphs.

The implementation preserves the ids of graphs, nodes, and edges when
possible. If some ids conflict, fresh ids are generated in the written
GraphML file. The `read_graphml` function has also been updated to
store the graph id in the graph attributes, just like node and edge
ids are stored in the corresponding attributes.

The `write_graphml` function supports gzip compression, as does
`read_graphml`.

Note that the JSON node-link serializer (the other part of #840) was
already implemented in #1091.

Compared to #1462:

- Keys are passed explicitly instead of being inferred (which allows
to use the types `float` and `int`, and to use default values);

- Attributes for graphs, nodes, and edges are taken from the weight of elements, instead of relying on callbacks. This allows write_graphml to act as a proper reciprocal of read_graphml. Round-trip tests have been added.

- IDs are taken from attributes when possible, instead of being generated from indices.

- Multiple graphs can be written to the same file.

- Gzip compression is supported.

- Tests have been added.

Regarding @IvanIsCoding's
comment (#1462 (comment)),
about using https://github.com/jonasbb/petgraph-graphml:

- Rustworkx's `graphml.rs` introduces an internal `Graph` data
structure, which is used for `read_graphml`. It is natural to
have `write_graphml` rely on the same data structure.

- `petgraph-graphml` generates ids from indices, which prevents us
from preserving ids accross the `read_graphml`/`write_graphml` round
trip.

* Fix clippy comments

* Prefix types with GraphML

Suggested by @IvanIsCoding: #1464 (comment)

* Black

* Add release notes

* Fix stubs error

* Add documentation

* Remove read_graphml_with_keys / write_graphml for single graph only

* Use `DictMap` everywhere

Suggested by @IvanIsCoding: #1464 (comment)

* rustfmt and clippy

* Remove unused math module (ruff check)

* Use `from __future__ import annotations` for Python <3.10

* Add stub for `write_graphml`

* Remove `read_graphml_with_keys` from documentation

* Apply suggestions from code review

* Update rustworkx/__init__.py

* Apply suggestions from code review

---------

Co-authored-by: Ivan Carvalho <8753214+IvanIsCoding@users.noreply.github.com>
SILIZ4 pushed a commit to SILIZ4/rustworkx that referenced this pull request Jul 4, 2025
* Add functions to parse node link JSON

This commit adds the missing functionality to parse node link json and
generate rustworkx graph objects from it. This adds two new functions
parse_node_link_json_str() to parse a node link json string and
parse_node_link_json_file() to parse a node link json file from a path.

Partial Qiskit#840

* Fix lint

* Add negative tests and fix deserialization error class

* Add missing stubs

* Rename functions

* Fix interop with loading non-rustworkx payloads
SILIZ4 pushed a commit to SILIZ4/rustworkx that referenced this pull request Jul 4, 2025
* Fix Qiskit#840: Add GraphML serializer

This commit adds a GraphML serializer:

```python
def write_graphml(
    graphs: list[PyGraph | PyDiGraph],
    keys: list[tuple[str, Domain, str, Type, Any]],
    path: str,
    /,
    compression: str | None = ...,
) -> None: ...
```

`keys` is a list of tuples: id, domain, name of the key, type, and
default value. This commit also introduces the
`read_graphml_with_keys` function, which returns the key definitions
in the same format, along with the list of parsed graphs.

The implementation preserves the ids of graphs, nodes, and edges when
possible. If some ids conflict, fresh ids are generated in the written
GraphML file. The `read_graphml` function has also been updated to
store the graph id in the graph attributes, just like node and edge
ids are stored in the corresponding attributes.

The `write_graphml` function supports gzip compression, as does
`read_graphml`.

Note that the JSON node-link serializer (the other part of Qiskit#840) was
already implemented in Qiskit#1091.

Compared to Qiskit#1462:

- Keys are passed explicitly instead of being inferred (which allows
to use the types `float` and `int`, and to use default values);

- Attributes for graphs, nodes, and edges are taken from the weight of elements, instead of relying on callbacks. This allows write_graphml to act as a proper reciprocal of read_graphml. Round-trip tests have been added.

- IDs are taken from attributes when possible, instead of being generated from indices.

- Multiple graphs can be written to the same file.

- Gzip compression is supported.

- Tests have been added.

Regarding @IvanIsCoding's
comment (Qiskit#1462 (comment)),
about using https://github.com/jonasbb/petgraph-graphml:

- Rustworkx's `graphml.rs` introduces an internal `Graph` data
structure, which is used for `read_graphml`. It is natural to
have `write_graphml` rely on the same data structure.

- `petgraph-graphml` generates ids from indices, which prevents us
from preserving ids accross the `read_graphml`/`write_graphml` round
trip.

* Fix clippy comments

* Prefix types with GraphML

Suggested by @IvanIsCoding: Qiskit#1464 (comment)

* Black

* Add release notes

* Fix stubs error

* Add documentation

* Remove read_graphml_with_keys / write_graphml for single graph only

* Use `DictMap` everywhere

Suggested by @IvanIsCoding: Qiskit#1464 (comment)

* rustfmt and clippy

* Remove unused math module (ruff check)

* Use `from __future__ import annotations` for Python <3.10

* Add stub for `write_graphml`

* Remove `read_graphml_with_keys` from documentation

* Apply suggestions from code review

* Update rustworkx/__init__.py

* Apply suggestions from code review

---------

Co-authored-by: Ivan Carvalho <8753214+IvanIsCoding@users.noreply.github.com>
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