Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Jan 26, 2021

Fixes #37274 by choosing an appropriate starting linear index for OffsetVectors using their axes.

Now

julia> a = OffsetVector(1:3, 0:2);

julia> b = @view oa[0];

julia> b[]
1

julia> b[1]
1

This solution is inspired by #39393

@mbauman
Copy link
Member

mbauman commented Jan 26, 2021

Juuust outside the online reviewable window is a comment that should now be removed:

https://github.com/JuliaLang/julia/pull/39404/files#diff-297c38464535b62f620e5b08e977417d289ac88e0af2a101a9fba46e3107f2e7R391

@mbauman
Copy link
Member

mbauman commented Jan 26, 2021

Eh, easier to actually do the edits than try to make that link work. Hah.

@mbauman
Copy link
Member

mbauman commented Jan 26, 2021

I must confess I'm a little surprised this hasn't cropped up before. Unlike #39393, this hits far more code paths than just a no-index getindex. Do we have workarounds in the Offset implementations? Or are we missing some test coverage?

@jishnub
Copy link
Member Author

jishnub commented Jan 27, 2021

This has been a bug in OffsetArrays for a while, and had almost certainly been missed there because of inadequate test coverage.

@jishnub jishnub closed this Jan 31, 2021
@jishnub jishnub reopened this Jan 31, 2021
@mbauman mbauman added backport 1.6 Change should be backported to release-1.6 bugfix This change fixes an existing bug labels Feb 1, 2021
@mbauman mbauman merged commit f2a2637 into JuliaLang:master Feb 1, 2021
@jishnub jishnub deleted the linearindexing branch February 1, 2021 18:00
KristofferC pushed a commit that referenced this pull request Feb 2, 2021
* Fix linear indexing for views of OffsetVectors

* update comments

Co-authored-by: Matt Bauman <[email protected]>
(cherry picked from commit f2a2637)
@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Feb 5, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
* Fix linear indexing for views of OffsetVectors

* update comments

Co-authored-by: Matt Bauman <[email protected]>
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
* Fix linear indexing for views of OffsetVectors

* update comments

Co-authored-by: Matt Bauman <[email protected]>
staticfloat pushed a commit that referenced this pull request Dec 23, 2022
* Fix linear indexing for views of OffsetVectors

* update comments

Co-authored-by: Matt Bauman <[email protected]>
(cherry picked from commit f2a2637)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This change fixes an existing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zero-dimensional views of AbstractVectors assume that the parent is 1-indexed

4 participants