-
Notifications
You must be signed in to change notification settings - Fork 38
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
fix common data model chunk propagation, source type propagation, and other things #592
Conversation
@@ -14,3 +12,7 @@ function RA._open(f, ::Type{GRIBsource}, filename::AbstractString; write=false, | |||
ds = GRIBDatasets.GRIBDataset(filename) | |||
RA._open(f, GRIBsource, ds; kw...) | |||
end | |||
|
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.
@tcarion @Alexander-Barth this is the hack that is currently required to make disk arrays chunking propagate out from the internal Varable
or DataValues
objects.
Note that with your current implementation this means anyone using CFVariable
can not do chunked reads of large datasets. The whole thing is read at once.
src/sources/commondatamodel.jl
Outdated
DiskArrays.haschunks(var::CFVariable) = _get_haschunks(var.var.var) | ||
|
||
function _get_eachchunk end | ||
function _get_haschunks end |
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.
@tcarion @Alexander-Barth this is where the hack is called to dig into the inner array that knows the actual chunk pattern. With the current design of NCDatasets.jl/GRIBDatasets.jl/CommonDataModel.jl anyone wanting chunking to work with CFVariable will need to do a hack like this.
(Note this is the Rasters.CFVariable
, not type piracy)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #592 +/- ##
==========================================
- Coverage 82.24% 81.97% -0.27%
==========================================
Files 61 61
Lines 4257 4245 -12
==========================================
- Hits 3501 3480 -21
- Misses 756 765 +9 ☔ View full report in Codecov by Sentry. |
This PR fixes a bunch of minor issues with the CommonDataModel.jl conversion.
FileArray
object so online files without extensions were reverting to gdal (but was not tested because we had reliability problems in the tests)_ncd
prefix in the commondatamodel.jl file.I'll need to write a for more tests to lock this down.