From 1fef7e5389a865d0ee8ae5893663a07e3746a944 Mon Sep 17 00:00:00 2001 From: Ryan Ly Date: Mon, 8 Jul 2024 00:01:07 -0400 Subject: [PATCH 1/2] Move scipy to optional requirements (#1140) * Move scipy to optional requirements * Update changelog * Add test for missing scipy importerror * Fix test --- CHANGELOG.md | 5 +++++ pyproject.toml | 1 - requirements-opt.txt | 2 ++ requirements.txt | 1 - src/hdmf/common/sparse.py | 22 +++++++++++++++++----- tests/unit/common/test_sparse.py | 23 ++++++++++++++++++++--- 6 files changed, 44 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f5db4918..90a4554bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # HDMF Changelog +## HDMF 4.0.0 (Upcoming) + +### Breaking changes +- Scipy is no longer a required dependency. Users using the `CSRMatrix` data type should install `scipy` separately. @rly [#1140](https://github.com/hdmf-dev/hdmf/pull/1140) + ## HDMF 3.14.2 (Upcoming) ### Bug fixes diff --git a/pyproject.toml b/pyproject.toml index a089113c0..86c26f779 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,7 +35,6 @@ dependencies = [ 'numpy>=1.18, <2.0', # pin below 2.0 until HDMF supports numpy 2.0 "pandas>=1.0.5", "ruamel.yaml>=0.16", - "scipy>=1.4", "zarr >= 2.12.0", "importlib-resources; python_version < '3.9'", # TODO: remove when minimum python version is 3.9 ] diff --git a/requirements-opt.txt b/requirements-opt.txt index c1d34220b..efa1e391b 100644 --- a/requirements-opt.txt +++ b/requirements-opt.txt @@ -4,3 +4,5 @@ zarr==2.17.1 linkml-runtime==1.7.4; python_version >= "3.9" schemasheets==0.2.1; python_version >= "3.9" oaklib==0.5.32; python_version >= "3.9" +scipy==1.14.0; python_version >= "3.10" # scipy is used only in CSRMatrix data type +scipy==1.11.3; python_version < "3.10" diff --git a/requirements.txt b/requirements.txt index 5182d5c2e..cc2312614 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,4 +5,3 @@ jsonschema==4.19.1 numpy==1.26.1 pandas==2.1.2 ruamel.yaml==0.18.2 -scipy==1.11.3 diff --git a/src/hdmf/common/sparse.py b/src/hdmf/common/sparse.py index db38d12e8..0dd7d9654 100644 --- a/src/hdmf/common/sparse.py +++ b/src/hdmf/common/sparse.py @@ -1,4 +1,11 @@ -import scipy.sparse as sps +try: + from scipy.sparse import csr_matrix + SCIPY_INSTALLED = True +except ImportError: + SCIPY_INSTALLED = False + class csr_matrix: # dummy class to prevent import errors + pass + from . import register_class from ..container import Container from ..utils import docval, popargs, to_uint_array, get_data_shape, AllowPositional @@ -7,7 +14,7 @@ @register_class('CSRMatrix') class CSRMatrix(Container): - @docval({'name': 'data', 'type': (sps.csr_matrix, 'array_data'), + @docval({'name': 'data', 'type': (csr_matrix, 'array_data'), 'doc': 'the data to use for this CSRMatrix or CSR data array.' 'If passing CSR data array, *indices*, *indptr*, and *shape* must also be provided'}, {'name': 'indices', 'type': 'array_data', 'doc': 'CSR index array', 'default': None}, @@ -16,13 +23,17 @@ class CSRMatrix(Container): {'name': 'name', 'type': str, 'doc': 'the name to use for this when storing', 'default': 'csr_matrix'}, allow_positional=AllowPositional.WARNING) def __init__(self, **kwargs): + if not SCIPY_INSTALLED: + raise ImportError( + "scipy must be installed to use CSRMatrix. Please install scipy using `pip install scipy`." + ) data, indices, indptr, shape = popargs('data', 'indices', 'indptr', 'shape', kwargs) super().__init__(**kwargs) - if not isinstance(data, sps.csr_matrix): + if not isinstance(data, csr_matrix): temp_shape = get_data_shape(data) temp_ndim = len(temp_shape) if temp_ndim == 2: - data = sps.csr_matrix(data) + data = csr_matrix(data) elif temp_ndim == 1: if any(_ is None for _ in (indptr, indices, shape)): raise ValueError("Must specify 'indptr', 'indices', and 'shape' arguments when passing data array.") @@ -31,9 +42,10 @@ def __init__(self, **kwargs): shape = self.__check_arr(shape, 'shape') if len(shape) != 2: raise ValueError("'shape' argument must specify two and only two dimensions.") - data = sps.csr_matrix((data, indices, indptr), shape=shape) + data = csr_matrix((data, indices, indptr), shape=shape) else: raise ValueError("'data' argument cannot be ndarray of dimensionality > 2.") + # self.__data is a scipy.sparse.csr_matrix self.__data = data @staticmethod diff --git a/tests/unit/common/test_sparse.py b/tests/unit/common/test_sparse.py index 7d94231f4..068d3a823 100644 --- a/tests/unit/common/test_sparse.py +++ b/tests/unit/common/test_sparse.py @@ -1,10 +1,25 @@ from hdmf.common import CSRMatrix from hdmf.testing import TestCase, H5RoundTripMixin - -import scipy.sparse as sps import numpy as np +import unittest + +try: + import scipy.sparse as sps + SCIPY_INSTALLED = True +except ImportError: + SCIPY_INSTALLED = False + + +@unittest.skipIf(SCIPY_INSTALLED, "scipy is installed") +class TestCSRMatrixNoScipy(TestCase): + def test_import_error(self): + data = np.array([[1, 0, 2], [0, 0, 3], [4, 5, 6]]) + with self.assertRaises(ImportError): + CSRMatrix(data=data) + +@unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") class TestCSRMatrix(TestCase): def test_from_sparse_matrix(self): @@ -18,6 +33,7 @@ def test_from_sparse_matrix(self): received = CSRMatrix(data=sps_mat) self.assertContainerEqual(received, expected, ignore_hdmf_attrs=True) + @unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") def test_2d_data(self): data = np.array([[1, 0, 2], [0, 0, 3], [4, 5, 6]]) csr_mat = CSRMatrix(data=data) @@ -153,7 +169,7 @@ def test_array_bad_dim(self): with self.assertRaisesWith(ValueError, msg): CSRMatrix(data=data, indices=indices, indptr=indptr, shape=shape) - +@unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") class TestCSRMatrixRoundTrip(H5RoundTripMixin, TestCase): def setUpContainer(self): @@ -164,6 +180,7 @@ def setUpContainer(self): return CSRMatrix(data=data, indices=indices, indptr=indptr, shape=shape) +@unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") class TestCSRMatrixRoundTripFromLists(H5RoundTripMixin, TestCase): """Test that CSRMatrix works with lists as well""" From 2f3044099a4e44042d1f6daa8a61d41d5f9f2657 Mon Sep 17 00:00:00 2001 From: Matthew Avaylon Date: Sun, 7 Jul 2024 21:11:26 -0700 Subject: [PATCH 2/2] Revert "Move scipy to optional requirements (#1140)" This reverts commit 1fef7e5389a865d0ee8ae5893663a07e3746a944. --- CHANGELOG.md | 5 ----- pyproject.toml | 1 + requirements-opt.txt | 2 -- requirements.txt | 1 + src/hdmf/common/sparse.py | 22 +++++----------------- tests/unit/common/test_sparse.py | 23 +++-------------------- 6 files changed, 10 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90a4554bf..5f5db4918 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,5 @@ # HDMF Changelog -## HDMF 4.0.0 (Upcoming) - -### Breaking changes -- Scipy is no longer a required dependency. Users using the `CSRMatrix` data type should install `scipy` separately. @rly [#1140](https://github.com/hdmf-dev/hdmf/pull/1140) - ## HDMF 3.14.2 (Upcoming) ### Bug fixes diff --git a/pyproject.toml b/pyproject.toml index 86c26f779..a089113c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,6 +35,7 @@ dependencies = [ 'numpy>=1.18, <2.0', # pin below 2.0 until HDMF supports numpy 2.0 "pandas>=1.0.5", "ruamel.yaml>=0.16", + "scipy>=1.4", "zarr >= 2.12.0", "importlib-resources; python_version < '3.9'", # TODO: remove when minimum python version is 3.9 ] diff --git a/requirements-opt.txt b/requirements-opt.txt index efa1e391b..c1d34220b 100644 --- a/requirements-opt.txt +++ b/requirements-opt.txt @@ -4,5 +4,3 @@ zarr==2.17.1 linkml-runtime==1.7.4; python_version >= "3.9" schemasheets==0.2.1; python_version >= "3.9" oaklib==0.5.32; python_version >= "3.9" -scipy==1.14.0; python_version >= "3.10" # scipy is used only in CSRMatrix data type -scipy==1.11.3; python_version < "3.10" diff --git a/requirements.txt b/requirements.txt index cc2312614..5182d5c2e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -5,3 +5,4 @@ jsonschema==4.19.1 numpy==1.26.1 pandas==2.1.2 ruamel.yaml==0.18.2 +scipy==1.11.3 diff --git a/src/hdmf/common/sparse.py b/src/hdmf/common/sparse.py index 0dd7d9654..db38d12e8 100644 --- a/src/hdmf/common/sparse.py +++ b/src/hdmf/common/sparse.py @@ -1,11 +1,4 @@ -try: - from scipy.sparse import csr_matrix - SCIPY_INSTALLED = True -except ImportError: - SCIPY_INSTALLED = False - class csr_matrix: # dummy class to prevent import errors - pass - +import scipy.sparse as sps from . import register_class from ..container import Container from ..utils import docval, popargs, to_uint_array, get_data_shape, AllowPositional @@ -14,7 +7,7 @@ class csr_matrix: # dummy class to prevent import errors @register_class('CSRMatrix') class CSRMatrix(Container): - @docval({'name': 'data', 'type': (csr_matrix, 'array_data'), + @docval({'name': 'data', 'type': (sps.csr_matrix, 'array_data'), 'doc': 'the data to use for this CSRMatrix or CSR data array.' 'If passing CSR data array, *indices*, *indptr*, and *shape* must also be provided'}, {'name': 'indices', 'type': 'array_data', 'doc': 'CSR index array', 'default': None}, @@ -23,17 +16,13 @@ class CSRMatrix(Container): {'name': 'name', 'type': str, 'doc': 'the name to use for this when storing', 'default': 'csr_matrix'}, allow_positional=AllowPositional.WARNING) def __init__(self, **kwargs): - if not SCIPY_INSTALLED: - raise ImportError( - "scipy must be installed to use CSRMatrix. Please install scipy using `pip install scipy`." - ) data, indices, indptr, shape = popargs('data', 'indices', 'indptr', 'shape', kwargs) super().__init__(**kwargs) - if not isinstance(data, csr_matrix): + if not isinstance(data, sps.csr_matrix): temp_shape = get_data_shape(data) temp_ndim = len(temp_shape) if temp_ndim == 2: - data = csr_matrix(data) + data = sps.csr_matrix(data) elif temp_ndim == 1: if any(_ is None for _ in (indptr, indices, shape)): raise ValueError("Must specify 'indptr', 'indices', and 'shape' arguments when passing data array.") @@ -42,10 +31,9 @@ def __init__(self, **kwargs): shape = self.__check_arr(shape, 'shape') if len(shape) != 2: raise ValueError("'shape' argument must specify two and only two dimensions.") - data = csr_matrix((data, indices, indptr), shape=shape) + data = sps.csr_matrix((data, indices, indptr), shape=shape) else: raise ValueError("'data' argument cannot be ndarray of dimensionality > 2.") - # self.__data is a scipy.sparse.csr_matrix self.__data = data @staticmethod diff --git a/tests/unit/common/test_sparse.py b/tests/unit/common/test_sparse.py index 068d3a823..7d94231f4 100644 --- a/tests/unit/common/test_sparse.py +++ b/tests/unit/common/test_sparse.py @@ -1,25 +1,10 @@ from hdmf.common import CSRMatrix from hdmf.testing import TestCase, H5RoundTripMixin -import numpy as np -import unittest - -try: - import scipy.sparse as sps - SCIPY_INSTALLED = True -except ImportError: - SCIPY_INSTALLED = False - - -@unittest.skipIf(SCIPY_INSTALLED, "scipy is installed") -class TestCSRMatrixNoScipy(TestCase): - def test_import_error(self): - data = np.array([[1, 0, 2], [0, 0, 3], [4, 5, 6]]) - with self.assertRaises(ImportError): - CSRMatrix(data=data) +import scipy.sparse as sps +import numpy as np -@unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") class TestCSRMatrix(TestCase): def test_from_sparse_matrix(self): @@ -33,7 +18,6 @@ def test_from_sparse_matrix(self): received = CSRMatrix(data=sps_mat) self.assertContainerEqual(received, expected, ignore_hdmf_attrs=True) - @unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") def test_2d_data(self): data = np.array([[1, 0, 2], [0, 0, 3], [4, 5, 6]]) csr_mat = CSRMatrix(data=data) @@ -169,7 +153,7 @@ def test_array_bad_dim(self): with self.assertRaisesWith(ValueError, msg): CSRMatrix(data=data, indices=indices, indptr=indptr, shape=shape) -@unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") + class TestCSRMatrixRoundTrip(H5RoundTripMixin, TestCase): def setUpContainer(self): @@ -180,7 +164,6 @@ def setUpContainer(self): return CSRMatrix(data=data, indices=indices, indptr=indptr, shape=shape) -@unittest.skipIf(not SCIPY_INSTALLED, "scipy is not installed") class TestCSRMatrixRoundTripFromLists(H5RoundTripMixin, TestCase): """Test that CSRMatrix works with lists as well"""