-
Notifications
You must be signed in to change notification settings - Fork 37
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
Breaking: Standardise cf standards and missingval handling #695
Conversation
How about inverting it? Instead of "cf" call it "raw". If raw=false, offset and scale are applied, if raw=true, offset and scale are NOT applied? And raw=true would be the default? Maybe "navalue" instead of "maskingval"? |
Currently I've got Main concern with |
Yes, you are right that with "raw" one may expect that data were not touched at all. Indeed, "navalue" originates from my heavy R usage! And I agree NA is not very julia way. Point taken:) |
Thanks for the input though, good to have all the options to choose from |
Coming back to this I like how short and obvious But to mean no scaling and no masking - so it will override everything else. |
ext/GeometryOpsDimensionalDataExt/GeometryOpsDimensionalDataExt.jl
Outdated
Show resolved
Hide resolved
|
||
""" | ||
create([f!], [filename], template; kw...) |
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.
When should I use this as opposed to Raster(data, dims; ...)
? What are the advantages beyond efficiency in writing to file?
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.
Or is the direct constructor going to call create
?
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.
Well Raster(filename)
reads an existing file... create(filename)
creates one.
So this is best for building something new from scratch, Raster
is best for opening something, rewrapping a DimArray, etc. Otherwise you need to define the Raster in-memory then write
it as separate steps.
(create
is already called in a lot of places under the hood where a new file might be created, like in nearly all methods with a filename
keyword. It's been here for years, just not publicly)
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.
Gotcha, I guess I'm not entirely clear on what to use when creating a purely in memory dataset. Does the pure Raster(data, dims; kwargs...)
constructor have the same options? Should it? That's the most intuitive option IMO to create an in memory raster...
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.
Yeah it will be pretty much identical for in-memory rasters. But if you want chunks and e.g. compression options you have to do it separately in write
. An in-memory raster doesn't have chunks or options like that.
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.
Sweet that sounds good. Will add a suggestion for a line in the docstring to clarify.
Co-authored-by: Anshul Singhvi <[email protected]>
This fixes a strange issue I was having where Rasters.jl read the wrong values from a Kerchunk/Zarr dataset.
* Breaking: Standardise cf standards and missingval handling (#695) * start adding cf keyword * add modifieddiskarray file * use scale and offset keywords * bugfix * bugfix * updates * create * tweaks * bugfixes * bugfix GRIB * bugfixes * add raw keyword * clean up extensions * updates * mosaic fixes and standardisation * bugfixes * closured in create * bugfixes * fix resample test for nokw * test create with a function * Apply suggestions from code review Co-authored-by: Anshul Singhvi <[email protected]> * Hook up scale and offset to Zarr datasets via CDM (#732) This fixes a strange issue I was having where Rasters.jl read the wrong values from a Kerchunk/Zarr dataset. * use coalesceval instead of maskingval * combine missingval and maskingval * one missingval * fixes * fix mosaic * tweaks and bugfixes * bugfix everything * fix aqua * fix netcdf metadata * docs tweaks * typo * bugfix and tweak docs * fix extension doctests * fix warp missingval * bugfix missingval tests * remove ext --------- Co-authored-by: Anshul Singhvi <[email protected]> * Breaking: Faster/better `aggregate` (#763) * optimise aggregate * specify DiskArrays.cache * cleanup * bugfix * bugfix * bugfix * tets more syntax * rework the meaning of Colon in aggregation * bugfix * comments * more aggregate tests * ismem is just the opposite of isdisk * fix ag test * fixes for disk * bugfix * bugfix gdal tests * Faster `mosaic` (#839) * faster mosaic * bugfix * remove comment * tweaks * cleanup and reorganise * add missing deps * Breaking: Line and Polygon `extract` optimisation (#824) * faster line extract * performance * extract in a separate file * flatten * bugfixes * some more comments * more perf tweaks * bugfix * fix extract ambiguities * more ambiguity * Bugfix geomtrait * arg fixes * add id column * tests and bugfixes * bugfix * set id default to _False() * fix tests * bugfix * bugfix * half fix ncdatasets mosaic * working LinerSolver with tests * fix tests * bugfix doctests * bugfix atol * bugfix create * bugfix tests * pad float intervals before view for fp error * bugfix lazy reorder * remove a # * bugfixes * remove show * fix example * fix examples * fix tests again * fix ambiguities * using Dates in examples * plots dep in examples * fix docs fp changes --------- Co-authored-by: Anshul Singhvi <[email protected]>
This PR standardises cf and missingval behaviour accross all sources.
New keywords are:
cf=true
: applyoffset
andscale
if the are not zero and one.maskingval=missing
this is the value the missingval will be converted to if there is one.I don't like either of these names! if you can think of anything better please suggest something.
@felixcremer @meggart this is the new diskarrays object that does 2-way cf and missingval/maskingval conversions
https://github.com/rafaqz/Rasters.jl/compare/cf?expand=1#diff-1478a88a8bdcffcf7f556cc1865dbe18d69b5c03f75b4481e1f5889e8a6b6b73