Annoying Deprecation Warning

[omad]

While testing out https://github.com/opendatacube/odc-dscache/pull/7 locally, I noticed pytest displaying a warning, which I assumed was coming from the odc-dscache code somewhere:

============================================================ warnings summary =============================================================
.venv/lib/python3.12/site-packages/datacube/api/core.py:37
odc-dscache/.venv/lib/python3.12/site-packages/datacube/api/core.py:37: DeprecationWarning: datacube.utils.geometry is now deprecated. Please use the odc-geo library instead.
    from datacube.utils.geometry import GeoBox as LegacyGeoGeoBox

Rerunning with PYTHONWARNINGS=error pytest --ignore=.venv --pdb, and I can see it's actually from within datacube-core, where it imports GeoBox as LegacyGeoGeoBox:

View pytest TraceBack of warning

=========================================================== test session starts ===========================================================
platform darwin -- Python 3.12.8, pytest-8.3.5, pluggy-1.6.0
rootdir: /Users/aye011/dev/odc-dscache
configfile: pyproject.toml
collecting ...
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> traceback >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
tests/test_dscache.py:1: in <module>
    from odc.dscache._dscache import mk_group_name, parse_group_name
odc/dscache/__init__.py:4: in <module>
    from ._dscache import DatasetCache, TileIdx, create_cache, open_ro, open_rw
odc/dscache/_dscache.py:6: in <module>
    from datacube.model import (
.venv/lib/python3.12/site-packages/datacube/__init__.py:22: in <module>
    from .api import Datacube
.venv/lib/python3.12/site-packages/datacube/api/__init__.py:9: in <module>
    from .core import Datacube, TerminateCurrentLoad
.venv/lib/python3.12/site-packages/datacube/api/core.py:37: in <module>
    from datacube.utils.geometry import GeoBox as LegacyGeoGeoBox
.venv/lib/python3.12/site-packages/datacube/utils/geometry/__init__.py:9: in <module>
    warn(
E   DeprecationWarning: datacube.utils.geometry is now deprecated. Please use the odc-geo library instead.

This import was added recently to help with Typing, but I think needs tweaking to not display this annoying warning every time someone imports datacube.

[SpacemanPaul]

I think this was introduced in https://github.com/opendatacube/datacube-core/pull/1912 - pretty sure it's the thing Peter was confused by and asked me to clarify but you guys merged it before I had a chance to respond. :)

[omad]

I'm looking at it now Paul, and it's more annoying than I thought. The old GeoBox type is also used in an isintance to check and optionally convert to an odc-geo GeoBox.

I've tried lots of different things with warnings, and can't get the behaviour that I want.

I think that optional back conversion either needs to operate based on a less explicit check, or to just disappear altogether.

[SpacemanPaul]

on a less explicit check

e.g. the weird hack that used to be there before #1912,

or wrap the import in a Deprecation warning suppression, as used in tests:

https://github.com/opendatacube/datacube-core/blob/b7dd60150f39dcb60422b516f64b4870a48fd9ba/tests/test_model.py#L73

[omad]

or wrap the import in a Deprecation warning suppression, as used in tests:

That's one of the things I tried yesterday which didn't work. Due to the way python imports run, and the import structure in datacube, suppressing the warning here prevents it from ever being shown.

[SpacemanPaul]

Can we do the check in reverse?

None: cool, do nothing. isinstance(odc.geo.geobox.GeoBox): cool, do nothing. Anything else: assume legacy geobox and convert?

[pjonsson]

There are basically two parts, the first is the type signature, and that can be solved with the regular if TYPE_CHECKING: and import in that if-statement, and the second is the isinstance check.

It doesn't solve the problem, but the import for the code could be done locally just before the isinstance check so the warning is only triggered when the class is required for the isinstance check.

If the logic can be reversed like Paul suggests that is even better, because then the deprecation warning would only be triggered when the user is passing a legacy geobox, and that should give a deprecation warning.