add abstract types for views and permuted arrays #255
Conversation
|
Maybe ReshapedDiskArray as well for completeness? Not sure if Padded/Rechunked etc also need abstrac types |
There was a problem hiding this comment.
I tried using it to implement to solve JuliaGeo/CommonDataModel.jl#32 (comment) and here are my comments
src/permute.jl
Outdated
| Base.parent(a::AbstractPermutedDiskArray) = a.a.parent | ||
| permuteddimsarray(a) = a.a | ||
| Base.size(a::AbstractPermutedDiskArray) = size(permuteddimsarray(a)) | ||
| Base.parent(a::AbstractPermutedDiskArray) = parent(permuteddimsarray(a)) |
There was a problem hiding this comment.
We should merge my other PR first, it actually changes this so there is no separate array. Maybe thats a problem?
There was a problem hiding this comment.
Don't think that's a problem at all! The permuteddimsarray was only necessary to get the perm and iperm anyway.
|
@tiemvanderdeure I fixed the merge with #249, it definately makes the abstract type less useful @lupemba I'm not sure if this will affect you, or if you are subtyping |
I have left out permuted arrays from my PR since CommonDataModel doesn't have specific methods for that yet. So it is not needed right now. I still think we should consider adding a support for permuted arrays in CommonDataModel before we make a breaking release. |
|
@tiemvanderdeure |
|
I think it's basically good to go. Let's see if tests pass now. |
| @@ -41,15 +48,15 @@ function eachchunk(a::PermutedDiskArray) | |||
| # Return permuted GridChunks | |||
| return GridChunks(genperm(gridchunks.chunks, perm)...) | |||
| end | |||
| function DiskArrays.readblock!(a::PermutedDiskArray, aout, i::OrdinalRange...) | |||
| function DiskArrays.readblock!(a::AbstractPermutedDiskArray, aout, i::OrdinalRange...) | |||
| iperm = _getiperm(a) | |||
There was a problem hiding this comment.
This is now an internal method in an Abstract typed function... Should we remove the underscore?
There was a problem hiding this comment.
Remove which underscore? You mean in _getiperm and _getperm
Per @lupemba 's excellent suggesion in JuliaGeo/NCDatasets.jl#274 (comment), this PR implements an AbstractPermutedDiskArray and AbstractSubDiskArray