Skip to content

Commit

Permalink
BUG: sparse: 'diagonal' of sparse matrices fixed to match numpy's dia…
Browse files Browse the repository at this point in the history
…gonal behaviour (scipy#11970)

* BUG: sparse: Added check for size in method 'diagonal' of sparse matrices to deal with matrices of size 0

'diagonal' raise ValueError if a sparse matrix size is 0,
but equivalent function 'numpy.diagonal' return an empty array, as mentioned in issue scipy#11949.
'diagonal' check if k is less than size, but in case of an empty matrix,
k is equal to the size and ValueError rises.
Added check so if any of sizes is 0, 'diagonal' return an empty array, like 'numpy.diagonal'.

* BUG: sparse: Removed raising a ValueError from `diagonal` method and fixed type of returned array

To match the behaviour of `numpy.diagonal` replaced raising ValueError
when passed a value exceeds the number of rows or columns with returning an empty array.
Also changed dtype of a returned empty array to match dtype of a sparse matrix.

* TST: sparse: added tests to cover offset in `diagonal` greater than the size of a matrix

New test cover that and few more shapes of
an empty sparse matrix(when one of the sizes is 0 and other not)

* MAINT: Fixed typo in comments

Co-authored-by: Warren Weckesser <[email protected]>

Co-authored-by: Warren Weckesser <[email protected]>
  • Loading branch information
ADmitri42 and WarrenWeckesser authored May 2, 2020
1 parent 787efe4 commit b887d9e
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 7 deletions.
2 changes: 1 addition & 1 deletion scipy/sparse/bsr.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def __repr__(self):
def diagonal(self, k=0):
rows, cols = self.shape
if k <= -rows or k >= cols:
raise ValueError("k exceeds matrix dimensions")
return np.empty(0, dtype=self.data.dtype)
R, C = self.blocksize
y = np.zeros(min(rows + min(k, 0), cols - max(k, 0)),
dtype=upcast(self.dtype))
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/compressed.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ def _mul_sparse_matrix(self, other):
def diagonal(self, k=0):
rows, cols = self.shape
if k <= -rows or k >= cols:
raise ValueError("k exceeds matrix dimensions")
return np.empty(0, dtype=self.data.dtype)
fn = getattr(_sparsetools, self.format + "_diagonal")
y = np.empty(min(rows + min(k, 0), cols - max(k, 0)),
dtype=upcast(self.dtype))
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/coo.py
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def todok(self, copy=False):
def diagonal(self, k=0):
rows, cols = self.shape
if k <= -rows or k >= cols:
raise ValueError("k exceeds matrix dimensions")
return np.empty(0, dtype=self.data.dtype)
diag = np.zeros(min(rows + min(k, 0), cols - max(k, 0)),
dtype=self.dtype)
diag_mask = (self.row + k) == self.col
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/dia.py
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ def transpose(self, axes=None, copy=False):
def diagonal(self, k=0):
rows, cols = self.shape
if k <= -rows or k >= cols:
raise ValueError("k exceeds matrix dimensions")
return np.empty(0, dtype=self.data.dtype)
idx, = np.nonzero(self.offsets == k)
first_col, last_col = max(0, k), min(rows + k, cols)
if idx.size == 0:
Expand Down
12 changes: 9 additions & 3 deletions scipy/sparse/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -724,13 +724,19 @@ def test_diagonal(self):
for m in mats:
rows, cols = array(m).shape
sparse_mat = self.spmatrix(m)
for k in range(-rows + 1, cols):
for k in range(-rows-1, cols+2):
assert_equal(sparse_mat.diagonal(k=k), diag(m, k=k))
assert_raises(ValueError, sparse_mat.diagonal, -rows)
assert_raises(ValueError, sparse_mat.diagonal, cols)
# Test for k beyond boundaries(issue #11949)
assert_equal(sparse_mat.diagonal(k=10), diag(m, k=10))
assert_equal(sparse_mat.diagonal(k=-99), diag(m, k=-99))

# Test all-zero matrix.
assert_equal(self.spmatrix((40, 16130)).diagonal(), np.zeros(40))
# Test empty matrix
# https://github.com/scipy/scipy/issues/11949
assert_equal(self.spmatrix((0, 0)).diagonal(), np.empty(0))
assert_equal(self.spmatrix((15, 0)).diagonal(), np.empty(0))
assert_equal(self.spmatrix((0, 5)).diagonal(10), np.empty(0))

def test_reshape(self):
# This first example is taken from the lil_matrix reshaping test.
Expand Down

0 comments on commit b887d9e

Please sign in to comment.