-
-
Notifications
You must be signed in to change notification settings - Fork 131
feat: Add sparse.interp function for numba backend #903
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A large part of this PR LGTM. I have just one overarching comment: The tests should look more like the tests in test_coo.py.
|
@hameerabbasi Thanks, but can you be a bit more specific? Do I understand correctly that you want a test that
|
That's correct, yes. |
|
You can also move the test to |
* Add shortcut for dense data to spare.interp * Move test to test_coo.py * Adjust test to compare sparse.interp to numpy.interp * Add test case for complex data * Use random test data
|
@hameerabbasi I moved the test to Let me know if there is anything else I can do. |
|
Thanks @peanutfun, I appreciate the addition! Just waiting for CI. Would you like a new release? |
|
@peanutfun The doctest and one other test need updates; would you be willing to make those changes? |
Head branch was pushed to by a user without write access
|
@hameerabbasi I hope I fixed it now! A new release would be awesome, but it's not urgent for me. |
|
@peanutfun Small doctest failure still remaining: https://github.com/pydata/sparse/actions/runs/19098274651/job/54563742981?pr=903#step:5:12738 |
|
@hameerabbasi Indeed! Apparently I did not run the doctests. How about now? π€ |
CodSpeed Performance ReportMerging #903 will degrade performances by 33.06%Comparing Summary
Benchmarks breakdown
|
|
That worked, thanks @peanutfun! |
|
Thanks for the support! This was my first contribution here, and it was a very smooth and pleasant experience π |
What type of PR is this? (check all applicable)
Related issues
Checklist
This adds
sparse.interp, the sparse equivalent tonumpy.interp. The latter does not work on sparse array types, so I decided to implement a "wrapper". It effectively callsnumpy.interpon the array data and its fill value and returns the original data type.This function should integrate nicely because numpy will automatically dispatch any call to
numpy.interptosparse.interp, if one of the arguments is a sparse array. This is explicitly tested.