search

opendatacube / datacube-core

Open Data Cube analyses continental scale Earth Observation data through time
http://www.opendatacube.org
Apache License 2.0
493 stars 175 forks source link

Use odc-geo GridSpec model internally #1545

Closed snowman2 closed 3 months ago

snowman2 commented 5 months ago

Testing out 1.9.0rc1:

Related https://github.com/opendatacube/datacube-core/pull/1424

 /opt/venv/lib/python3.11/site-packages/datacube/model/__init__.py:567: ODC2DeprecationWarning: Call to deprecated class GridSpec. (This version of GridSpec has been deprecated. Please use the GridSpec class defined in odc-geo.) -- Deprecated since version 1.9.0.
    return GridSpec(crs=crs, **gs_params)

https://github.com/opendatacube/datacube-core/blob/0c5475e54df968aac60864f4542630da20ce7767/datacube/model/__init__.py#L526

robbibt commented 5 months ago

I remember seeing a commit from @SpacemanPaul or @Ariana-B saying the odc-geo GridSpec was intentionally excluded from the odc-geo retrofit - I can't remember what the reasoning was though.

The new odc-geo GridSpec is more powerful and easier to use than the old one though, so it would be great to use it in datacube-1.9.

(Also pinging @Kirill888 in case there's an obvious incompatibility here - I know it's changed from being defined by distance units to pixel width/height).

robbibt commented 5 months ago

Here: https://github.com/opendatacube/datacube-core/commit/e75368c978ed6b16a3b4bf67ba1f63c50076e791

snowman2 commented 4 months ago

odc-geo GridSpec was intentionally excluded from the odc-geo retrofit

Maybe filter the warning internally then?

robbibt commented 4 months ago

My feeling is that supporting two similar but different GridSpecs within ODC is going to be a recipe for confusion going forward. I'd love for us to switch to the odc-geo version if at all possible.

Kirill888 commented 4 months ago

not sure what the reason was for exclusion, maybe it didn't produce exactly equivalent binning to older code on top of changing constructor interface? I vaguely remember @emmaai raising something along those lines https://github.com/opendatacube/odc-geo/issues/97, but I might misremember also. There were some fixes in binning code since, but maybe that's what caused differences instead of fixing them, not sure.

Functionally new code is equivalent, but constructor is very different, and some properties were renamed for clarity.

SpacemanPaul commented 4 months ago

It's 1.9.0-pre01 - it's not a release candidate, it's a pre-release and still has plenty of gaps.

Similar to the GeoBox issue, the whole GridSpec workflow in core will be deprecated* and eventually removed, migration to odc-geo is encouraged. Not retrofitted because the plan is to eventually remove it from core all together.