pratyushadk commented on issue #50906:
URL: https://github.com/apache/arrow/issues/50906#issuecomment-5396014542
Thanks for the question !
**The API convenience scenario**
The issue surfaces directly through PyArrow's public API.
`SparseCSRMatrix.from_dense_numpy()` internally calls `from_tensor()`, which
routes to `TensorToSparseCSRMatrix` in `numpy_convert.cc`, which calls
`SparseCSRMatrix::Make`, which reaches the `NotImplemented("TODO for ndim <=
1")` in `csx_converter.cc`. The full verified chain:
```
pa.SparseCSRMatrix.from_dense_numpy(np.array([1, 0, 2, 0, 0, 3]))
→ from_tensor() [pyarrow/tensor.pxi:841]
→ TensorToSparseCSRMatrix() [python/numpy_convert.cc:550]
→ SparseCSRMatrix::Make() [sparse_tensor.cc:101]
→ MakeSparseCSXMatrixFromTensor() [tensor/csx_converter.cc]
→ NotImplementedError: TODO for ndim <= 1
```
A user passing a 1D numpy array hits `NotImplementedError: TODO for ndim <=
1` — a message that reads as an unfinished internal placeholder, not an
intentional design boundary.
**Why a user would try this**
Arrow has explicit scipy interop via `from_scipy()` and `to_scipy()`. scipy
itself accepts 1D arrays for CSR and treats them as single-row matrices:
```python
import scipy.sparse
scipy.sparse.csr_matrix([1, 0, 2, 0, 0, 3]) # works fine, shape (1, 6)
```
A user migrating a scipy workflow to Arrow, or writing generic code over
sparse formats, would reasonably call `from_dense_numpy` on a 1D array and
expect it to work. Arrow's own `SparseCOOTensor.from_dense_numpy()` already
accepts 1D tensors without error (test:
`TestSparseCOOTensor::CreationFromNumericTensor1D`), so the inconsistency is
surprising from within Arrow itself too.
**What this PR does**
It treats a 1D vector as a degenerate single-row CSR matrix with `indptr =
[0, nnz]`, which is exactly what scipy does. The implementation is a strict
simplification of the existing 2D path — a single linear scan, no new
abstractions, no changes to the hot 2D loop.
If this is still considered out of scope, the minimum fix would be replacing:
```cpp
return Status::NotImplemented("TODO for ndim <= 1");
```
with:
```cpp
return Status::Invalid("SparseCSR/CSC only supports 2D tensors");
```
so the error is at least intentional and communicates a clear design
decision to users rather than reading like an incomplete TODO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
