Export AbstractSparseMatrixCSC, getcolptr, indtype - #767
Conversation
AbstractSparseMatrixCSC and getcolptr are heavily used in LinearSolve.jl. We find uses of getcolptr in Sparspak.jl and Pardiso.jl indtype is a useful complement to eltype which could be extended for subtypes of AbstractSparseArray.
|
Let’s get it done. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #767 +/- ##
==========================================
+ Coverage 84.16% 84.19% +0.03%
==========================================
Files 13 13
Lines 9393 9405 +12
==========================================
+ Hits 7906 7919 +13
+ Misses 1487 1486 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The public getcolptr documentation incorrectly guarantees writability for fixed-sparsity matrices.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Exports key CSC sparse-matrix APIs and documents their use.
Changes:
- Exports
AbstractSparseMatrixCSC,getcolptr, andindtype. - Adds docstrings for
getcolptrandindtype.
File summaries
| File | Description |
|---|---|
src/SparseArrays.jl |
Exports the three APIs. |
src/sparsematrix.jl |
Documents getcolptr. |
src/abstractsparse.jl |
Documents indtype. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suggested by copilot. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Should be ok now. |
|
I am ok with merging this. Let's give it a few days for feedback since we are exporting a couple new things here. |
AbstractSparseMatrixCSC and getcolptr are heavily used in LinearSolve.jl.
We find uses of getcolptr in Sparspak.jl and Pardiso.jl (and yes, in my package ExtendableSparse.jl)
indtype is a useful complement to eltype which could be extended for subtypes of AbstractSparseArray.SparseMatrixCSC
I intend to replace #395 with this PR. My main concern ist "ExplicitImports sanity" of the package ecosystem. IMHO #395 was (perceived as) too ambitious, probably because of the word 'interface' in the title. The present PR doesn't care about "interface", but just pragmatically tries to formalize what is current practice.
As for 'indtype', this is a slightly different discussion, we may just make it public, and I would also agree with keeping it internal, in order to prevent delaying of fixing the situation with AbstractSparseMatrixCSC and getcolptr.