Skip to content

Respect sparse_threshold for pandas SparseDtype columns - #537

Open
Danila Pechenev (Danila-Pechenev) wants to merge 2 commits into
Quantco:mainfrom
Danila-Pechenev:fix/378-pandas-sparse-density
Open

Respect sparse_threshold for pandas SparseDtype columns#537
Danila Pechenev (Danila-Pechenev) wants to merge 2 commits into
Quantco:mainfrom
Danila-Pechenev:fix/378-pandas-sparse-density

Conversation

@Danila-Pechenev

Copy link
Copy Markdown

Fixes #378.

Summary

Update from_df so pandas SparseDtype columns respect sparse_threshold
instead of being unconditionally routed to sparse storage.

Why

Issue #378 predates the current Narwhals-based from_df implementation.
In current main, pandas sparse columns are handled separately because their
dtype is not supported by Narwhals, but this special case bypasses
sparse_threshold.

The updated logic preserves logical non-zero density semantics while avoiding
unnecessary scans for low-density zero-fill sparse columns.

For zero-fill SparseDtype columns, pandas sparse storage density is used as a
fast first check. If it is already at or below the threshold, the column is
known to be sparse without scanning its logical values. If it is above the
threshold, logical density is evaluated so explicitly stored zeros are handled
correctly.

For non-zero fill values, classification uses logical non-zero density because
pandas storage density does not represent non-zero density in that case.

Empty and missing-fill sparse columns preserve the existing sparse behavior.

Tests

Added regression coverage for:

  • zero-fill sparse columns below and above sparse_threshold;
  • density exactly at the threshold;
  • explicitly stored zeros;
  • non-zero fill values where storage and logical densities disagree;
  • empty pandas sparse columns;
  • missing fill values.

Checklist

  • Added a CHANGELOG.rst entry

@MarcAntoineSchmidtQC

Copy link
Copy Markdown
Member

When we wrote this function, the intention was that sparse_threshold would be used to sparsify columns and not densify already sparse ones. The documentation is not clear on this, so I understand why this PR was made.

In my mind, users that pass in sparse columns already have checked that it made sense for their column to be sparse, so we don't want to overwrite this.

When looking at #378, the issue had actually been fixed by moving to narwhal. Bypassing the density check to keep the sparse columns sparse solved the inefficiency.

I see 3 questions/observations from this PR:

  1. Should we densify an already sparse column that is below the sparse_threshold?
  2. Tabmat's SparseMatrix is intended to work with zero-filled sparse arrays and nothing else. This PR made me realize that we don't deal with those cases. So we should check that the fill value is always zero.
  3. The SparseMatrix creation itself is quite inefficient right now when the input data is a pandas SparseArray. Should we improve this? (not addressed in this PR but related).

Martin Stancsics (@stanmart), I would love to get your input here. What do you think?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve performance of tabmat.from_pandas for sparse columns

2 participants