Conversation
…ture_types fixed to support sparse.COO, tests
eroell
reviewed
Sep 18, 2026
eroell
left a comment
Collaborator
There was a problem hiding this comment.
Please add a benchmark with compute time and memory consumption in the PR comment, where a large dense 3D and a large sparse 3D, and also the 2D sparse and dense arrays are tested.
The sparse COO version should run for a ~500k x 1000k x 90 tensor of about 1% data density.
…arse arrays wo densfiying, harmonize_missing_values accepts only numeric sparse.COO
…islab/ehrdata into fix/harmonize-missing-sparse
…armonize_on_read catches the NotImplementedError, test adjusted
Collaborator
Author
|
Benchmark with compute time and memory usage Used
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #305
Both functions previously either crashed or silently no-op'd on a sparse.COO X/layer. This adds proper support to both, without ever densifying the array.
infer_feature_types_detect_feature_types_sparse_coohelper implements this; infer_feature_types branches to it for sparse.COO input and keeps the existing dataframe-based path unchanged for everything else.harmonize_missing_valuesfill_valuefrom 0 to NaN, so the array stays sparse)varsargument: variable names whose 0 is a real measured value, not missing. Previously-implicit zeros of these columns are materialized as explicit stored 0 entries (so they dont get changed to the new NaN fill value); every other column stays sparseTests: added coverage in
test_feature_types.pyfor 2D/3D sparse.COO harmonization, vars exclusion, an unknown-var error, scipy.sparse being unaffected, and sparse.COO feature-type inference (including binary detection and the all-NaN-column error). Two pre-existing IO round-trip tests (test_h5ed.py/test_zarr.py) updated to passharmonize_missing_values=False, since they test binsparse encoding fidelity specifically and predate this behavior change.