Cannot inherit csr_matrix properly

[xeechou]

Hi, I found csr_matrix data structure is quite suitable for doing Depth First Search and Beadth First Search, so I tried to inherit csr_matrix to make a new sgraph class.

class sgraph(csr_matrix):
    def dfs(self):
        (m,n) = self.get_shape()
        if m != n:
            raise NameError('this is not a graph')
        nodes = {i:0 for i in range(m)}
        visited = [0] * m;
        visited[0] = 1
        while nodes:
            _dfs(self, nodes, visited)
            print('\n')

    def _dfs(self, nodes, visited):
        stack = [get_key(nodes)]
        #dfs is trickier because  we could meet nodes after we pushed it already
        while stack:
            u = stack.pop()
            if visited[u] == 2: 
                                #we visited before
                continue
            #u have to visit it first
            print(u)
            del nodes[u]
            visited[u] = 2
            for v in self.getrow(u).indices:
                if visited[v] != 2:
                    visited[v] = 1
                    stack.append(v)

But I could not construct a object with this class.

if __name__ == "__main__":
    b = sgraph([[1,0,0,0,0],
                [0,0,1,1,0],
                [0,2,1,0,1],
                [1,0,0,0,1],
                [0,2,1,0,0]])
    #bfs(b)
    b.dfs()

I got the following error:

Traceback (most recent call last):
  File "error.py", line 49, in <module>
    [0,2,1,0,0]])
  File "/usr/lib/python2.7/site-packages/scipy/sparse/compressed.py", line 69,
in __init__
    self._set_self(self.__class__(coo_matrix(arg1, dtype=dtype)))
  File "/usr/lib/python2.7/site-packages/scipy/sparse/compressed.py", line 31,
in __init__
    arg1 = arg1.asformat(self.format)
  File "/usr/lib/python2.7/site-packages/scipy/sparse/base.py", line 219, in
asformat
    return getattr(self,'to' + format)()
  File "/usr/lib/python2.7/site-packages/scipy/sparse/base.py", line 508, in
__getattr__
    raise AttributeError(attr + " not found")
AttributeError: tosgr not found

Could u help to inherit csr_matrix properly

[perimosocordiae]

The issue is that sparse matrix classes take their self.format attribute from the first three characters of their class name: https://github.com/scipy/scipy/blob/master/scipy/sparse/base.py#L70

Unfortunately, this isn't particularly easy to work around. I'd recommend not subclassing for your application, but perhaps keeping the graph as an instance variable in your search class.

[perimosocordiae]

Also note that this sort of question is better suited for StackOverflow, where many more eyes will see it.

I'll leave the issue open for now because I'm wondering if the difficulty of subclassing is a bug or not.

[xeechou]

thanks :)

[rgommers]

I think these classes simply aren't designed for subclassing by users. It can be done, but probably never a good idea.

[josef-pkt]

just a thought:
If subclassing requires the use of internal implementation details, that it might be better to explicitly state that it's not protected for future backwards compatibility.
If there are recommended recipes for it, then changing things will become more difficult.
(example stats.gaussian_kde where subclassing was a required work around the original limitations.)

(I'm Not going to subclass sparse matrices to replace dot by element wise multiplication. :)

[matthew-brett]

For the particular problem, I guess you could override self.format with a property?

[perimosocordiae]

@matthew-brett you'd have to do some hackery with a no-op setter method, because spmatrix.__init__ assigns to self.format unconditionally and it's used by _cs_matrix.__init__, before any user code has a chance to run.

I'm coming around to the idea of a refactor, in which the format string is set explicitly (probably as a read-only property) for each class. The "first three characters of the class name" approach is a little too magic for my tastes. Does anyone remember why it was designed that way originally?

[xeechou]

So, the quick solution is to replace the subclass name to something like csr_sgraph. Yep, it is better if there is a chance to set the format explicitly.

[matthew-brett]

@perimosocordiae - it sounds like a refactor could solve this particular problem without causing compatibility problems - is that right?

[perimosocordiae]

That's the idea. I'll open a PR soon.

[rgommers]

The PR seems fine, but I think we should explicitly document that subclassing is not recommended/supported. It's quite hard to not break custom subclasses by accident, and from experience with ndarray I think we know that subclassing is rarely the best option.

[perimosocordiae]

I agree that a bit of a warning in the docs is a good idea. Where would be best for that sort of thing?

[pv]

fixed in gh-5530