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

Add simple pydata/sparse dispatch #29031

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mtsokol
Copy link
Contributor

@mtsokol mtsokol commented May 16, 2024

Hi!

This PR adds a simple scipy.sparse dispatch for https://github.com/pydata/sparse (transforming pydata/sparse arrays to scipy.sparse matrices).

Same efforts to support pydata/sparse input and convert to scipy.sparse were completed in scipy.sparse.linalg and scipy.sparse.csgraph:

Copy link

github-actions bot commented May 16, 2024

✔️ Linting Passed

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

Generated for commit: 205af1b. Link to the linter CI: here

@mtsokol mtsokol force-pushed the pydata-sparse-dispatch branch 3 times, most recently from 3ebf683 to 9de215c Compare May 16, 2024 17:21
@mtsokol
Copy link
Contributor Author

mtsokol commented May 17, 2024

I think I need some guidance which specific files I should modify to add sparse test dependency.

@betatim
Copy link
Member

betatim commented May 17, 2024

Thanks for making a PR!

Before working more on the code side I think it would be worth expanding on the use-case and motivation for this change. Maybe there is some document or blog post that explains how this fits into the whole PyData ecosystem. The reason I am saying all this is that adding a new dependency to scikit-learn is one of the hardest things to do. The default answer is "no" - and even if it isn't a straight no, then there are a lot of questions and discussions. So if you have some links to things people can read, find answers and inform the debate that would be great.

@glemaitre
Copy link
Member

In addition to @betatim, I would mention that we recently add support for scipy sparse arrays in addition to the scipy sparse matrices. From what I foresee in the long term is that the scipy sparse arrays implementation should allow us to remove any specific sparse Python code (not the specialize Cython one).

So I'm wondering if this is wise to add support for another sparse container that go away from this goal while we could request our user to make the conversion before to provide us the data, isn't it?

@hameerabbasi
Copy link

Thanks everyone, I've opened a discussion thread over here: #29064

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants