Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: sparse: raise error for array classes, document/test old behavior #20490

Merged
merged 9 commits into from
May 7, 2024
8 changes: 6 additions & 2 deletions scipy/sparse/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,15 @@ def _lil_container(self):
from ._lil import lil_array
return lil_array

def __init__(self, maxprint=MAXPRINT):
def __init__(self, arg1, maxprint=MAXPRINT):
self._shape = None
if self.__class__.__name__ == '_spbase':
raise ValueError("This class is not intended"
" to be instantiated directly.")
if isinstance(self, sparray) and np.isscalar(arg1):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementing the suggestion from our call means that _spbase now needs to check the first argument which resulted in this change. Not sure if the lack of arguments in _spbase was a choice or not, but maybe there's a way around this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... Good point about lack of arguments being a choice or not.
It looks like setting maxprint actually doesn't work for all the subclasses that override __init__. So currently we can't choose a non-default maxprint value for most of the formats.

That could be fixed by adding the maxprint parameter to __init__ methods throughout. Or by adding **kwargs parameters that get passed through to the superclass __init__. The second option is harder to read for someone trying to figure out what all the options are.

And this could be fixed in a separate PR. Let me know if you prefer that and I'll open a separate issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the maxprint functionality has been inaccessible for a while, unfortunately. We should clean that up in a later PR.

raise ValueError(
"scipy sparse array classes do not support instantiation from a scalar"
)
self.maxprint = maxprint

# Use this in 1.14.0 and later:
Expand Down Expand Up @@ -1505,7 +1509,7 @@ def getrow(self, i):

class sparray:
"""A namespace class to separate sparray from spmatrix"""
pass


sparray.__doc__ = _spbase.__doc__

Expand Down
8 changes: 5 additions & 3 deletions scipy/sparse/_bsr.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class _bsr_base(_cs_matrix, _minmax_mixin):
_format = 'bsr'

def __init__(self, arg1, shape=None, dtype=None, copy=False, blocksize=None):
_data_matrix.__init__(self)
_data_matrix.__init__(self, arg1)

if issparse(arg1):
if arg1.format == self.format and copy:
Expand Down Expand Up @@ -94,8 +94,10 @@ def __init__(self, arg1, shape=None, dtype=None, copy=False, blocksize=None):
if not isshape(blocksize):
raise ValueError(f'invalid blocksize={blocksize}')
if tuple(blocksize) != self.data.shape[1:]:
raise ValueError('mismatching blocksize={} vs {}'.format(
blocksize, self.data.shape[1:]))
raise ValueError(
f'mismatching blocksize={blocksize}'
f' vs {self.data.shape[1:]}'
)
else:
raise ValueError('unrecognized bsr_array constructor usage')
else:
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/_compressed.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class _cs_matrix(_data_matrix, _minmax_mixin, IndexMixin):
"""

def __init__(self, arg1, shape=None, dtype=None, copy=False):
_data_matrix.__init__(self)
_data_matrix.__init__(self, arg1)

if issparse(arg1):
if arg1.format == self.format and copy:
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/_coo.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class _coo_base(_data_matrix, _minmax_mixin):
_format = 'coo'

def __init__(self, arg1, shape=None, dtype=None, copy=False):
_data_matrix.__init__(self)
_data_matrix.__init__(self, arg1)
is_array = isinstance(self, sparray)
if not copy:
copy = copy_if_needed
Expand Down
4 changes: 2 additions & 2 deletions scipy/sparse/_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
# TODO implement all relevant operations
# use .data.__methods__() instead of /=, *=, etc.
class _data_matrix(_spbase):
def __init__(self):
_spbase.__init__(self)
def __init__(self, arg1):
_spbase.__init__(self, arg1)

@property
def dtype(self):
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/_dia.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class _dia_base(_data_matrix):
_format = 'dia'

def __init__(self, arg1, shape=None, dtype=None, copy=False):
_data_matrix.__init__(self)
_data_matrix.__init__(self, arg1)

if issparse(arg1):
if arg1.format == "dia":
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/_dok.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class _dok_base(_spbase, IndexMixin, dict):
_format = 'dok'

def __init__(self, arg1, shape=None, dtype=None, copy=False):
_spbase.__init__(self)
_spbase.__init__(self, arg1)

is_array = isinstance(self, sparray)
if isinstance(arg1, tuple) and isshape(arg1, allow_1d=is_array):
Expand Down
2 changes: 1 addition & 1 deletion scipy/sparse/_lil.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class _lil_base(_spbase, IndexMixin):
_format = 'lil'

def __init__(self, arg1, shape=None, dtype=None, copy=False):
_spbase.__init__(self)
_spbase.__init__(self, arg1)
self.dtype = getdtype(dtype, arg1, default=float)

# First get the shape
Expand Down
35 changes: 34 additions & 1 deletion scipy/sparse/tests/test_construct.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@

from scipy.sparse import (csr_matrix, coo_matrix,
csr_array, coo_array,
sparray, spmatrix,
csc_array, bsr_array,
dia_array, dok_array,
lil_array, csc_matrix,
bsr_matrix, dia_matrix,
lil_matrix, sparray, spmatrix,
_construct as construct)
from scipy.sparse._construct import rand as sprand

Expand All @@ -37,6 +41,35 @@ def _sprandn_array(m, n, density=0.01, format="coo", dtype=None, random_state=No


class TestConstructUtils:

@pytest.mark.parametrize("cls", [
csc_array, csr_array, coo_array, bsr_array,
dia_array, dok_array, lil_array
])
def test_singleton_array_constructor(self, cls):
with pytest.raises(
ValueError,
match=(
'scipy sparse array classes do not support '
'instantiation from a scalar'
)
):
cls(0)

@pytest.mark.parametrize("cls", [
csc_matrix, csr_matrix, coo_matrix,
bsr_matrix, dia_matrix, lil_matrix
])
def test_singleton_matrix_constructor(self, cls):
"""
This test is for backwards compatibility post scipy 1.13.
The behavior observed here is what is to be expected
with the older matrix classes. This test comes with the
exception of dok_matrix, which was not working pre scipy1.12
(unlike the rest of these).
"""
assert cls(0).shape == (1, 1)

def test_spdiags(self):
diags1 = array([[1, 2, 3, 4, 5]])
diags2 = array([[1, 2, 3, 4, 5],
Expand Down