-
Notifications
You must be signed in to change notification settings - Fork 193
Add functions to parse node link JSON #1091
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
Conversation
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
Pull Request Test Coverage Report for Build 9620134374Details
💛 - Coveralls |
There was a problem hiding this 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.
There was a problem hiding this 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
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 #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>
* 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
* 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>
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