-
Notifications
You must be signed in to change notification settings - Fork 6
Make AbstractVariable a subtype of AbstractDiskArray #35
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
Changes from all commits
81f24cd
a30bf71
2e06265
7c382b8
0321c61
c6cb2f0
f9df8eb
1edfc96
7232d96
1472682
5159cb7
e9f6418
ef58671
f47f24b
a03812e
5d1a5e8
f31d810
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -279,7 +279,7 @@ function Base.broadcasted(::GroupedVariableStyle,f::Function,A::GroupedVariable{ | |
| A.v,A.coordname,A.group_fun,A.groupmap,A.dim,map_fun) | ||
| end | ||
|
|
||
| function GroupedVariable(v::TV,coordname,group_fun::TF,groupmap,dim,map_fun::TM) where TV <: AbstractVariable where {TF,TM} | ||
| function GroupedVariable(v::TV,coordname,group_fun::TF,groupmap,dim,map_fun::TM) where TV <: Union{AbstractVariable,SubVariable} where {TF,TM} | ||
| TGM = typeof(groupmap) | ||
|
|
||
| #TG = Base.return_types(selectdim,(TV,Int,Int,))[1] | ||
|
|
@@ -361,7 +361,7 @@ close(ds) | |
| ``` | ||
|
|
||
| """ | ||
| function groupby(v::AbstractVariable,(coordname,group_fun)::Pair{<:SymbolOrString,TF}) where TF | ||
| function groupby(v::Union{AbstractVariable,SubVariable},(coordname,group_fun)::Pair{<:SymbolOrString,TF}) where TF | ||
| # for NCDatasets 0.12 | ||
| c = v[String(coordname)][:] | ||
| class = group_fun.(c) | ||
|
|
@@ -462,6 +462,9 @@ Base.BroadcastStyle(::Type{<:ReducedGroupedVariable}) = ReducedGroupedVariableSt | |
| Base.BroadcastStyle(::DefaultArrayStyle,::ReducedGroupedVariableStyle) = ReducedGroupedVariableStyle() | ||
| Base.BroadcastStyle(::ReducedGroupedVariableStyle,::DefaultArrayStyle) = ReducedGroupedVariableStyle() | ||
|
|
||
| Base.BroadcastStyle(::DiskArrays.ChunkStyle,::ReducedGroupedVariableStyle) = ReducedGroupedVariableStyle() | ||
| Base.BroadcastStyle(::ReducedGroupedVariableStyle,::DiskArrays.ChunkStyle) = ReducedGroupedVariableStyle() | ||
|
|
||
| function Base.similar(bc::Broadcasted{ReducedGroupedVariableStyle}, ::Type{ElType}) where ElType | ||
| # Scan the inputs for the ReducedGroupedVariable: | ||
| A = find_gv(ReducedGroupedVariable,bc) | ||
|
|
@@ -587,3 +590,16 @@ function dataset(gr::ReducedGroupedVariable) | |
| gr.reduce_fun, | ||
| ) | ||
| end | ||
|
|
||
| # ReducedGroupedVariable is a Variable and therefore an AbstractDiskArray. | ||
| # getindex is overloaded for ReducedGroupedVariable and therefore | ||
| # ReducedGroupedVariable is defined to use getindex. | ||
| # An alternative solution is to remove getindex definitions | ||
| # and then implement the read logic in readblock! | ||
| function DiskArrays.readblock!(gr::ReducedGroupedVariable{T,N}, | ||
| aout, | ||
| indexes::Vararg{OrdinalRange, N}) where {T,N} | ||
|
|
||
| aout .= Base.getindex(gr,indexes...) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently |
||
| return aout | ||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -70,10 +70,18 @@ end | |||||
|
|
||||||
| Base.parent(v::MemoryVariable) = v.data | ||||||
| Base.size(v::MemoryVariable) = size(parent(v)) | ||||||
| Base.getindex(v::MemoryVariable,ij::TIndices...) = parent(v)[ij...] | ||||||
| function Base.setindex!(v::MemoryVariable,data,ij...) | ||||||
|
|
||||||
| function DiskArrays.readblock!(v::MemoryVariable{T, N}, | ||||||
| aout, | ||||||
| indexes::Vararg{OrdinalRange, N}) where {T, N} | ||||||
|
|
||||||
| aout .= parent(v)[indexes...] | ||||||
| end | ||||||
|
|
||||||
|
|
||||||
| function DiskArrays.writeblock!(v::MemoryVariable{T, N}, data, indexes::Vararg{OrdinalRange, N}) where {T, N} | ||||||
| sz = size(v) | ||||||
| parent(v)[ij...] = data | ||||||
| parent(v)[indexes...] = data | ||||||
|
|
||||||
| root = _root(v) | ||||||
| for idim = findall(size(v) .> sz) | ||||||
|
|
@@ -83,6 +91,19 @@ function Base.setindex!(v::MemoryVariable,data,ij...) | |||||
| return data | ||||||
| end | ||||||
|
|
||||||
| function DiskArrays.writeblock!(v::MemoryVariable{T, 0}, data) where {T} | ||||||
| sz = size(v) | ||||||
| parent(v)[] = data[] | ||||||
|
|
||||||
| root = _root(v) | ||||||
| for idim = findall(size(v) .> sz) | ||||||
| dname = v.dimnames[idim] | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There is a bunch of direct field access in this PR for fields that have interface methods
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this code is simply copied from the I don't think we should expand the scope of this PR anymore.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, it just looks like new code to me and you use assessor methods everywhere else... |
||||||
| grow_unlimited_dimension(v.parent_dataset,dname,size(v,idim)) | ||||||
| end | ||||||
| return data | ||||||
| end | ||||||
|
|
||||||
|
|
||||||
| CDM.load!(v::MemoryVariable,buffer,ij...) = buffer .= view(parent(v),ij...) | ||||||
| CDM.name(v::Union{MemoryVariable,MemoryDataset}) = v.name | ||||||
| CDM.dimnames(v::MemoryVariable) = v.dimnames | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReducedGroupedVariableuse different broadcasting thanDiskArrays. This might lead to some confusion.