Make sure that get_items is backwards compatible

CHANGELOG.md

@@ -30,6 +30,7 @@
 - Return all validation errors from validation methods of `JsonSchemaSTACValidator` ([#1120](https://github.com/stac-utils/pystac/pull/1120))
 - EO extension updated to v1.1.0 ([#1131](https://github.com/stac-utils/pystac/pull/1131))
 - Use `id` in STACTypeError instead of entire dict ([#1126](https://github.com/stac-utils/pystac/pull/1126))
+- Make sure that `get_items` is backwards compatible ([#1139](https://github.com/stac-utils/pystac/pull/1139))
 
 ### Deprecated
 

pystac/catalog.py

@@ -476,7 +476,14 @@ def get_item(self, id: str, recursive: bool = False) -> Optional[Item]:
             "Use next(self.get_items(id), None) instead",
             DeprecationWarning,
         )
-        return next(self.get_items(id, recursive=recursive), None)
+        if not recursive:
+            return next((i for i in self.get_items() if i.id == id), None)
+        else:
+            for root, _, _ in self.walk():
+                item = root.get_item(id, recursive=False)
+                if item is not None:
+                    return item
+            return None
 
     def get_items(self, *ids: str, recursive: bool = False) -> Iterator[Item]:
         """Return all items or specific items of this catalog.
@@ -503,7 +510,6 @@ def get_items(self, *ids: str, recursive: bool = False) -> Iterator[Item]:
                 self.get_items(recursive=False),
                 *(child.get_items(recursive=True) for child in self.get_children()),
             )
-
         if ids:
             yield from (i for i in items if i.id in ids)
         else:
@@ -544,7 +550,7 @@ def remove_item(self, item_id: str) -> None:
                     item.set_root(None)
         self.links = new_links
 
-    def get_all_items(self) -> Iterable[Item]:
+    def get_all_items(self) -> Iterator[Item]:
         """
         DEPRECATED.
 
@@ -563,7 +569,10 @@ def get_all_items(self) -> Iterable[Item]:
             "get_item is deprecated and will be removed in v2",
             DeprecationWarning,
         )
-        return self.get_items(recursive=True)
+        return chain(
+            self.get_items(),
+            *(child.get_all_items() for child in self.get_children()),
+        )
 
     def get_item_links(self) -> List[Link]:
         """Return all item links of this catalog.

pystac/collection.py

@@ -707,7 +707,14 @@ def get_item(self, id: str, recursive: bool = False) -> Optional[Item]:
         Return:
             Item or None: The item with the given ID, or None if not found.
         """
-        return next(self.get_items(id, recursive=recursive), None)
+        try:
+            return next(self.get_items(id, recursive=recursive), None)
+        except TypeError as e:
+            if any("recursive" in arg for arg in e.args):
+                # For inherited classes that do not yet support recursive
+                # See https://github.com/stac-utils/pystac-client/issues/485
+                return super().get_item(id, recursive=recursive)
+            raise e
 
     def get_assets(
         self,

tests/test_catalog.py

@@ -9,7 +9,7 @@
 from copy import deepcopy
 from datetime import datetime
 from pathlib import Path
-from typing import Any, Dict, List, Optional, Tuple, Union, cast
+from typing import Any, Dict, Iterator, List, Optional, Tuple, Union, cast
 
 import pytest
 
@@ -1501,7 +1501,11 @@ class CatalogSubClassTest(unittest.TestCase):
     case_1 = TestCases.get_path("data-files/catalogs/test-case-1/catalog.json")
 
     class BasicCustomCatalog(pystac.Catalog):
-        pass
+        def get_items(self) -> Iterator[Item]:  # type: ignore
+            # This get_items does not have the `recursive` kwarg. This mimics
+            # the current state of pystac-client and is intended to test
+            # backwards compatibility of inherited classes
+            return super().get_items()
 
     def setUp(self) -> None:
         self.stac_io = pystac.StacIO.default()
@@ -1523,6 +1527,18 @@ def test_clone(self) -> None:
 
         self.assertIsInstance(cloned_catalog, self.BasicCustomCatalog)
 
+    def test_get_all_items_works(self) -> None:
+        custom_catalog = self.BasicCustomCatalog.from_file(self.case_1)
+        cloned_catalog = custom_catalog.clone()
+        with pytest.warns(DeprecationWarning):
+            cloned_catalog.get_all_items()
+
+    def test_get_item_works(self) -> None:
+        custom_catalog = self.BasicCustomCatalog.from_file(self.case_1)
+        cloned_catalog = custom_catalog.clone()
+        with pytest.warns(DeprecationWarning):
+            cloned_catalog.get_item("area-1-1-imagery")
+
 
 def test_custom_catalog_from_dict(catalog: Catalog) -> None:
     # https://github.com/stac-utils/pystac/issues/862

tests/test_collection.py

@@ -6,7 +6,7 @@
 import unittest
 from copy import deepcopy
 from datetime import datetime
-from typing import Any, Dict, Optional
+from typing import Any, Dict, Iterator, Optional
 
 import pytest
 from dateutil import tz
@@ -496,7 +496,11 @@ class CollectionSubClassTest(unittest.TestCase):
     MULTI_EXTENT = TestCases.get_path("data-files/collections/multi-extent.json")
 
     class BasicCustomCollection(pystac.Collection):
-        pass
+        def get_items(self) -> Iterator[Item]:  # type: ignore
+            # This get_items does not have the `recursive` kwarg. This mimics
+            # the current state of pystac-client and is intended to test
+            # backwards compatibility of inherited classes
+            return super().get_items()
 
     def setUp(self) -> None:
         self.stac_io = pystac.StacIO.default()
@@ -518,6 +522,33 @@ def test_clone(self) -> None:
 
         self.assertIsInstance(cloned_collection, self.BasicCustomCollection)
 
+    def test_collection_get_item_works(self) -> None:
+        path = TestCases.get_path(
+            "data-files/catalogs/test-case-1/country-1/area-1-1/collection.json"
+        )
+        custom_collection = self.BasicCustomCollection.from_file(path)
+        collection = custom_collection.clone()
+        with pytest.warns(DeprecationWarning):
+            collection.get_item("area-1-1-imagery")
+
+
+class CollectionPartialSubClassTest(unittest.TestCase):
+    class BasicCustomCollection(pystac.Collection):
+        def get_items(  # type: ignore
+            self, *, recursive: bool = False
+        ) -> Iterator[Item]:
+            # This get_items does not allow ids as args.
+            return super().get_items(recursive=recursive)
+
+    def test_collection_get_item_raises_type_error(self) -> None:
+        path = TestCases.get_path(
+            "data-files/catalogs/test-case-1/country-1/area-1-1/collection.json"
+        )
+        custom_collection = self.BasicCustomCollection.from_file(path)
+        collection = custom_collection.clone()
+        with pytest.raises(TypeError, match="takes 1 positional argument"):
+            collection.get_item("area-1-1-imagery")
+
 
 def test_custom_collection_from_dict(collection: Collection) -> None:
     # https://github.com/stac-utils/pystac/issues/862