Python copy does not replicate PyDiGraph.copy()

[lukepmccombs]

Information

• rustworkx version: 0.12.1
• Python version: 3.10.6
• Rust version: 1.64.0
• Operating system: Ubuntu 22.04.2 LTS

What is the current behavior?

check_cycles is not copied when a PyDiGraph is copied using the python native copy.copy.

What is the expected behavior?

copy.copy(PyDiGraph) should produce identical results to PyDiGraph.copy().

Steps to reproduce the problem

import rustworkx as rx
import copy


a = rx.PyDiGraph(check_cycle=True)
print(a.check_cycle)

b = a.copy()
print(b.check_cycle)

c = copy.copy(a)
print(c.check_cycle)

d = copy.deepcopy(a)
print(d.check_cycle)

True
True
False
False

Edit: My original test used an object that copy.deepcopy() does not copy into a new object. Upon further testing, it behaves as expected.

[lukepmccombs]

The check_cycles issue seems to stem from the implementation of __getstate__ in src/digraph.rs, which is missing an entry for it.

[lukepmccombs]

Here is a bandaid fix I made to add support for copy at the very least.

import copyreg
import rustworkx as rx

def __pickle_pydigraph(net: rx.PyDiGraph):
    return rx.PyDiGraph, (net.check_cycle,), net.__getstate__()

copyreg.pickle(rx.PyDiGraph, __pickle_pydigraph)

Edit: Found a better way of doing this.

[mtreinish]

Hmm, yeah the deep copy is a oversight and simple bug to fix, we just missed check_cycle in the __getstate__/__setstate__ methods on PyDiGraph (I expect because it was copy and pasted from PyGraph and modified).

As for the shallow copy bug, I'm not sure why it's missing that attribute we don't define anything custom related to the copy.copy() function. But I'm thinking the best path forward might be to just define __copy__ on the classes and have it internally do what the copy() method does and return self.clone().

Would you be interested in pushing a PR to fix these issues?

[lukepmccombs]

From my own research into the issue, the default behavior of copy and deepcopy is to get the state using __getstate__ and set the state of a new object using __setstate__. Deepcopy further deepcopies the state in between. The fix should only require adding check_cycle to both functions. I can take a look at fixing it myself when I have the time, I don't imagine it will be hard.