(Digraph) object has no attribute '_adj' (with more info)

[stablum]

I'm duplicating the ticket #2741 because I cannot re-open it myself.

While trying copy.deepcopy of a graph, I am encountering the following exception:

File "/home/francesco/.local/lib/python3.6/site-packages/networkx/classes/reportviews.py", line 894, in __setstate__
    self._adjdict = G._succ if hasattr(G, "succ") else G._adj
AttributeError: 'TrackGraph' object has no attribute '_adj'

where TrackGraph is a subclass of DiGraph:

class TrackGraph(nx.DiGraph):
    ...

Version is the following:

>>> nx.__version__
'2.0'

I cannot provide the full code because it's a big project, and I could not isolate the bug in a simpler case.
My hypothesis is that the order in which the unplicking of the attributes happens is relevant.
Apparently one of these attributes is an OutEdgeView. I cannot see which attribute because python's pickle is notoriously uninformative during debugging.

Anyways, I think that that attribute is being set (and hence __setstate__ triggered) before _adj or other attributes are set. In that case the OutEdgeView.__setstate__ is faced with an almost empty object, without _adj.

Here is a demonstration:

import pickle

class Bar(object):
    def __init__(self,foo,value):
        self.value = value
        self.foo = foo
    
    def __setstate__(self,state):
        self.value = state['value']
        print("__setstate__ value:",self.value)
        self.foo = state['foo']
        if 'a' in dir(self.foo):
            print(self.foo.a)
        else:
            print("self.foo does not have 'a'")
        if 'b' in dir(self.foo):
            print(self.foo.b)
        else:
            print("self.foo does not have 'b'")

    def __str__(self):
        return "Bar, value={}".format(self.value)

class Foo(object):
    def __init__(self):
        self.a = Bar(self,42)
        self.b = Bar(self,23)

def main():
    foo1 = Foo()
    with open('/tmp/foo','wb') as f:
        pickle.dump(foo1,f)
    print("just pickled. Now let's unpickle")
    with open('/tmp/foo','rb') as f:
        foo2 = pickle.load(f)
    print("just unpickled.",foo2)
    print("foo2.a",foo2.a)
    print("foo2.b",foo2.b)
    print("foo2.a.foo",foo2.a.foo)
    print("foo2.b.foo",foo2.b.foo)

if __name__=="__main__":
    main()

Which produces the following output:

just pickled. Now let's unpickle
__setstate__ value: 42
self.foo <__main__.Foo object at 0x7f41b2af2240> does not have 'a'
self.foo <__main__.Foo object at 0x7f41b2af2240> does not have 'b'
__setstate__ value: 23
self.foo <__main__.Foo object at 0x7f41b2af2240> does not have 'a'
self.foo <__main__.Foo object at 0x7f41b2af2240> does not have 'b'
just unpickled. <__main__.Foo object at 0x7f41b2af2240>
foo2.a Bar, value=42
foo2.b Bar, value=23
foo2.a.foo <__main__.Foo object at 0x7f41b2af2240>
foo2.b.foo <__main__.Foo object at 0x7f41b2af2240>

This demonstrates that foo2.a.foo is of course not fully restored when foo2.a.__setstate__ is triggered. But even worse, after a has been restored, it is not set yet to foo2, as can be demonstrated by foo2.b.__setstate__ that cannot access a. This seems to be the problem in OutEdgeView.

Opinions?

[dschult]

I'm confused... Is this a problem when you deepcopy or when you pickle?

The OutEdgeView object is likely to be the G.edges object. And it makes sense that trying to create that object before the graph exists will cause trouble. But I still don't understand enough about what the problem is or maybe what you are trying to do. Are you pickling as part of a deepcopy?

[stablum]

copy.deepcopy also makes use of __setstate__, so the problem is very closely related. In fact, you could replace the pickling/unpickling in the previous snippet with deepcopy to get a similar behavior. So, no, my pickling is not part of a deepcopy. The problem emerged in two distinct parts of my code, one related to a deep copy and the other on pickling.
Is the problem clear? Do you agree it is a bug? I might have a solution in mind...

[dschult]

I haven't had time to look at it thoroughly. But it seems like we need to make sure G.edges and G.nodes are not part of any pickle process. They get recreated when needed. Might be simplest just to have them created every time we use them. But I think a setstate on TrackGraph could do the trick. Maybe there is a way to put it on Graph so that it works for the other flavors of graph: DiGraph/MultiGraph/MultiDiGraph/OrderedGraph, etc.

