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.

Sign up for GitHub

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

Jump to bottom

Can getindex(A::AbstractArray, inds::AbstractArray) result be a view? #47078

Closed
nalimilan opened this issue Oct 6, 2022 · 2 comments · Fixed by #47099
Closed

Can getindex(A::AbstractArray, inds::AbstractArray) result be a view? #47078

nalimilan opened this issue Oct 6, 2022 · 2 comments · Fixed by #47099
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation

Comments

@nalimilan
Copy link
Member

It's not clear whether the AbstractArray interface allows implementations to define getindex(A::AbstractArray, inds::AbstractArray) so that it returns a view of A that is affected by later mutation of A or inds. This lead to a bug in DataFrames as filter assumes that inds can be emptied, which had the unintended effect of emptying the view (JuliaData/DataFrames.jl#3192). This will be fixed soon in DataFrames anyway by copying inds, but it would be good to clarify what is expected by the interface and adjust either the manual or the filter implementation.
@nalimilan nalimilan added docs This change adds or pertains to documentation arrays [a, r, r, a, y, s] labels Oct 6, 2022
@nalimilan nalimilan mentioned this issue Oct 6, 2022
Merged
@timholy
Copy link
Member

timholy commented Oct 6, 2022

IMO the answer is that getindex should never return a view, but if it's not specified I agree that's worth specifying.

@Moelf Moelf mentioned this issue Oct 8, 2022
Merged
@Moelf Moelf mentioned this issue Aug 31, 2023
Open
@3f6a
Copy link

3f6a commented Sep 1, 2023

getindex should copy. A view can be explicitly requested with @views, view, etc.

timholy added a commit that referenced this issue Sep 17, 2023
@Moelf @nalimilan @timholy
mention getindex() shouldn't return a "view" (#47099)
ee371a1 
fix  #47078

---------

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
Co-authored-by: Tim Holy <tim.holy@gmail.com>
NHDaly pushed a commit that referenced this issue Sep 20, 2023
@Moelf @nalimilan
@timholy @NHDaly
mention getindex() shouldn't return a "view" (#47099)
4f98dc1 
fix  #47078

---------

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
Co-authored-by: Tim Holy <tim.holy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arrays [a, r, r, a, y, s] docs This change adds or pertains to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

mention getindex() shouldn't return a "view" Moelf/julia
3 participants
@nalimilan @timholy @3f6a