Separate out parent= into parent= or parent_id=. Use overloads to help create combinations of each parameter

Author: schloerke

src/posit/connect/permissions.py

@@ -158,7 +158,7 @@ def destroy(self, *permissions: str | Group | User | Permission) -> list[Permiss
         Returns
         -------
         list[Permission]
-            The removed permissions. If a permission is not found, there is nothing to remove and 
+            The removed permissions. If a permission is not found, there is nothing to remove and
             it is not included in the returned list.
 
         Examples

src/posit/connect/tags.py

@@ -1,6 +1,6 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Optional
+from typing import TYPE_CHECKING, Optional, overload
 
 from typing_extensions import NotRequired, TypedDict, Unpack
 
@@ -266,42 +266,37 @@ def get(self, tag_id: str) -> Tag:
         response = self._ctx.session.get(url)
         return Tag(self._ctx, path, **response.json())
 
-    class _NameParentAttrs(TypedDict, total=False):
-        name: str
-        """
-        The name of the tag.
-
-        Note: tag names are only unique within the scope of a parent, which
-        means that it is possible to have multiple results when querying by
-        name; however, querying by both `name` and `parent` ensures a single
-        result.
-        """
-        parent: NotRequired[str | Tag | None]
-        """The identifier for the parent tag. If there is no parent, the tag is a top-level tag."""
-
     def _update_parent_kwargs(self, kwargs: dict) -> dict:
         parent = kwargs.get("parent", None)
         if parent is None:
+            # No parent to upgrade, return the kwargs as is
             return kwargs
 
+        if not isinstance(parent, Tag):
+            raise TypeError(
+                "`parent=` must be a Tag instance. If using a string, please use `parent_id=`"
+            )
+
+        parent_id = kwargs.get("parent_id", None)
+        if parent_id:
+            raise ValueError("Cannot provide both `parent=` and `parent_id=`")
+
         ret_kwargs = {**kwargs}
 
         # Remove `parent` from ret_kwargs
         # and store the `parent_id` in the ret_kwargs below
         del ret_kwargs["parent"]
 
-        if isinstance(parent, Tag):
-            parent: str = parent["id"]
+        ret_kwargs["parent_id"] = parent["id"]
+        return ret_kwargs
 
-        if isinstance(parent, str):
-            if parent == "":
-                raise ValueError("Tag `parent` cannot be an empty string")
-            ret_kwargs["parent_id"] = parent
-            return ret_kwargs
+    # Allow for every combination of `name` and (`parent` or `parent_id`)
+    @overload
+    def find(self, /, *, name: str = ..., parent: Tag = ...) -> list[Tag]: ...
+    @overload
+    def find(self, /, *, name: str = ..., parent_id: str = ...) -> list[Tag]: ...
 
-        raise TypeError("`parent=` must be a string or Tag instance")
-
-    def find(self, /, **kwargs: Unpack[Tags._NameParentAttrs]) -> list[Tag]:
+    def find(self, /, **kwargs) -> list[Tag]:
         """
         Find tags by name and/or parent.
 
@@ -313,7 +308,10 @@ def find(self, /, **kwargs: Unpack[Tags._NameParentAttrs]) -> list[Tag]:
         ----------
         name : str, optional
             The name of the tag.
-        parent : str, Tag, optional
+        parent : Tag, optional
+            The parent `Tag` object. If there is no parent, the tag is a top-level tag. Only one of
+            `parent` or `parent_id` can be provided.
+        parent_id : str, optional
             The identifier for the parent tag. If there is no parent, the tag is a top-level tag.
 
         Returns
@@ -348,15 +346,25 @@ def find(self, /, **kwargs: Unpack[Tags._NameParentAttrs]) -> list[Tag]:
         results = response.json()
         return [Tag(self._ctx, f"{self._path}/{result['id']}", **result) for result in results]
 
-    def create(self, /, **kwargs: Unpack[Tags._NameParentAttrs]) -> Tag:
+    @overload
+    def create(self, /, *, name: str) -> Tag: ...
+    @overload
+    def create(self, /, *, name: str, parent: Tag) -> Tag: ...
+    @overload
+    def create(self, /, *, name: str, parent_id: str) -> Tag: ...
+
+    def create(self, /, **kwargs) -> Tag:
         """
         Create a tag.
 
         Parameters
         ----------
         name : str
             The name of the tag.
-        parent : str, Tag, optional
+        parent : Tag, optional
+            The parent `Tag` object. If there is no parent, the tag is a top-level tag. Only one of
+            `parent` or `parent_id` can be provided.
+        parent_id : str, optional
             The identifier for the parent tag. If there is no parent, the tag is a top-level tag.
 
         Returns
@@ -456,8 +464,7 @@ def add(self, *tags: str | Tag) -> None:
 
         url = self._ctx.url + self._path
         for tag_id in tag_ids:
-            _ = self._ctx.session.post(url, json={"tag_id": tag_id})
-        return
+            self._ctx.session.post(url, json={"tag_id": tag_id})
 
     def delete(self, *tags: str | Tag) -> None:
         """
@@ -481,4 +488,3 @@ def delete(self, *tags: str | Tag) -> None:
         for tag_id in tag_ids:
             tag_url = f"{url}/{tag_id}"
             self._ctx.session.delete(tag_url)
-        return

tests/posit/connect/test_tags.py

@@ -70,18 +70,14 @@ def test_find_tags_by_parent(self):
         client = Client(api_key="12345", url="https://connect.example")
 
         # invoke
-        by_str_tags = client.tags.find(parent="3")
+        by_str_tags = client.tags.find(parent_id="3")
         parent_tag = Tag(client._ctx, "/v1/tags/3", id="3", name="Parent")
         by_tag_tags = client.tags.find(parent=parent_tag)
-        by_parent_id_tags = client.tags.find(
-            parent_id=3,  # pyright: ignore[reportCallIssue]
-        )
 
         # assert
-        assert mock_get_tags.call_count == 3
+        assert mock_get_tags.call_count == 2
         assert len(by_str_tags) == 7
         assert len(by_tag_tags) == 7
-        assert len(by_parent_id_tags) == 7
 
     @responses.activate
     def test_get(self):
@@ -115,16 +111,20 @@ def test_create_tag(self):
         tag = Tag(client._ctx, "/v1/tags/1", id="3", name="Tag")
 
         # invoke
-        academy_tag_parent_id = client.tags.create(name="academy", parent=tag["id"])
+        academy_tag_parent_id = client.tags.create(name="academy", parent_id=tag["id"])
         academy_tag_parent_tag = client.tags.create(name="academy", parent=tag)
 
         with pytest.raises(TypeError):
             client.tags.create(
                 name="academy",
-                parent=123,  # pyright: ignore[reportArgumentType]
+                parent="not a tag",  # pyright: ignore[reportArgumentType]
             )
         with pytest.raises(ValueError):
-            client.tags.create(name="academy", parent="")
+            client.tags.create(  # pyright: ignore[reportCallIssue]
+                name="academy",
+                parent=tag,
+                parent_id="asdf",
+            )
 
         # assert
         assert mock_create_tag.call_count == 2