From 1bd168a605c2a0fa5e887c3e78bd37844b307709 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Mon, 8 May 2023 14:32:11 -0600 Subject: [PATCH 1/6] Fix docs to work with Sphinx Theme 1.11 (#867) * Fix docs to work with Sphinx Theme 1.11 * Update docs/source/_templates/sidebar.html Co-authored-by: Matthew Treinish --- docs/source/_templates/layout.html | 300 +++++++++------------------ docs/source/_templates/page.html | 14 -- docs/source/_templates/sidebar.html | 99 +++++++++ docs/source/_templates/versions.html | 25 --- docs/source/conf.py | 12 +- docs/source/requirements.txt | 3 +- 6 files changed, 205 insertions(+), 248 deletions(-) delete mode 100644 docs/source/_templates/page.html create mode 100644 docs/source/_templates/sidebar.html delete mode 100644 docs/source/_templates/versions.html diff --git a/docs/source/_templates/layout.html b/docs/source/_templates/layout.html index 102d6029ed..515164542d 100644 --- a/docs/source/_templates/layout.html +++ b/docs/source/_templates/layout.html @@ -1,48 +1,25 @@ -{# TEMPLATE VAR SETTINGS #} -{%- set url_root = pathto('', 1) %} -{%- if url_root == '#' %}{% set url_root = '' %}{% endif %} +{# Sphinx template variable setup #} {%- if not embedded and docstitle %} {%- set titlesuffix = " — "|safe + docstitle|e %} {%- else %} {%- set titlesuffix = "" %} {%- endif %} -{%- set lang_attr = 'en' if language == None else (language | replace('_', '-')) %} -{% import 'theme_variables.jinja' as theme_variables %} +{%- set lang_attr = 'en' if language == None else (language | replace('_', '-')) -%} - - + - - {{ metatags }} + - {% block htmltitle %} + {{ metatags }} + {%- block htmltitle %} {{ title|striptags|e }}{{ titlesuffix }} - {% endblock %} - - {# FAVICON #} - {% if favicon %} - - {% endif %} - {# CANONICAL URL #} - {% if theme_canonical_url %} - - {% endif %} - - {# CSS #} - - {# OPENSEARCH #} - {% if not embedded %} - {% if use_opensearch %} - - {% endif %} - - {% endif %} + {%- endblock %} + {%- if favicon_url %} + + {%- endif %} - - + {#- CSS #} {%- for css in css_files %} {%- if css|attr("rel") %} @@ -51,124 +28,108 @@ {%- endif %} {%- endfor %} {%- for cssfile in extra_css_files %} - - {%- endfor %} - - {%- block linktags %} - {%- if hasdoc('about') %} - - {%- endif %} - {%- if hasdoc('genindex') %} - - {%- endif %} - {%- if hasdoc('search') %} - - {%- endif %} - {%- if hasdoc('copyright') %} - - {%- endif %} - {%- if next %} - - {%- endif %} - {%- if prev %} - - {%- endif %} - {%- endblock %} - {%- block extrahead %} {% endblock %} - - {# Keep modernizr in head - http://modernizr.com/docs/#installing #} - + + {%- endfor -%} + + + {%- if analytics_enabled %} + + + + {%- endif -%} - - {% block extrabody %} {% endblock %} - {# SIDE NAV, TOGGLES ON MOBILE #} + - {% include "versions.html" %} + + {% include "languages.html" %} +
Table of Contents
- + + {% include "sidebar.html" %}
+ +
{% include "breadcrumbs.html" %}
- -
- Shortcuts -
+
-
+
{%- block content %} - {% if theme_style_external_links|tobool %} -
- {% else %}
- {% endif %}
{%- block document %}
{% block body %}{% endblock %}
- {% if self.comments()|trim %} -
- {% block comments %}{% endblock %} -
- {% endif%}
{%- endblock %} {% include "footer.html" %} @@ -176,6 +137,7 @@ {%- endblock %}
+
@@ -186,93 +148,32 @@
- {% if not embedded %} +{%- block footer %} {% endblock %} - {% if sphinx_version >= "1.8.0" %} - - {%- for scriptfile in script_files %} - {{ js_tag(scriptfile) }} - {%- endfor %} - {% else %} - - {%- for scriptfile in script_files %} - - {%- endfor %} - {% endif %} + + + {%- for scriptfile in script_files %} + {{ js_tag(scriptfile) }} + {%- endfor %} - {% endif %} + + + + - - - - + + -{%- block footer %} {% endblock %} - -
-
-
- - - -
-
-
-
- - -
-
-
- -
-
- -
-
-
- - - - - + diff --git a/docs/source/_templates/page.html b/docs/source/_templates/page.html deleted file mode 100644 index 429a7dedd9..0000000000 --- a/docs/source/_templates/page.html +++ /dev/null @@ -1,14 +0,0 @@ -{% extends "!page.html" %} - -{% block footer %} - -{% endblock %} \ No newline at end of file diff --git a/docs/source/_templates/sidebar.html b/docs/source/_templates/sidebar.html new file mode 100644 index 0000000000..13c36ae8a7 --- /dev/null +++ b/docs/source/_templates/sidebar.html @@ -0,0 +1,99 @@ + + + +{% if expandable_sidebar %} + +{% endif %} + + + \ No newline at end of file diff --git a/docs/source/_templates/versions.html b/docs/source/_templates/versions.html deleted file mode 100644 index 7e445164d0..0000000000 --- a/docs/source/_templates/versions.html +++ /dev/null @@ -1,25 +0,0 @@ -
- - {{ version_label }} - - -
-
-
Versions
-
Current Release
-
Development
-
Previous Releases
- {% for version in version_list %} -
{{ version }}
- {% endfor %} -
-
- -
diff --git a/docs/source/conf.py b/docs/source/conf.py index be034aca75..23faf0b940 100644 --- a/docs/source/conf.py +++ b/docs/source/conf.py @@ -19,8 +19,8 @@ # General configuration: -project = u'rustworkx' -copyright = u'2021, rustworkx Contributors' +project = 'rustworkx' +copyright = '2021, rustworkx Contributors' # The short X.Y version. @@ -40,6 +40,7 @@ 'sphinx.ext.intersphinx', 'sphinxemoji.sphinxemoji', 'sphinx_reredirects', + 'qiskit_sphinx_theme', ] html_static_path = ['_static'] templates_path = ['_templates'] @@ -86,30 +87,25 @@ .. note:: - This is the documnetation for the current state of the development branch + This is the documentation for the current state of the development branch of rustworkx. The documentation or APIs here can change prior to being released. """ # HTML Output Options - html_theme = 'qiskit_sphinx_theme' - html_theme_options = { 'logo_only': False, 'display_version': True, 'prev_next_buttons_location': 'bottom', 'style_external_links': True, } - htmlhelp_basename = 'rustworkx' # Latex options - latex_elements = {} - latex_documents = [ ('index', 'rustworkx.tex', u'rustworkx Documentation', u'rustworkx Contributors', 'manual'), diff --git a/docs/source/requirements.txt b/docs/source/requirements.txt index a960d28832..57816784a6 100644 --- a/docs/source/requirements.txt +++ b/docs/source/requirements.txt @@ -1,11 +1,10 @@ m2r2 sphinx>=3.0.0 -sphinx_rtd_theme jupyter-sphinx pydot pillow>=4.2.1 reno>=3.4.0 -qiskit-sphinx-theme>=1.7 +qiskit-sphinx-theme~=1.11.1 matplotlib>=3.4 sphinx-reredirects sphinxemoji From 5a3f9b386b17728fd235982ddcf1e44d5f1b596d Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Mon, 8 May 2023 15:16:17 -0600 Subject: [PATCH 2/6] Turn off CI for forks (#868) Co-authored-by: Matthew Treinish --- .github/workflows/docs_dev.yml | 1 + .github/workflows/main.yml | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/.github/workflows/docs_dev.yml b/.github/workflows/docs_dev.yml index 9e1c932047..c55b0aab84 100644 --- a/.github/workflows/docs_dev.yml +++ b/.github/workflows/docs_dev.yml @@ -5,6 +5,7 @@ on: jobs: deploy: + if: github.repository_owner == 'Qiskit' runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 4f18920f29..56db029261 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -10,6 +10,7 @@ concurrency: cancel-in-progress: true jobs: build_lint: + if: github.repository_owner == 'Qiskit' name: Build, rustfmt, and python lint runs-on: ubuntu-latest steps: @@ -49,6 +50,7 @@ jobs: name: rustworkx_core_docs path: target/doc/rustworkx_core tests: + if: github.repository_owner == 'Qiskit' needs: [build_lint] name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} ${{ matrix.msrv }} runs-on: ${{ matrix.platform.os }} @@ -87,6 +89,7 @@ jobs: - name: 'Run tests' run: tox -epy tests_stubs: + if: github.repository_owner == 'Qiskit' needs: [tests] name: python-stubs-${{ matrix.python-version }} runs-on: ubuntu-latest @@ -107,6 +110,7 @@ jobs: - name: 'Run rustworkx stub tests' run: tox -estubs tests_retworkx_compat: + if: github.repository_owner == 'Qiskit' needs: [build_lint] name: python${{ matrix.python-version }}-${{ matrix.platform.python-architecture }} ${{ matrix.platform.os }} ${{ matrix.msrv }} runs-on: ${{ matrix.platform.os }} @@ -147,6 +151,7 @@ jobs: cd tests stestr run -t ./retworkx_backwards_compat coverage: + if: github.repository_owner == 'Qiskit' needs: [tests] name: Coverage runs-on: ubuntu-latest @@ -189,6 +194,7 @@ jobs: github-token: ${{ secrets.GITHUB_TOKEN }} path-to-lcov: coveralls.info docs: + if: github.repository_owner == 'Qiskit' needs: [tests] name: Build Docs runs-on: ubuntu-latest From 8686896a9668c61e8b3468836a8552cde2a7dba5 Mon Sep 17 00:00:00 2001 From: Binh Vu Date: Tue, 9 May 2023 12:54:16 -0700 Subject: [PATCH 3/6] Fix pickle/deepcopy not preserve original edge indices (#589) * fix issue #585 that pickling graph & digraph do not preserve original edge index * fix clippy lints - collapsible_else_if * Simplify logic in __setstate__ * Add release note * Fix lint --------- Co-authored-by: Matthew Treinish --- ...-edge-indices-pickle-83fddf149441fa9f.yaml | 10 + src/digraph.rs | 231 +++++++++++++----- src/graph.rs | 223 ++++++++++++----- tests/rustworkx_tests/digraph/test_pickle.py | 41 ++++ tests/rustworkx_tests/graph/test_pickle.py | 41 ++++ 5 files changed, 418 insertions(+), 128 deletions(-) create mode 100644 releasenotes/notes/fix-edge-indices-pickle-83fddf149441fa9f.yaml create mode 100644 tests/rustworkx_tests/digraph/test_pickle.py create mode 100644 tests/rustworkx_tests/graph/test_pickle.py diff --git a/releasenotes/notes/fix-edge-indices-pickle-83fddf149441fa9f.yaml b/releasenotes/notes/fix-edge-indices-pickle-83fddf149441fa9f.yaml new file mode 100644 index 0000000000..238fe3cf62 --- /dev/null +++ b/releasenotes/notes/fix-edge-indices-pickle-83fddf149441fa9f.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixed an issue when using ``copy.deepcopy()`` on :class:`~.PyDiGraph` and + :class:`~.PyGraph` objects when there were removed edges from the graph + object. Previously, if there were any holes in the edge indices caused by + the removal the output copy of the graph object would incorrectly have + flatten the indices. This has been corrected so that the edge indices are + recreated exactly after a ``deepcopy()``. + Fixed `#585 `__ diff --git a/src/digraph.rs b/src/digraph.rs index 9339869fca..ba81cbae5f 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -43,7 +43,7 @@ use petgraph::graph::{EdgeIndex, NodeIndex}; use petgraph::prelude::*; use petgraph::visit::{ - GraphBase, IntoEdgeReferences, IntoNodeReferences, NodeCount, NodeFiltered, NodeIndexable, + EdgeIndexable, GraphBase, IntoEdgeReferences, IntoNodeReferences, NodeCount, NodeFiltered, Visitable, }; @@ -298,97 +298,196 @@ impl PyDiGraph { } fn __getstate__(&self, py: Python) -> PyResult { + let mut nodes: Vec = Vec::with_capacity(self.graph.node_count()); + let mut edges: Vec = Vec::with_capacity(self.graph.edge_bound()); + + // save nodes to a list along with its index + for node_idx in self.graph.node_indices() { + let node_data = self.graph.node_weight(node_idx).unwrap(); + nodes.push((node_idx.index(), node_data).to_object(py)); + } + + // edges are saved with none (deleted edges) instead of their index to save space + for i in 0..self.graph.edge_bound() { + let idx = EdgeIndex::new(i); + let edge = match self.graph.edge_weight(idx) { + Some(edge_w) => { + let endpoints = self.graph.edge_endpoints(idx).unwrap(); + (endpoints.0.index(), endpoints.1.index(), edge_w).to_object(py) + } + None => py.None(), + }; + edges.push(edge); + } + let out_dict = PyDict::new(py); - let node_dict = PyDict::new(py); - let mut out_list: Vec = Vec::with_capacity(self.graph.edge_count()); - out_dict.set_item("nodes", node_dict)?; + let nodes_lst: PyObject = PyList::new(py, nodes).into(); + let edges_lst: PyObject = PyList::new(py, edges).into(); + out_dict.set_item("nodes", nodes_lst)?; + out_dict.set_item("edges", edges_lst)?; out_dict.set_item("nodes_removed", self.node_removed)?; out_dict.set_item("multigraph", self.multigraph)?; out_dict.set_item("attrs", self.attrs.clone_ref(py))?; out_dict.set_item("check_cycle", self.check_cycle)?; - let dir = petgraph::Direction::Incoming; - for node_index in self.graph.node_indices() { - let node_data = self.graph.node_weight(node_index).unwrap(); - node_dict.set_item(node_index.index(), node_data)?; - for edge in self.graph.edges_directed(node_index, dir) { - let edge_w = edge.weight(); - let triplet = (edge.source().index(), edge.target().index(), edge_w).to_object(py); - out_list.push(triplet); - } - } - let py_out_list: PyObject = PyList::new(py, out_list).into(); - out_dict.set_item("edges", py_out_list)?; Ok(out_dict.into()) } fn __setstate__(&mut self, py: Python, state: PyObject) -> PyResult<()> { + let dict_state = state.downcast::(py)?; + let nodes_lst = dict_state.get_item("nodes").unwrap().downcast::()?; + let edges_lst = dict_state.get_item("edges").unwrap().downcast::()?; self.graph = StablePyGraph::::new(); let dict_state = state.downcast::(py)?; - - let nodes_dict = dict_state.get_item("nodes").unwrap().downcast::()?; - let edges_list = dict_state.get_item("edges").unwrap().downcast::()?; - let nodes_removed_raw = dict_state - .get_item("nodes_removed") - .unwrap() - .downcast::()?; - self.node_removed = nodes_removed_raw.extract()?; - let multigraph_raw = dict_state + self.multigraph = dict_state .get_item("multigraph") .unwrap() - .downcast::()?; - self.multigraph = multigraph_raw.extract()?; + .downcast::()? + .extract()?; + self.node_removed = dict_state + .get_item("nodes_removed") + .unwrap() + .downcast::()? + .extract()?; let attrs = match dict_state.get_item("attrs") { Some(attr) => attr.into(), None => py.None(), }; self.attrs = attrs; - let check_cycle_raw = dict_state + self.check_cycle = dict_state .get_item("check_cycle") .unwrap() - .downcast::()?; - self.check_cycle = check_cycle_raw.extract()?; - let mut node_indices: Vec = Vec::new(); - for raw_index in nodes_dict.keys() { - let tmp_index = raw_index.downcast::()?; - node_indices.push(tmp_index.extract()?); - } - if node_indices.is_empty() { + .downcast::()? + .extract()?; + + // graph is empty, stop early + if nodes_lst.is_empty() { return Ok(()); } - let max_index: usize = *node_indices.iter().max().unwrap(); - if max_index + 1 != node_indices.len() { - self.node_removed = true; - } - let mut tmp_nodes: Vec = Vec::new(); - let mut node_count: usize = 0; - while max_index >= self.graph.node_bound() { - match nodes_dict.get_item(node_count) { - Some(raw_data) => { - self.graph.add_node(raw_data.into()); - } - None => { + + if !self.node_removed { + for item in nodes_lst.iter() { + let node_w = item + .downcast::() + .unwrap() + .get_item(1) + .unwrap() + .extract() + .unwrap(); + self.graph.add_node(node_w); + } + } else if nodes_lst.len() == 1 { + // graph has only one node, handle logic here to save one if in the loop later + let item = nodes_lst + .get_item(0) + .unwrap() + .downcast::() + .unwrap(); + let node_idx: usize = item.get_item(0).unwrap().extract().unwrap(); + let node_w = item.get_item(1).unwrap().extract().unwrap(); + + for _i in 0..node_idx { + self.graph.add_node(py.None()); + } + self.graph.add_node(node_w); + for i in 0..node_idx { + self.graph.remove_node(NodeIndex::new(i)); + } + } else { + let last_item = nodes_lst + .get_item(nodes_lst.len() - 1) + .unwrap() + .downcast::() + .unwrap(); + + // use a pointer to iter the node list + let mut pointer = 0; + let mut next_node_idx: usize = nodes_lst + .get_item(pointer) + .unwrap() + .downcast::() + .unwrap() + .get_item(0) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + + // list of temporary nodes that will be removed later to re-create holes + let node_bound_1: usize = last_item.get_item(0).unwrap().extract().unwrap(); + let mut tmp_nodes: Vec = + Vec::with_capacity(node_bound_1 + 1 - nodes_lst.len()); + + for i in 0..nodes_lst.len() + 1 { + if i < next_node_idx { + // node does not exist let tmp_node = self.graph.add_node(py.None()); tmp_nodes.push(tmp_node); + } else { + // add node to the graph, and update the next available node index + let item = nodes_lst + .get_item(pointer) + .unwrap() + .downcast::() + .unwrap(); + + let node_w = item.get_item(1).unwrap().extract().unwrap(); + self.graph.add_node(node_w); + pointer += 1; + if pointer < nodes_lst.len() { + next_node_idx = nodes_lst + .get_item(pointer) + .unwrap() + .downcast::() + .unwrap() + .get_item(0) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + } } - }; - node_count += 1; - } - for tmp_node in tmp_nodes { - self.graph.remove_node(tmp_node); - } - for raw_edge in edges_list.iter() { - let edge = raw_edge.downcast::()?; - let raw_p_index = edge.get_item(0)?.downcast::()?; - let p_index: usize = raw_p_index.extract()?; - let raw_c_index = edge.get_item(1)?.downcast::()?; - let c_index: usize = raw_c_index.extract()?; - let edge_data = edge.get_item(2)?; - self.graph.add_edge( - NodeIndex::new(p_index), - NodeIndex::new(c_index), - edge_data.into(), - ); + } + // Remove any temporary nodes we added + for tmp_node in tmp_nodes { + self.graph.remove_node(tmp_node); + } } + + // to ensure O(1) on edge deletion, use a temporary node to store missing edges + let tmp_node = self.graph.add_node(py.None()); + + for item in edges_lst { + if item.is_none() { + // add a temporary edge that will be deleted later to re-create the hole + self.graph.add_edge(tmp_node, tmp_node, py.None()); + } else { + let triple = item.downcast::().unwrap(); + let edge_p: usize = triple + .get_item(0) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + let edge_c: usize = triple + .get_item(1) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + let edge_w = triple.get_item(2).unwrap().extract().unwrap(); + self.graph + .add_edge(NodeIndex::new(edge_p), NodeIndex::new(edge_c), edge_w); + } + } + + // remove the temporary node will remove all deleted edges in bulk, + // the cost is equal to the number of edges + self.graph.remove_node(tmp_node); + Ok(()) } diff --git a/src/graph.rs b/src/graph.rs index 3b0f2c43a4..7857cef9c8 100644 --- a/src/graph.rs +++ b/src/graph.rs @@ -46,7 +46,7 @@ use petgraph::algo; use petgraph::graph::{EdgeIndex, NodeIndex}; use petgraph::prelude::*; use petgraph::visit::{ - GraphBase, IntoEdgeReferences, IntoNodeReferences, NodeCount, NodeFiltered, NodeIndexable, + EdgeIndexable, GraphBase, IntoEdgeReferences, IntoNodeReferences, NodeCount, NodeFiltered, }; /// A class for creating undirected graphs @@ -192,88 +192,187 @@ impl PyGraph { } fn __getstate__(&self, py: Python) -> PyResult { + let mut nodes: Vec = Vec::with_capacity(self.graph.node_count()); + let mut edges: Vec = Vec::with_capacity(self.graph.edge_bound()); + + // save nodes to a list along with its index + for node_idx in self.graph.node_indices() { + let node_data = self.graph.node_weight(node_idx).unwrap(); + nodes.push((node_idx.index(), node_data).to_object(py)); + } + + // edges are saved with none (deleted edges) instead of their index to save space + for i in 0..self.graph.edge_bound() { + let idx = EdgeIndex::new(i); + let edge = match self.graph.edge_weight(idx) { + Some(edge_w) => { + let endpoints = self.graph.edge_endpoints(idx).unwrap(); + (endpoints.0.index(), endpoints.1.index(), edge_w).to_object(py) + } + None => py.None(), + }; + edges.push(edge); + } + let out_dict = PyDict::new(py); - let node_dict = PyDict::new(py); - let mut out_list: Vec = Vec::with_capacity(self.graph.edge_count()); - out_dict.set_item("nodes", node_dict)?; + let nodes_lst: PyObject = PyList::new(py, nodes).into(); + let edges_lst: PyObject = PyList::new(py, edges).into(); + out_dict.set_item("nodes", nodes_lst)?; + out_dict.set_item("edges", edges_lst)?; out_dict.set_item("nodes_removed", self.node_removed)?; out_dict.set_item("multigraph", self.multigraph)?; out_dict.set_item("attrs", self.attrs.clone_ref(py))?; - for node_index in self.graph.node_indices() { - let node_data = self.graph.node_weight(node_index).unwrap(); - node_dict.set_item(node_index.index(), node_data)?; - } - for edge in self.graph.edge_indices() { - let edge_w = self.graph.edge_weight(edge); - let endpoints = self.graph.edge_endpoints(edge).unwrap(); - - let triplet = (endpoints.0.index(), endpoints.1.index(), edge_w).to_object(py); - out_list.push(triplet); - } - let py_out_list: PyObject = PyList::new(py, out_list).into(); - out_dict.set_item("edges", py_out_list)?; Ok(out_dict.into()) } fn __setstate__(&mut self, py: Python, state: PyObject) -> PyResult<()> { - self.graph = StablePyGraph::::default(); let dict_state = state.downcast::(py)?; - let nodes_dict = dict_state.get_item("nodes").unwrap().downcast::()?; - let edges_list = dict_state.get_item("edges").unwrap().downcast::()?; - let nodes_removed_raw = dict_state - .get_item("nodes_removed") - .unwrap() - .downcast::()?; - self.node_removed = nodes_removed_raw.extract()?; - let multigraph_raw = dict_state + let nodes_lst = dict_state.get_item("nodes").unwrap().downcast::()?; + let edges_lst = dict_state.get_item("edges").unwrap().downcast::()?; + + self.graph = StablePyGraph::::default(); + self.multigraph = dict_state .get_item("multigraph") .unwrap() - .downcast::()?; - self.multigraph = multigraph_raw.extract()?; - let attrs = match dict_state.get_item("attrs") { + .downcast::()? + .extract()?; + self.node_removed = dict_state + .get_item("nodes_removed") + .unwrap() + .downcast::()? + .extract()?; + self.attrs = match dict_state.get_item("attrs") { Some(attr) => attr.into(), None => py.None(), }; - self.attrs = attrs; - - let mut node_indices: Vec = Vec::new(); - for raw_index in nodes_dict.keys() { - let tmp_index = raw_index.downcast::()?; - node_indices.push(tmp_index.extract()?); - } - if node_indices.is_empty() { + // graph is empty, stop early + if nodes_lst.is_empty() { return Ok(()); } - let max_index: usize = *node_indices.iter().max().unwrap(); - let mut tmp_nodes: Vec = Vec::new(); - let mut node_count: usize = 0; - while max_index >= self.graph.node_bound() { - match nodes_dict.get_item(node_count) { - Some(raw_data) => { - self.graph.add_node(raw_data.into()); - } - None => { + + if !self.node_removed { + for item in nodes_lst.iter() { + let node_w = item + .downcast::() + .unwrap() + .get_item(1) + .unwrap() + .extract() + .unwrap(); + self.graph.add_node(node_w); + } + } else if nodes_lst.len() == 1 { + // graph has only one node, handle logic here to save one if in the loop later + let item = nodes_lst + .get_item(0) + .unwrap() + .downcast::() + .unwrap(); + let node_idx: usize = item.get_item(0).unwrap().extract().unwrap(); + let node_w = item.get_item(1).unwrap().extract().unwrap(); + + for _i in 0..node_idx { + self.graph.add_node(py.None()); + } + self.graph.add_node(node_w); + for i in 0..node_idx { + self.graph.remove_node(NodeIndex::new(i)); + } + } else { + let last_item = nodes_lst + .get_item(nodes_lst.len() - 1) + .unwrap() + .downcast::() + .unwrap(); + + // use a pointer to iter the node list + let mut pointer = 0; + let mut next_node_idx: usize = nodes_lst + .get_item(pointer) + .unwrap() + .downcast::() + .unwrap() + .get_item(0) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + + // list of temporary nodes that will be removed later to re-create holes + let node_bound_1: usize = last_item.get_item(0).unwrap().extract().unwrap(); + let mut tmp_nodes: Vec = + Vec::with_capacity(node_bound_1 + 1 - nodes_lst.len()); + + for i in 0..nodes_lst.len() + 1 { + if i < next_node_idx { + // node does not exist let tmp_node = self.graph.add_node(py.None()); tmp_nodes.push(tmp_node); + } else { + // add node to the graph, and update the next available node index + let item = nodes_lst + .get_item(pointer) + .unwrap() + .downcast::() + .unwrap(); + + let node_w = item.get_item(1).unwrap().extract().unwrap(); + self.graph.add_node(node_w); + pointer += 1; + if pointer < nodes_lst.len() { + next_node_idx = nodes_lst + .get_item(pointer) + .unwrap() + .downcast::() + .unwrap() + .get_item(0) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + } } - }; - node_count += 1; - } - for tmp_node in tmp_nodes { - self.graph.remove_node(tmp_node); + } + for tmp_node in tmp_nodes { + self.graph.remove_node(tmp_node); + } } - for raw_edge in edges_list.iter() { - let edge = raw_edge.downcast::()?; - let raw_p_index = edge.get_item(0)?.downcast::()?; - let parent: usize = raw_p_index.extract()?; - let p_index = NodeIndex::new(parent); - let raw_c_index = edge.get_item(1)?.downcast::()?; - let child: usize = raw_c_index.extract()?; - let c_index = NodeIndex::new(child); - let edge_data = edge.get_item(2)?; - - self.graph.add_edge(p_index, c_index, edge_data.into()); + + // to ensure O(1) on edge deletion, use a temporary node to store missing edges + let tmp_node = self.graph.add_node(py.None()); + + for item in edges_lst { + if item.is_none() { + // add a temporary edge that will be deleted later to re-create the hole + self.graph.add_edge(tmp_node, tmp_node, py.None()); + } else { + let triple = item.downcast::().unwrap(); + let edge_p: usize = triple + .get_item(0) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + let edge_c: usize = triple + .get_item(1) + .unwrap() + .downcast::() + .unwrap() + .extract() + .unwrap(); + let edge_w = triple.get_item(2).unwrap().extract().unwrap(); + self.graph + .add_edge(NodeIndex::new(edge_p), NodeIndex::new(edge_c), edge_w); + } } + + // remove the temporary node will remove all deleted edges in bulk, + // the cost is equal to the number of edges + self.graph.remove_node(tmp_node); + Ok(()) } diff --git a/tests/rustworkx_tests/digraph/test_pickle.py b/tests/rustworkx_tests/digraph/test_pickle.py new file mode 100644 index 0000000000..306fd119ca --- /dev/null +++ b/tests/rustworkx_tests/digraph/test_pickle.py @@ -0,0 +1,41 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import pickle +import unittest + +import rustworkx as rx + + +class TestPickleDiGraph(unittest.TestCase): + def test_noweight_graph(self): + g = rx.PyDAG() + for i in range(4): + g.add_node(None) + g.add_edges_from_no_data([(0, 1), (1, 2), (3, 0), (3, 1)]) + g.remove_node(0) + + gprime = pickle.loads(pickle.dumps(g)) + self.assertEqual([1, 2, 3], gprime.node_indices()) + self.assertEqual([None, None, None], gprime.nodes()) + self.assertEqual({1: (1, 2, None), 3: (3, 1, None)}, dict(gprime.edge_index_map())) + + def test_weight_graph(self): + g = rx.PyDAG() + g.add_nodes_from(["A", "B", "C", "D"]) + g.add_edges_from([(0, 1, "A -> B"), (1, 2, "B -> C"), (3, 0, "D -> A"), (3, 1, "D -> B")]) + g.remove_node(0) + + gprime = pickle.loads(pickle.dumps(g)) + self.assertEqual([1, 2, 3], gprime.node_indices()) + self.assertEqual(["B", "C", "D"], gprime.nodes()) + self.assertEqual({1: (1, 2, "B -> C"), 3: (3, 1, "D -> B")}, dict(gprime.edge_index_map())) diff --git a/tests/rustworkx_tests/graph/test_pickle.py b/tests/rustworkx_tests/graph/test_pickle.py new file mode 100644 index 0000000000..44220f1130 --- /dev/null +++ b/tests/rustworkx_tests/graph/test_pickle.py @@ -0,0 +1,41 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import pickle +import unittest + +import rustworkx as rx + + +class TestPickleGraph(unittest.TestCase): + def test_noweight_graph(self): + g = rx.PyGraph() + for i in range(4): + g.add_node(None) + g.add_edges_from_no_data([(0, 1), (1, 2), (3, 0), (3, 1)]) + g.remove_node(0) + + gprime = pickle.loads(pickle.dumps(g)) + self.assertEqual([1, 2, 3], gprime.node_indices()) + self.assertEqual([None, None, None], gprime.nodes()) + self.assertEqual({1: (1, 2, None), 3: (3, 1, None)}, dict(gprime.edge_index_map())) + + def test_weight_graph(self): + g = rx.PyGraph() + g.add_nodes_from(["A", "B", "C", "D"]) + g.add_edges_from([(0, 1, "A -> B"), (1, 2, "B -> C"), (3, 0, "D -> A"), (3, 1, "D -> B")]) + g.remove_node(0) + + gprime = pickle.loads(pickle.dumps(g)) + self.assertEqual([1, 2, 3], gprime.node_indices()) + self.assertEqual(["B", "C", "D"], gprime.nodes()) + self.assertEqual({1: (1, 2, "B -> C"), 3: (3, 1, "D -> B")}, dict(gprime.edge_index_map())) From c17eea558841d2b473226917cd3da90e24a616ee Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 9 May 2023 21:59:17 +0000 Subject: [PATCH 4/6] Bump serde from 1.0.160 to 1.0.162 (#863) Bumps [serde](https://github.com/serde-rs/serde) from 1.0.160 to 1.0.162. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](https://github.com/serde-rs/serde/compare/v1.0.160...1.0.162) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Matthew Treinish --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e4267fc23..b9dbbb3cfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -581,18 +581,18 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "serde" -version = "1.0.160" +version = "1.0.162" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb2f3770c8bce3bcda7e149193a069a0f4365bda1fa5cd88e03bca26afc1216c" +checksum = "71b2f6e1ab5c2b98c05f0f35b236b22e8df7ead6ffbf51d7808da7f8817e7ab6" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.160" +version = "1.0.162" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "291a097c63d8497e00160b166a967a4a79c64f3facdd01cbd7502231688d77df" +checksum = "a2a0814352fd64b58489904a44ea8d90cb1a91dcb6b4f5ebabc32c8318e93cb6" dependencies = [ "proc-macro2", "quote", From a16c18d63635961075037c614559f3b9214434f9 Mon Sep 17 00:00:00 2001 From: matanco64 <38103422+matanco64@users.noreply.github.com> Date: Wed, 10 May 2023 22:13:48 +0300 Subject: [PATCH 5/6] Add reverse inplace function for digraph (#853) * added a reverse_inplace function in digraph, the function reverses the direction of the edges in the digraph implemented by switching the indices of the nodes in an edge. * added python tests for the reverse_inplace function. testing a simple case and a case for a large graph. * ran rust fmt and clippy, also added more detailed documentation * rename reverse_inplace to reverse * change excepts to unwraps (If this fails is because of PyO3. It panics and there is not much point in printing a message) * added tests for empty graph and graph with node removed in the middle * added interface signature for IDEs * ran cargo fmt * Fix doc syntax --------- Co-authored-by: Matthew Treinish --- rustworkx/digraph.pyi | 1 + src/digraph.rs | 33 +++++++++++++++++ tests/rustworkx_tests/digraph/test_edges.py | 41 +++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/rustworkx/digraph.pyi b/rustworkx/digraph.pyi index 56b977d98e..13735b5fc3 100644 --- a/rustworkx/digraph.pyi +++ b/rustworkx/digraph.pyi @@ -166,6 +166,7 @@ class PyDiGraph(Generic[S, T]): deliminator: Optional[str] = ..., weight_fn: Optional[Callable[[T], str]] = ..., ) -> None: ... + def reverse(self) -> None: ... def __delitem__(self, idx: int, /) -> None: ... def __getitem__(self, idx: int, /) -> S: ... def __getstate__(self) -> Any: ... diff --git a/src/digraph.rs b/src/digraph.rs index ba81cbae5f..c4d7518d09 100644 --- a/src/digraph.rs +++ b/src/digraph.rs @@ -2819,6 +2819,39 @@ impl PyDiGraph { self.clone() } + /// Reverse the direction of all edges in the graph, in place. + /// + /// This method modifies the graph instance to reverse the direction of all edges. + /// It does so by iterating over all edges in the graph and removing each edge, + /// then adding a new edge in the opposite direction with the same weight. + /// + /// For Example:: + /// + /// import rustworkx as rx + /// + /// graph = rx.PyDiGraph() + /// + /// # Generate a path directed path graph with weights + /// graph.extend_from_weighted_edge_list([ + /// (0, 1, 3), + /// (1, 2, 5), + /// (2, 3, 2), + /// ]) + /// # Reverse edges + /// graph.reverse() + /// + /// assert graph.weighted_edge_list() == [(3, 2, 2), (2, 1, 5), (1, 0, 3)]; + #[pyo3(text_signature = "(self)")] + pub fn reverse(&mut self, py: Python) { + let indices = self.graph.edge_indices().collect::>(); + for idx in indices { + let (source_node, dest_node) = self.graph.edge_endpoints(idx).unwrap(); + let weight = self.graph.edge_weight(idx).unwrap().clone_ref(py); + self.graph.remove_edge(idx); + self.graph.add_edge(dest_node, source_node, weight); + } + } + /// Return the number of nodes in the graph fn __len__(&self) -> PyResult { Ok(self.graph.node_count()) diff --git a/tests/rustworkx_tests/digraph/test_edges.py b/tests/rustworkx_tests/digraph/test_edges.py index 54a448bfeb..2d9f56ae52 100644 --- a/tests/rustworkx_tests/digraph/test_edges.py +++ b/tests/rustworkx_tests/digraph/test_edges.py @@ -962,3 +962,44 @@ def test_extend_from_weighted_edge_list(self): graph.extend_from_weighted_edge_list(edge_list) self.assertEqual(len(graph), 4) self.assertEqual(["a", "b", "c", "d", "e"], graph.edges()) + + def test_reverse_graph(self): + graph = rustworkx.PyDiGraph() + graph.add_nodes_from([i for i in range(4)]) + edge_list = [ + (0, 1, "a"), + (1, 2, "b"), + (0, 2, "c"), + (2, 3, "d"), + (0, 3, "e"), + ] + graph.add_edges_from(edge_list) + graph.reverse() + self.assertEqual([(1, 0), (2, 1), (2, 0), (3, 2), (3, 0)], graph.edge_list()) + + def test_reverse_large_graph(self): + LARGE_AMOUNT_OF_NODES = 10000000 + + graph = rustworkx.PyDiGraph() + graph.add_nodes_from(range(LARGE_AMOUNT_OF_NODES)) + edge_list = list(zip(range(LARGE_AMOUNT_OF_NODES), range(1, LARGE_AMOUNT_OF_NODES))) + weighted_edge_list = [(s, d, "a") for s, d in edge_list] + graph.add_edges_from(weighted_edge_list) + graph.reverse() + reversed_edge_list = [(d, s) for s, d in edge_list] + self.assertEqual(reversed_edge_list, graph.edge_list()) + + def test_reverse_empty_graph(self): + graph = rustworkx.PyDiGraph() + edges_before = graph.edge_list() + graph.reverse() + self.assertEqual(graph.edge_list(), edges_before) + + def test_removed_middle_node_reverse(self): + graph = rustworkx.PyDiGraph() + graph.add_nodes_from(list(range(5))) + edge_list = [(0, 1), (2, 1), (1, 3), (3, 4), (4, 0)] + graph.extend_from_edge_list(edge_list) + graph.remove_node(1) + graph.reverse() + self.assertEqual(graph.edge_list(), [(4, 3), (0, 4)]) From 70e49653a4d7ff0dae62d3f16d5da343ba2e77da Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 11 May 2023 15:47:09 +0000 Subject: [PATCH 6/6] Bump serde from 1.0.162 to 1.0.163 (#869) Bumps [serde](https://github.com/serde-rs/serde) from 1.0.162 to 1.0.163. - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](https://github.com/serde-rs/serde/compare/v1.0.162...v1.0.163) --- updated-dependencies: - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b9dbbb3cfa..a3c0813656 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -581,18 +581,18 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd" [[package]] name = "serde" -version = "1.0.162" +version = "1.0.163" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71b2f6e1ab5c2b98c05f0f35b236b22e8df7ead6ffbf51d7808da7f8817e7ab6" +checksum = "2113ab51b87a539ae008b5c6c02dc020ffa39afd2d83cffcb3f4eb2722cebec2" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.162" +version = "1.0.163" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2a0814352fd64b58489904a44ea8d90cb1a91dcb6b4f5ebabc32c8318e93cb6" +checksum = "8c805777e3930c8883389c602315a24224bcc738b63905ef87cd1420353ea93e" dependencies = [ "proc-macro2", "quote",