From 28adaf1fcf2c17cf487129b7d645492159f96ab3 Mon Sep 17 00:00:00 2001
From: Phil Varner <pvarner@element84.com>
Date: Thu, 26 May 2022 10:04:53 -0400
Subject: [PATCH] clarify type requirements for search intersects parameter,
 update some documentation (#174)

* clarify type requirements for search intersects parameter, update some documentation
* add python 3.10 to setup.py and github ci config
---
 .github/workflows/continuous-integration.yml | 15 ++++++++---
 CHANGELOG.md                                 |  1 +
 README.md                                    |  3 +--
 docs/usage.rst                               |  4 +--
 pystac_client/item_search.py                 | 28 +++++++++++++++-----
 tests/test_item_search.py                    |  8 +++++-
 6 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
index 9889282e..84985391 100644
--- a/.github/workflows/continuous-integration.yml
+++ b/.github/workflows/continuous-integration.yml
@@ -9,12 +9,21 @@ on:
 jobs:
   build:
     name: build
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     strategy:
+      fail-fast: false
       matrix:
-        python-version: ["3.7", "3.8", "3.9"]
+        python-version:
+          - "3.7"
+          - "3.8"
+          - "3.9"
+          - "3.10"
+#          - "3.11-dev"
+        os:
+          - ubuntu-latest
+#          - windows-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
 
       - name: Set up Python ${{ matrix.python-version }}
         uses: actions/setup-python@v3
diff --git a/CHANGELOG.md b/CHANGELOG.md
index f6f5c59a..340b1852 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -33,6 +33,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Bumped PySTAC dependency to >= 1.4.0 [#147](https://github.com/stac-utils/pystac-client/pull/147)
 - Search `filter-lang` defaults to `cql2-json` instead of `cql-json`
 - Search `filter-lang` will be set to `cql2-json` if the `filter` is a dict, or `cql2-text` if it is a string
+- Search parameter `intersects` is now typed to only accept a str, dict, or object that implements `__geo_interface__`
 
 ## Removed
 
diff --git a/README.md b/README.md
index 9ae6feb5..46444dfb 100644
--- a/README.md
+++ b/README.md
@@ -1,5 +1,4 @@
-STAC Client
-===============
+# STAC Client <!-- omit in toc -->
 
 [![CI](https://github.com/stac-utils/pystac-client/actions/workflows/continuous-integration.yml/badge.svg)](https://github.com/stac-utils/pystac-client/actions/workflows/continuous-integration.yml)
 [![Release](https://github.com/stac-utils/pystac-client/actions/workflows/release.yml/badge.svg)](https://github.com/stac-utils/pystac-client/actions/workflows/release.yml)
diff --git a/docs/usage.rst b/docs/usage.rst
index 311076dd..3bbd8e93 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -129,7 +129,7 @@ The :meth:`pystac_client.Client.search` method provides an interface for making
 .. code-block:: python
 
     >>> from pystac_client import API
-    >>> api = API.from_file('https://eod-catalog-svc-prod.astraea.earth')
+    >>> api = API.from_file('https://planetarycomputer.microsoft.com/api/stac/v1')
     >>> results = api.search(
     ...     bbox=[-73.21, 43.99, -73.12, 44.05],
     ...     datetime=['2019-01-01T00:00:00Z', '2019-01-02T00:00:00Z'],
@@ -169,7 +169,7 @@ implementation of this ``"next"`` link parsing assumes that the link follows the
 described in the `STAC API - Item Search: Paging <https://github.com/radiantearth/stac-api-spec/tree/master/item-search#paging>`__
 section. See the :mod:`Paging <pystac_client.paging>` docs for details on how to customize this behavior.
 
-Query Filter
+Query Extension
 ------------
 
 If the Catalog supports the `Query
diff --git a/pystac_client/item_search.py b/pystac_client/item_search.py
index c2f34f21..a391535e 100644
--- a/pystac_client/item_search.py
+++ b/pystac_client/item_search.py
@@ -22,6 +22,11 @@
                             r"(?P<remainder>(T|t)\d{2}:\d{2}:\d{2}(\.\d+)?"
                             r"(?P<tz_info>Z|([-+])(\d{2}):(\d{2}))?)?)?)?")
 
+# todo: add runtime_checkable when we drop 3.7 support
+# class GeoInterface(Protocol):
+#     def __geo_interface__(self) -> dict:
+#         ...
+
 DatetimeOrTimestamp = Optional[Union[datetime_, str]]
 Datetime = Union[Tuple[str], Tuple[str, str]]
 DatetimeLike = Union[DatetimeOrTimestamp, Tuple[DatetimeOrTimestamp, DatetimeOrTimestamp],
@@ -37,7 +42,8 @@
 IDsLike = Union[IDs, str, List[str], Iterator[str]]
 
 Intersects = dict
-IntersectsLike = Union[str, Intersects, object]
+IntersectsLike = Union[str, object, Intersects]
+# todo: after 3.7 is dropped, replace object with GeoInterface
 
 Query = dict
 QueryLike = Union[Query, List[str]]
@@ -132,7 +138,9 @@ class ItemSearch:
             - ``2017/2018`` expands to ``2017-01-01T00:00:00Z/2018-12-31T23:59:59Z``
             - ``2017-06/2017-07`` expands to ``2017-06-01T00:00:00Z/2017-07-31T23:59:59Z``
             - ``2017-06-10/2017-06-11`` expands to ``2017-06-10T00:00:00Z/2017-06-11T23:59:59Z``
-        intersects: A GeoJSON-like dictionary or JSON string. Results filtered to only those intersecting the geometry
+        intersects: A string or dictionary representing a GeoJSON geometry, or an object that implements a
+            ``__geo_interface__`` property as supported by several libraries including Shapely, ArcPy, PySAL, and
+            geojson. Results filtered to only those intersecting the geometry.
         ids: List of Item ids to return. All other filter parameters that further restrict the number of search results
             (except ``limit``) are ignored.
         collections: List of one or more Collection IDs or :class:`pystac.Collection` instances. Only Items in one
@@ -142,11 +150,11 @@ class ItemSearch:
         filter_lang: Language variant used in the filter body. If `filter` is a dictionary or not provided, defaults
             to 'cql2-json'. If `filter` is a string, defaults to `cql2-text`.
         sortby: A single field or list of fields to sort the response by
-        fields: A list of fields to return in the response. Note this may result in invalid JSON.
-            Use `get_all_items_as_dict` to avoid errors
-        max_items: The maximum number of items to get, even if there are more matched items
+        fields: A list of fields to include in the response. Note this may result in invalid STAC objects, as they
+            may not have required fields. Use `get_all_items_as_dict` to avoid object unmarshalling errors.
+        max_items: The maximum number of items to get, even if there are more matched items.
         method: The http method, 'GET' or 'POST'
-        stac_io: An instance of of StacIO for retrieving results. Normally comes from the Client that returns this ItemSearch
+        stac_io: An instance of StacIO for retrieving results. Normally comes from the Client that returns this ItemSearch
         client: An instance of a root Client used to set the root on resulting Items
     """
     def __init__(self,
@@ -409,9 +417,15 @@ def _format_fields(self, value: Optional[FieldsLike]) -> Optional[Fields]:
     def _format_intersects(value: Optional[IntersectsLike]) -> Optional[Intersects]:
         if value is None:
             return None
+        if isinstance(value, dict):
+            return deepcopy(value)
         if isinstance(value, str):
             return json.loads(value)
-        return deepcopy(getattr(value, '__geo_interface__', value))
+        if hasattr(value, '__geo_interface__'):
+            return deepcopy(getattr(value, '__geo_interface__'))
+        raise Exception(
+            "intersects must be of type None, str, dict, or an object that implements __geo_interface__"
+        )
 
     @lru_cache(1)
     def matched(self) -> int:
diff --git a/tests/test_item_search.py b/tests/test_item_search.py
index 336c2ab3..8e5d2915 100644
--- a/tests/test_item_search.py
+++ b/tests/test_item_search.py
@@ -260,6 +260,10 @@ def test_intersects_json_string(self):
         search = ItemSearch(url=SEARCH_URL, intersects=json.dumps(INTERSECTS_EXAMPLE))
         assert search._parameters['intersects'] == INTERSECTS_EXAMPLE
 
+    def test_intersects_non_geo_interface_object(self):
+        with pytest.raises(Exception):
+            ItemSearch(url=SEARCH_URL, intersects=object())
+
     def test_filter_lang_default_for_dict(self):
         search = ItemSearch(url=SEARCH_URL, filter={})
         assert search._parameters['filter-lang'] == 'cql2-json'
@@ -377,7 +381,9 @@ def test_intersects_results(self):
 
         # Geo-interface object
         class MockGeoObject:
-            __geo_interface__ = intersects_dict
+            @property
+            def __geo_interface__(self):
+                return intersects_dict
 
         intersects_obj = MockGeoObject()
         search = ItemSearch(url=SEARCH_URL, intersects=intersects_obj, collections='naip')