-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Incorrect filtering of subarrays #4763
Comments
The issue seems to be that we're missing an implementation of logical indexing for SubArrays. It's being interpreted as |
@timholy beat me to it. The problem is that filter creates a boolean array that is used for indexing. Subarray does not support a boolean array for indexing. The method that gets invoked for boolean array indexing is function getindex{T,S<:Integer}(s::SubArray{T,1}, I::AbstractVector{S}) because Bool is a subtype of integer. I think this will be fixed if we define function getindex{T,S<:Integer}(s::SubArray{T,1}, I::AbstractVector{Bool}) |
Agreed. I'll let you beat me this time, if you're game :-). |
I'll give it a try, but probably it will not be good enough for Stefan. |
And I will not have time to read |
I'm not sure where I got this stern reputation from. Is it the viking horns? |
Yeah, horns are awesome :) |
Tim and Ivar, thank you for pointing out the root of the problem. |
@StefanKarpinski I mentioned you because I often see you saying NO to slow implementations and new language features with too specific use cases. Too me it looks like you're doing a terrific job to make the language pleasant, consistent and easy to learn. The subarray code is also performance critical, so it is highly likely that there is a better way to implement it than what I would end up with. |
Ah, yes. Saying "no" is definitely a huge part of my job :-) The current SubArray code is not exactly immaculate, so I'm a little less protective of it. The whole thing is badly in need of an overhaul. |
Implement logical indexing for SubArray. Fixes #4763
filter
produces incorrect output for subarrays:The text was updated successfully, but these errors were encountered: