✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: f4b4d15. Link to the linter CI: here}

To convert the one remaining A.getnnz I had to adjust the sparsefuncs.py function count_nonzero to support CSC format. I adjusted the test for that as well.

Thanks for the PR 🙏!

Looks like we were still using sparse matrix specific constructs like * instead of @ and that no tests were complaining about it. It kind of makes me think that we have a lack of coverage about testing algorithms with both sparse matrices and sparse arrays, i.e. what #27090 was trying to achieve.

I guess tackling this lack of coverage in this PR is a bit too much to ask, not 100% sure 🤔.

I think #27090 did a very good job of:

• making sure that code worked for inputs that are either sparse matrix or sparse array
• checking that tests which used sparse constructors checked that either constructor would work

The approach used here to identify needed changes is to (locally on my installation) monkey_patch the scipy.sparse matrix classes so that calling __mul__ with a non-scalar other argument raises an error. And calling getformat raises, etc. This does not check that sparse array inputs work. That's already done. This flags any internal computations involving sparse matrices that cannot work with sparse arrays due to old-style-syntax. I believe these are a few places where, internally, sparse matrices are being created, manipulated and results are returned without the sparse matrix being exposed publicly.

So the goal here is somewhat different from that of the previous issue. The library is already tested to work for sparse matrix and sparse array. This effort is preparing the library to work with all internal sparse creation using sparse arrays. Pass 1 makes sure that internally used syntax will support either. Pass 2 shifts internal creation to sparse arrays. Ideally, the library should work with either type of input throughout this process. Decisions during pass 2 will need to be made as part of that process. The very long term goal is that after pass 2, and after almost all users have shifted to sparse arrays, we could allow users to install a version of SciPy without support for sparse matrices.

I will look into adding tests that flag these cases when run with sparse arrays. But these leftovers are the places where creation of the sparse entity is hidden behind multiple layers of object creation and transforming. For example, the first line of the diff fixes handling a sparse matrix that arises from 1) creating a ColumnTransformer, 2) calling that transformer's .fit_transform method on a numpy array with Python Object dtype that holds strings, integers and True/False. It's not intuitive why that should return a sparse matrix. But the result is a sparse matrix -- and the rabbit hole of creation parameters and conditions is deep(!) involving many private helper functions! So I don't promise anything, but I will look into it.

I also discovered that I did not run the doctests with my monkey-patched tool. So I will add any additionally discovered fixes (and tests of them where appropriate) too.

Further tests revealed two more matmul operations that needed to be switched from * to @.

All the fixes in the PR are orthogonal to the goals from #27090 in that these new fixes are not dependent on sparse inputs. This PR changes the handling of sparse matrices created internally to sklearn. And the changes here should not change the results for any sklearn code -- only the syntax used to get those results.

I've got the count_nonzero function working with CSC format and the added sample_weight argument. The tricky part was making CSC work with row-based sample_weights. But with it now working for CSR and CSC, it would be fairly straightforwad to allow sample weights across columns (probably not a normal usecase though). It was a good dive into that feature and could help scipy add a sample_weight feature -- though no promises there... :)

Looks like this version of mypy doesn't allow match/case syntax. I guess that requires mypy under Python 3.10.
I rewrote it with if/else.

OK.. I think this change does what we want. But let's make sure:

Original code: n_missing = imputer_mask.getnnz(axis=0). Need to remove getnnz method.
Option 1: Use sum of mask instead: n_missing = imputer_mask.sum(axis=0)
Option 2: Use index arrays to find nnz by row: code here depends on format, but avoids sum

if imputer_mask.format == "csr":
    n_missing = np.bincount(imputer_mask.indices, minlength=imputer_mask.shape[1])
elif imputer_mask.format == "csc":
    n_missing = np.diff(imputer_mask.indptr)

I've got Option 2 in the PR now. One commit prior has Option 1. They both are passing tests.

If both options have the similar performance, I would go with imputer_mask.sum(axis=0) because it is simpler.

The times are pretty similar -- sum is slower for small arrays, but faster for large arrays. So sum is simpler and faster for the big problems. I've put Option 1 into the code for this PR.

I think this PR deserves a changelog entry.

I added an entry for the changelog -- but someone should check for style and detail level. e.g. I did not state that this should not affect any results produced by the library. Indeed users should not notice anything.

I put the changelog type as "other" type (couldn't find "maint") and in the many_modules part of upcoming changes. But these can be changed...

@lorentzenchr can you explain more why you think a changelog is needed?

My understanding it that this PR is making sure that our internal code is compatible with both sparse matrices and sparse arrays. This should not affect users.

It is a visible improvement preparing us for the removal of sparse matrices. I just think it deserves visibility.

I have tweaked the changelog to follow our guidelines and enabled auto-merge, thanks @dschult!