Closed Kirill888 closed 3 months ago
Thanks for the swift action on this Kirill - I will review tomorrow.
Attention: Patch coverage is 98.88889%
with 1 line
in your changes missing coverage. Please review.
Project coverage is 95.48%. Comparing base (
6af5d0c
) to head (e80f39a
). Report is 33 commits behind head on develop.
Files | Patch % | Lines |
---|---|---|
odc/geo/_xr_interop.py | 96.55% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks heaps for this @Kirill888 - the test cases appear to match the functionality I was hoping for nicely, but I'll do a "user" test today and verify that TIFFs exported using the updated code work as intended (e.g. in ESRI etc).
@robbibt thanks, BTW pleas use pip install odc-geo==0.4.7rc1
for your test (that release is not on conda, only pypi)
I think this is working perfectly. For example, we have a dataset with an Xarray nodata: nan
attribute:
import datacube
import odc.geo.xr
from datacube.utils.cog import write_cog
dc = datacube.Datacube()
query_params = dict(
x=(142.13223, 142.65461),
y=(-32.17591, -32.54618),
time=("2022", "2022"),
)
ds = dc.load(product="ga_ls8cls9c_gm_cyear_3", measurements=["edev"], **query_params)
da = ds.edev.squeeze()
This gets written out with a GeoTIFF nodata flag nodata=nan
using both datacube
and odc-geo
tooling:
da.odc.write_cog("nodata_nan_odcgeo.tif")
write_cog(da, "nodata_nan_datacube.tif")
However, if we write out data without a Xarray nodata: nan
attribute, datacube
doesn't include a GeoTIFF nodata flag, but now odc-geo
does!
del da.attrs["nodata"]
da.odc.write_cog("nodata_missing_odcgeo.tif")
write_cog(da, "nodata_missing_datacube.tif")
I can though return a true missing nodata value like this:
da.odc.write_cog("nodata_truenone_odcgeo.tif", nodata=None)
I do want to do a quick test of the COG overviews stuff, so will approve this as soon as I've finished that.
I think some of the naming is confusing, but the approach is sound.
I also find "MaybeAutoNodata" a bit confusing - took me a while to work out what it meant.
I think some of the naming is confusing, but the approach is sound.
I also find "MaybeAutoNodata" a bit confusing - took me a while to work out what it meant.
What about SomeNodata
instead of MaybeAutoNodata
, @SpacemanPaul @robbibt
I think some of the naming is confusing, but the approach is sound.
I also find "MaybeAutoNodata" a bit confusing - took me a while to work out what it meant.
What about
SomeNodata
instead ofMaybeAutoNodata
, @SpacemanPaul @robbibt
Yes SomeNodata
or even AnyNodata
works for me.
Updated,
Removed no longer relevant MaybeNodata
type, if anyone was using that type, they were probably also using Nodata
type too, and that have changed, so...
use SomeNodata
instead of MaybeAutoNodata
name
add nodata
setter
OK, overview generation is working much better for our DEA Intertidal example: even if nodata
is not set, we still get a result that follows GDAL's expected functionality:
datacube-core
with nodata=nan
Xarray attribute (overviews generated correctly ✔️):
datacube-core
with no nodata Xarray attribute (overviews generated incorrectly ❌):
odc-geo
with nodata=nan
Xarray attribute (overviews generated correctly ✔️):
odc-geo
with no nodata Xarray attribute (overviews generated correctly ✔️):
MaybeAutoNodata
~SomeNodata
--None|int|float|str|"auto"
Nodata
is nowNone|float|int
MaybeNodata
is now ~None|float|int|str
~ have been removedNone
None
now means "no nodata value"resolve_nodata()
is used to handle nodata options consistently across the library.odc.nodata
is used to extractnodata
from xarraysnan
values, and GDAL needsnodata=nan
to handle it correctly.nodata types explained
MaybeAutoNodata
~SomeNodata
should be used in APIs that allow configuring nodata and nodata overridesNodata
is a type suitable to pass to GDAL{src,dst)_nodata
it can beNone
which means expect no missing values in the data.FillValue
is a type suitable fornp.full
and is resolved fromNodata
+dtype
NaN
for not-configured floating point images0
for not-configured integer imagesnodata
if set (NaN
is fine here too)