-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BlockSparseArrays] Improve the design of block views #1481
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1481 +/- ##
==========================================
+ Coverage 43.65% 44.14% +0.49%
==========================================
Files 136 144 +8
Lines 8806 9374 +568
==========================================
+ Hits 3844 4138 +294
- Misses 4962 5236 +274 ☔ View full report in Codecov by Sentry. |
@ogauthe merging this now. Please try it out and report any issues you come across. |
Nice to see this |
This fixes a few issues related to block views listed in #1336. The strategy here is to change the design such that calls like
@view a[Block(1, 1)]
return something likeSubArray(a, Block(1, 1))
, while before this PR it returned the block of theBlockSparseArray
directly, which causes problems when getting views of blocks that aren't stored. There are still issues to work out with that new design but they should be easy to fix.To summarize, this fixes:
a[Block(2, 2)] .= 1
ora[Block(2, 2)] .= randn(2, 2)
.@view(a[Block(1, 1)])[1:1, 1:2] = rand(1, 2)
or@views a[Block(1, 1)][1:1, 1:2] .= rand(1, 2)
.a[Block(2), Block(2)] = randn(3, 3)
(before onlya[Block(2, 2)] = randn(3, 3)
worked).a[Block(1, 1)] = randn(3, 3)
ofsize(a[Block(1, 1)]) != (3, 3)
.