stac-utils / pystac-client

Python client for searching STAC APIs
https://pystac-client.readthedocs.io
Other
154 stars 47 forks source link

using StacApiIO with regular pystac breaks #706

Closed bossie closed 2 months ago

bossie commented 2 months ago

I figured passing an instance of StacApiIO would be an easy way to add retries and request timeout handling to regular pystac but this does not work e.g.

import pystac
from pystac_client.stac_api_io import StacApiIO

stac_io = None
stac_io = StacApiIO(timeout=60)
stac_object = pystac.read_file(href="collection.json", stac_io=stac_io)

print(stac_object.get_root().id)

fails with:

Traceback (most recent call last):
  File "/usr/lib/python3.8/runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.8/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/home/bossie/PycharmProjects/openeo/openeo-geopyspark-driver/openeogeotrellis/test_pystac_client.py", line 30, in <module>
    test_stac_api_io_with_regular_pystac()
  File "/home/bossie/PycharmProjects/openeo/openeo-geopyspark-driver/openeogeotrellis/test_pystac_client.py", line 26, in test_stac_api_io_with_regular_pystac
    print(stac_object.get_root().id)
  File "/home/bossie/PycharmProjects/openeo/venv38/lib/python3.8/site-packages/pystac_client/collection_client.py", line 108, in get_root
    raise ValueError(
ValueError: `CollectionClient.root` is not have a valid `Client` object.

This is unexpected and seems like a violation of the Liskov substitution principle (CollectionClient IS-A pystac.Collection).

collection.json

bossie commented 2 months ago

Tested with pystac-client 0.7.6 (Python 3.8).

gadomski commented 2 months ago

Confirmed this issue against https://raw.githubusercontent.com/radiantearth/stac-spec/master/examples/collection.json and on current main, 5d465dae9e0a799e694141672fbfafca57df8c0f. That check seems unnecessary, IMO, so I'm going to propose removing it.

gadomski commented 2 months ago

Turns out the fix was slightly different, but I have a proposed one up in #709 which uses your example script as the test -- thanks!