-
Notifications
You must be signed in to change notification settings - Fork 36
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
Refactor using internal wrappers #168
Conversation
Codecov Report
@@ Coverage Diff @@
## master #168 +/- ##
===========================================
+ Coverage 74.45% 85.78% +11.33%
===========================================
Files 21 21
Lines 1229 1379 +150
===========================================
+ Hits 915 1183 +268
+ Misses 314 196 -118
Continue to review full report at Codecov.
|
This is passing and pretty much finished besides some tests for @mauro3, @ali-ramadhan and @vlandau if you have any time to look over the code (there's a lot so I can't ask for a full review), or could just switch to using this branch that would be a big help. @meggart DiskArrays.jl has been hugely helpful in this PR, thanks for all the work you put into it. I mostly resolved meggart/DiskArrays.jl#28 using Flatten.jl to re-wrap an opened object with the DiskArray wrappers. So |
Cool! 🎉 |
This PR is a massive refactor of GeoData.jl internals, structuring things how I always wanted to but hadn't worked out how to do in practice initially.
In its final form this should have minimal impact on use, but a large impact on maintainability, clarity and consistency.
Notably, nearly all the code deletions are actually code, and the additions are mostly extra tests. We should have about 800 lines less code in the packege after this.
Notable changes:
AbstractGeoStack
is now<: AbstractDimStack
. This means the Tables.jl interface will work as well as a lot of base array methods applied to all layers, and the packages are consistent again (this was not easy)FileArray/FileStack
are used as internal wrappers instead of raw strings. This is the main thing that allowed removing so much code and standardizing everything further.read(A)
can be used to transfer data to memory - forGeoArray
,GeoStack
andGeoSeries
- to manually break laziness. UsingGeoArray(A)
no longer does this - it may still may be lazy internally.NCDarray/NCDstack/GRDarra/GDALarray
etc types are all gone. We can add aliases to them to keep code working for a while, but they are no longer needed. Everything is just aGeoArray
orGeoStack
and file loading is specified by the wrapped object.open
do block. This can use broadcast etc facilitated by DiskArrays.jlcrop
,extend
methods allow resizing multiple arrays/stacks to match dimensionsslice
allows lazily slicing arrays/stacks to series. This is very useful for standardising accross datasets where e.g. the time dimension may be within a file (e.g. netcdf) or between multipole files (e.g. tif). It will also allow distributed out of core processing as the slices are just string pointers to the files with the metadata to open them.TODO
Closes #136
Closes #134
Closes #64
Closes #70