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

[Python] Compatibility with SciPy 1.15's stricter sparse code #45229

Open
h-vetinari opened this issue Jan 11, 2025 · 2 comments
Open

[Python] Compatibility with SciPy 1.15's stricter sparse code #45229

h-vetinari opened this issue Jan 11, 2025 · 2 comments

Comments

@h-vetinari
Copy link
Contributor

Describe the enhancement requested

The test suite in conda-forge/arrow-cpp-feedstock#1664 has a single test failure

=================================== FAILURES ===================================
____________ test_sparse_coo_tensor_scipy_roundtrip[f2-arrow_type8] ____________

dtype_str = 'f2', arrow_type = DataType(halffloat)

    @pytest.mark.skipif(not coo_matrix, reason="requires scipy")
    @pytest.mark.parametrize('dtype_str,arrow_type', tensor_type_pairs)
    def test_sparse_coo_tensor_scipy_roundtrip(dtype_str, arrow_type):
        dtype = np.dtype(dtype_str)
        data = np.array([1, 2, 3, 4, 5, 6]).astype(dtype)
        row = np.array([0, 0, 2, 3, 1, 3])
        col = np.array([0, 2, 0, 4, 5, 5])
        shape = (4, 6)
        dim_names = ('x', 'y')
    
        # non-canonical sparse coo matrix
>       scipy_matrix = coo_matrix((data, (row, col)), shape=shape)

pyarrow/tests/test_sparse_tensor.py:408:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
scipy/sparse/_coo.py:62: in __init__
    self.data = getdata(obj, copy=copy, dtype=dtype)
scipy/sparse/_sputils.py:150: in getdata
    getdtype(data.dtype)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

dtype = dtype('float16'), a = None, default = None

    def getdtype(dtype, a=None, default=None):
        """Form a supported numpy dtype based on input arguments.
    
        Returns a valid ``numpy.dtype`` from `dtype` if not None,
        or else ``a.dtype`` if possible, or else the given `default`
        if not None, or else raise a ``TypeError``.
    
        The resulting ``dtype`` must be in ``supported_dtypes``:
            bool_, int8, uint8, int16, uint16, int32, uint32,
            int64, uint64, longlong, ulonglong, float32, float64,
            longdouble, complex64, complex128, clongdouble
        """
        if dtype is None:
            try:
                newdtype = a.dtype
            except AttributeError as e:
                if default is not None:
                    newdtype = np.dtype(default)
                else:
                    raise TypeError("could not interpret data type") from e
        else:
            newdtype = np.dtype(dtype)
    
        if newdtype not in supported_dtypes:
            supported_dtypes_fmt = ", ".join(t.__name__ for t in supported_dtypes)
>           raise ValueError(f"scipy.sparse does not support dtype {newdtype.name}. "
                             f"The only supported types are: {supported_dtypes_fmt}.")
E           ValueError: scipy.sparse does not support dtype float16. The only supported types are: bool, int8, uint8, int16, uint16, int32, uint32, int64, uint64, longlong, ulonglong, float32, float64, longdouble, complex64, complex128, clongdouble.

scipy/sparse/_sputils.py:137: ValueError
=============================== warnings summary ===============================

As far as I can tell, SciPy hasn't supported float16 in sparse code for the last several years, so what happened most likely is simply that the checks got stricter in the API. If I'm wrong about that, please raise an issue in scipy.

Sidenote: compare also the migration guide for getting from scipy.sparse.*_matrix to scipy.sparse.*_array, though that probably warrants a separate issue.

Component(s)

Python

@raulcd
Copy link
Member

raulcd commented Jan 13, 2025

@rok @AlenkaF @jorisvandenbossche I don't think we have any CI job that tests with scipy and this code is coming from a long time ago: cdaf988
We should exercise those tests on CI.

@rok
Copy link
Member

rok commented Jan 15, 2025

Agreed @raulcd. We should also follow the migration guide as @h-vetinari suggests.

@rok rok self-assigned this Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants