Improve self-documentation of IterableObj and related classes

EliahKagan · EliahKagan · commit dfee31f2100d · 2023-12-22T04:59:17.000-05:00
- Fill in the missing part of the explanation of why to favor iter_items over list_items in IterableObj and Iterable (#1775). - Move the explanation of how subclasses must treat arguments from the list_items methods to the iter_items methods, because the iter_items methdos are the ones that are abstract and must be overridden by a well-behaved subclass, and also because, since the iter_items methods are preferred for use, they should be the place where less duplicated shared documentation resides. - Subtantially reword the existing documentation for clarity, especially regarding the signifance of extra args and kwargs. - Put the iter_items method before (i.e. above) the list_items method (in each of the IterableObj and Iterable classes), because that method is the one that should be used more often, and because it is also now the one with the more detailed docstring. - Remove and old comment on a return type that said exactly the exact same thing as the annotation. - In Iterable, note deprecation more consistently (and thus in more places). - Rewrite the IterableClassWatcher class docstring to explain exactly what that metaclass achieves.
diff --git a/git/util.py b/git/util.py
@@ -1183,7 +1183,8 @@ def __delitem__(self, index: Union[SupportsIndex, int, slice, str]) -> None:
 
 
 class IterableClassWatcher(type):
-    """Metaclass that watches."""
+    """Metaclass that issues :class:`DeprecationWarning` when :class:`git.util.Iterable`
+    is subclassed."""
 
     def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None:
         for base in bases:
@@ -1199,39 +1200,49 @@ def __init__(cls, name: str, bases: Tuple, clsdict: Dict) -> None:
 
 
 class Iterable(metaclass=IterableClassWatcher):
-    """Defines an interface for iterable items, so there is a uniform way to retrieve
-    and iterate items within the git repository."""
+    """Deprecated, use :class:`IterableObj` instead.
+
+    Defines an interface for iterable items, so there is a uniform way to retrieve
+    and iterate items within the git repository.
+    """
 
     __slots__ = ()
 
     _id_attribute_ = "attribute that most suitably identifies your instance"
 
     @classmethod
-    def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any:
+    def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any:
+        # return typed to be compatible with subtypes e.g. Remote
+        """Deprecated, use :class:`IterableObj` instead.
+
+        Find (all) items of this type.
+
+        Subclasses can specify ``args`` and ``kwargs`` differently, and may use them for
+        filtering. However, when the method is called with no additional positional or
+        keyword arguments, subclasses are obliged to to yield all items.
+
+        :return: Iterator yielding Items
         """
-        Deprecated, use IterableObj instead.
+        raise NotImplementedError("To be implemented by Subclass")
+
+    @classmethod
+    def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any:
+        """Deprecated, use :class:`IterableObj` instead.
+
+        Find (all) items of this type and collect them into a list.
 
-        Find all items of this type - subclasses can specify args and kwargs differently.
-        If no args are given, subclasses are obliged to return all items if no additional
-        arguments arg given.
+        For more information about the arguments, see :meth:`list_items`.
 
-        :note: Favor the iter_items method as it will
+        :note: Favor the :meth:`iter_items` method as it will avoid eagerly collecting
+            all items. When there are many items, that can slow performance and increase
+            memory usage.
 
         :return: list(Item,...) list of item instances
         """
         out_list: Any = IterableList(cls._id_attribute_)
         out_list.extend(cls.iter_items(repo, *args, **kwargs))
         return out_list
 
-    @classmethod
-    def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Any:
-        # return typed to be compatible with subtypes e.g. Remote
-        """For more information about the arguments, see list_items.
-
-        :return: Iterator yielding Items
-        """
-        raise NotImplementedError("To be implemented by Subclass")
-
 
 @runtime_checkable
 class IterableObj(Protocol):
@@ -1246,30 +1257,37 @@ class IterableObj(Protocol):
     _id_attribute_: str
 
     @classmethod
-    def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> IterableList[T_IterableObj]:
+    @abstractmethod
+    def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Iterator[T_IterableObj]:
+        # Return-typed to be compatible with subtypes e.g. Remote.
+        """Find (all) items of this type.
+
+        Subclasses can specify ``args`` and ``kwargs`` differently, and may use them for
+        filtering. However, when the method is called with no additional positional or
+        keyword arguments, subclasses are obliged to to yield all items.
+
+        For more information about the arguments, see list_items.
+
+        :return: Iterator yielding Items
         """
-        Find all items of this type - subclasses can specify args and kwargs differently.
-        If no args are given, subclasses are obliged to return all items if no additional
-        arguments arg given.
+        raise NotImplementedError("To be implemented by Subclass")
+
+    @classmethod
+    def list_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> IterableList[T_IterableObj]:
+        """Find (all) items of this type and collect them into a list.
+
+        For more information about the arguments, see :meth:`list_items`.
 
-        :note: Favor the iter_items method as it will
+        :note: Favor the :meth:`iter_items` method as it will avoid eagerly collecting
+            all items. When there are many items, that can slow performance and increase
+            memory usage.
 
         :return: list(Item,...) list of item instances
         """
         out_list: IterableList = IterableList(cls._id_attribute_)
         out_list.extend(cls.iter_items(repo, *args, **kwargs))
         return out_list
 
-    @classmethod
-    @abstractmethod
-    def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> Iterator[T_IterableObj]:  # Iterator[T_IterableObj]:
-        # Return-typed to be compatible with subtypes e.g. Remote.
-        """For more information about the arguments, see list_items.
-
-        :return: Iterator yielding Items
-        """
-        raise NotImplementedError("To be implemented by Subclass")
-
 
 # } END classes
 
