changeset 1517:7ee7df7d9b61 feature/grids/componentview

Remove review comments
author Jonatan Werpers <jonatan@werpers.com>
date Thu, 21 Mar 2024 07:43:48 +0100
parents 8d64f8981bb0
children 0cd6cf62af93
files src/Grids/grid.jl test/Grids/grid_test.jl
diffstat 2 files changed, 1 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/src/Grids/grid.jl	Thu Mar 21 07:42:19 2024 +0100
+++ b/src/Grids/grid.jl	Thu Mar 21 07:43:48 2024 +0100
@@ -42,12 +42,6 @@
 
 componentview(gf, component_index...) = ArrayComponentView(gf, component_index)
 
-# REVIEW: Should this only be defined for vector-valued component types of the same dimension?
-# Now one can for instance do:  v = [rand(2,2),rand(2,2), rand(2,1)] and cv = componentview(v,1,2)
-# resulting in #undef in the third component of cv.
-# RESPONSE: I don't think it's possible to stop the user from
-# doing stupid things. My inclination is to just keep it simple and let the
-# user read the error messages they get.
 struct ArrayComponentView{CT,T,D,AT <: AbstractArray{T,D}, IT} <: AbstractArray{CT,D}
     v::AT
     component_index::IT
@@ -60,11 +54,7 @@
 
 Base.size(cv) = size(cv.v)
 Base.getindex(cv::ArrayComponentView, i::Int) = cv.v[i][cv.component_index...]
-Base.getindex(cv::ArrayComponentView, I::Vararg{Int}) = cv.v[I...][cv.component_index...] #REVIEW: Will this allocate if I... slices v? if so, we should probably use a view on v?
-# RESPONSE: I imagine the values of `cv` will be small static vectors most of
-# the time for which this won't be a problem. I say we cross that bridge when
-# there is an obvoius need. (Just slapping a @view on there seems to be
-# changing the return tyoe to a 0-dimensional array. That's where i gave up.)
+Base.getindex(cv::ArrayComponentView, I::Vararg{Int}) = cv.v[I...][cv.component_index...]
 IndexStyle(::Type{<:ArrayComponentView{<:Any,<:Any,AT}}) where AT = IndexStyle(AT)
 
 # TODO: Implement setindex!?
--- a/test/Grids/grid_test.jl	Thu Mar 21 07:42:19 2024 +0100
+++ b/test/Grids/grid_test.jl	Thu Mar 21 07:43:48 2024 +0100
@@ -91,7 +91,6 @@
     end
 
     @testset "components" begin
-        # REVIEW: I think we can reduce the index ranges.
         v = [@SMatrix[1 3; 2 4] .+ 100*i .+ 10*j for i ∈ 1:3, j∈ 1:4]
 
         @test ArrayComponentView(v, (1, 1))  == [1 .+ 100*i .+ 10*j for i ∈ 1:3, j∈ 1:4]