Is this similar to your ideas for a solution?

[stablum]

I think that the simplest way is to replace the tricky instance variable with lazy-load @Attribute getters. Here is my solution:

diff --git a/networkx/classes/reportviews.py b/networkx/classes/reportviews.py
index ac5255f2..d4c015b4 100644
--- a/networkx/classes/reportviews.py
+++ b/networkx/classes/reportviews.py
@@ -890,9 +890,18 @@ class OutEdgeView(Set, Mapping):
         return {'_graph': self._graph}
 
     def __setstate__(self, state):
-        self._graph = G = state['_graph']
-        self._adjdict = G._succ if hasattr(G, "succ") else G._adj
-        self._nodes_nbrs = self._adjdict.items
+        self._graph = state['_graph']
+
+    @property
+    def _adjdict(self):
+        G = self._graph
+        if not hasattr(self,'__adjdict'):
+            self.__adjdict = G._succ if hasattr(G, "succ") else G._adj
+        return self.__adjdict
+
+    @property
+    def _nodes_nbrs(self):
+        return self._adjdict.items
 
     @classmethod
     def _from_iterable(self, it):
@@ -902,7 +911,6 @@ class OutEdgeView(Set, Mapping):
 
     def __init__(self, G):
         self._graph = G
-        self._adjdict = G._succ if hasattr(G, "succ") else G._adj
         self._nodes_nbrs = self._adjdict.items
 
     # Set methods
@@ -1040,9 +1048,18 @@ class InEdgeView(OutEdgeView):
     __slots__ = ()
 
     def __setstate__(self, state):
-        self._graph = G = state['_graph']
-        self._adjdict = G._pred if hasattr(G, "pred") else G._adj
-        self._nodes_nbrs = self._adjdict.items
+        self._graph = state['_graph']
+
+    @property
+    def _adjdict(self):
+        G = self._graph
+        if not hasattr(self,'__adjdict'):
+            self.__adjdict = G._succ if hasattr(G, "pred") else G._adj
+        return self.__adjdict
+
+    @property
+    def _nodes_nbrs(self):
+        return self._adjdict.items
 
     dataview = InEdgeDataView
 
@@ -1140,8 +1157,17 @@ class InMultiEdgeView(OutMultiEdgeView):
 
     def __setstate__(self, state):
         self._graph = G = state['_graph']
-        self._adjdict = G._pred if hasattr(G, "pred") else G._adj
-        self._nodes_nbrs = self._adjdict.items
+
+    @property
+    def _adjdict(self):
+        G = self._graph
+        if not hasattr(self,'__adjdict'):
+            self.__adjdict = G._succ if hasattr(G, "pred") else G._adj
+        return self.__adjdict
+
+    @property
+    def _nodes_nbrs(self):
+        return self._adjdict.items
 
     dataview = InMultiEdgeDataView
 

[stablum]

Here is the pull request, seems to be working: #2796

[dschult]

I'm not sure I understand the unpickling problem you are trying to solve.
Can you simply add a setstate method for TrackGraph that avoids pickling the EdgeViews?

As far as I understand, pickling and unpickling works fine for the NetworkX classes. It just isn't working for your TrackGraph. Is that correct?

[dschult]

See #2915 which might help fix this problem.

[dschult]

@stablum The PR #3011 removed circular references between the base classes and graphviews. So pickle and deepcopy should be easier to subclass.

Would it be easy for you to try your TrackGraph with the latest version of NetworkX and see if the issue is fixed? Thanks...

[dschult]

@stablum I have simplified the process for subclassing Graph/DiGraph etc.
The GraphViews were adding too much overhead to avoid cyclic references and you weren't the only one having issues with subclassing. I'm hopeful that the new system will be easier.

For starters, get rid of fresh_copy and any getstate stuff. It you have a directed and undirected TrackGraph and you want to_directed to work, you should overwrite two methods to tell what class to use when converting directed-ness.

def to_directed_class(self):
    return TrackDiGraph
def to_undirected_class(self):
    return TrackGraph

Please let me know how this conversion goes. I think it makes it much better for you. This is going into NetworkX 2.0 which should be released soon.