Skip to content
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

Fix a case of wrong aliasing for setindex #164

Merged
merged 4 commits into from
Apr 15, 2024
Merged

Fix a case of wrong aliasing for setindex #164

merged 4 commits into from
Apr 15, 2024

Conversation

meggart
Copy link
Owner

@meggart meggart commented Apr 2, 2024

This occured in setindex! cases where source and destination shape were not the same, e.g.

a[1:10,2:2] = 1:10

A test was added for this in setindex tests.

@meggart
Copy link
Owner Author

meggart commented Apr 2, 2024

For reference: This is the failing PR over at Zarr.jl JuliaIO/Zarr.jl#140

src/diskarray.jl Outdated Show resolved Hide resolved
src/diskarray.jl Outdated Show resolved Hide resolved
meggart and others added 2 commits April 3, 2024 12:58
Co-authored-by: Rafael Schouten <[email protected]>
Co-authored-by: Rafael Schouten <[email protected]>
@meggart
Copy link
Owner Author

meggart commented Apr 3, 2024

And in general, I think it would be good to increase the number of unit tests for setindex!. A lot of work has gone into thorough testing of getindex operations and the resolution of indices goes through the same machinery for both, but some case are different, especially since the user supplies 2 arrays instead of one some rules need to be obeyed, so there are numerous combinations of additional or missing trailing indices of singleton dimensions to test, both for the array to be written to and for the array containing the data to write.

@meggart meggart merged commit c926307 into main Apr 15, 2024
12 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.

2 participants