Thank you @thomasjpfan! I think I have fixed the PR.

One observation is that MeanShift.fit_predict(X) won't work with sparse input. It defaults to the superclass fit_predict then yo-yos back down to MeanShift.fit(X), which doesn't work with sparse input. I'm not sure if this is an issue.

I'm not sure if this is an issue.

It can be an issue. fit does not support sparse matrices, so having predict support sparse matrices is a bit inconsistent.

I am okay with predict supporting sparse matrices while fit does not. I think another maintainers opinion would help see if we want this change.

Thank you!

If the team decides against the change then we need to update the documentation. Currently the doc says MeanShift.predict accepts X : {array-like, sparse matrix}.

I don't really like the inconsistency. It is unlikely to fit a dense X and predict a sparse X with such clustering.
I would prefer to update the documentation stating that we only support array-like.

@milana2 Could you revert the changes and just update the documentation.

I don't really like the inconsistency. It is unlikely to fit a dense X and predict a sparse X with such clustering. I would prefer to update the documentation stating that we only support array-like.

I am okay with this option. @milana2 Did you have a use case, where you fit on a dense X and predict on a sparse X?

@milana2 Could you revert the changes and just update the documentation.

Will do.

@milana2 Did you have a use case, where you fit on a dense X and predict on a sparse X?

We don't have a use case, no. (While working on a project, we noticed inconsistencies in the handling of sparse data and we ran an analysis to look for inconsistencies between code and documentation. We have reported two of the cases we found. There are others. I can open an Issue with the full list if you think that would be useful?)

Thanks @milana2 Looks good.