Skip to content

Fix reify to handle sparse arrays and other objects without __len__#12103

Merged
jacobtomlinson merged 8 commits intodask:mainfrom
batcity:issue-11643-fix
Nov 10, 2025
Merged

Fix reify to handle sparse arrays and other objects without __len__#12103
jacobtomlinson merged 8 commits intodask:mainfrom
batcity:issue-11643-fix

Conversation

@batcity
Copy link
Copy Markdown
Contributor

@batcity batcity commented Oct 17, 2025

  • Closes 11643
  • Tests added / passed

Tests for the bags module pass:

image

Tests for the utils module pass:

image
  • Passes pre-commit run --all-files
image

How did I verify that the fix works:

I recreated the test mentioned in the issue above, here's the code:

 if __name__ == "__main__":
    import dask.bag as db
    import numpy as np
    from dask import delayed
    from scipy.sparse import csr_array


    def add(x, y):
        return x + y


    @delayed
    def create_sparse_array_delayed():
        return csr_array(np.random.random((10, 10)))


    @delayed
    def create_array_delayed():
        return np.random.random((10, 10))


    db.from_sequence(
        [csr_array(np.random.random((10, 10))), csr_array(np.random.random((10, 10)))]).fold(
        add).compute()  # works with sparse arrays when created from sequence
    db.from_delayed([create_array_delayed(), create_array_delayed()]).fold(add).compute()  # works with numpy arrays
    print(db.from_delayed([create_sparse_array_delayed(), create_sparse_array_delayed()]).fold(add).compute()) 

This now returns the result instead of the bug:

image

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Oct 17, 2025

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

      9 files  ±  0        9 suites  ±0   3h 15m 8s ⏱️ + 10m 13s
 18 154 tests + 15   16 939 ✅ + 15   1 215 💤 ±0  0 ❌ ±0 
162 614 runs  +135  150 521 ✅ +136  12 093 💤  - 1  0 ❌ ±0 

Results for commit 3ccdaef. ± Comparison against base commit 0fe7582.

♻️ This comment has been updated with latest results.

jacobtomlinson
jacobtomlinson previously approved these changes Oct 23, 2025
Copy link
Copy Markdown
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that our CI environment has scipy installed could you add some tests that use some actual sparse arrays, rather than mocking everything?

@jacobtomlinson jacobtomlinson dismissed their stale review October 23, 2025 11:26

Accidentally clicked approve

Copy link
Copy Markdown
Member

@jacobtomlinson jacobtomlinson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fine to me, but I'd appreciate an additional review from another maintainer before merging this.

cc @TomAugspurger @jrbourbeau @quasiben if you're around

@batcity
Copy link
Copy Markdown
Contributor Author

batcity commented Oct 23, 2025

The latest test failures are unrelated to this PR btw

Copy link
Copy Markdown
Member

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm always looking for ways to reduce the amount of guessing / duck typing we do, but I'm not sure if there's a way around that here. Just one comment about a couple of checks in is_empty that might fail. Otherwise I think this is fine.

@jacobtomlinson jacobtomlinson changed the title issue-11643: Fix reify to handle sparse arrays and other objects without __len__ Fix reify to handle sparse arrays and other objects without __len__ Nov 10, 2025
@jacobtomlinson jacobtomlinson merged commit a66a897 into dask:main Nov 10, 2025
23 of 24 checks passed
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.

3 participants