Python: Add new linting rules to High-level client (#302)

Author: alexluck-sift

python/lib/sift_client/.ruff.toml

@@ -0,0 +1,60 @@
+extend = "../../pyproject.toml"
+
+[lint]
+select = [
+    # Core linting
+    "F", # pyflakes - detect various errors
+    "W", # pycodestyle warnings - style recommendations
+    "B", # flake8-bugbear - detect potential bugs and design problems
+    "PERF", # perflint - performance optimizations
+    "RUF", # ruff-specific rules
+    #     "ASYNC",  # flake8-async - async/await best practices # TODO
+
+    # Code style and formatting
+    "I", # isort - import sorting
+    "N", # pep8-naming - naming conventions
+    "C4", # flake8-comprehensions - better list/dict/set comprehensions
+    "UP", # pyupgrade - modernize syntax to latest Python
+
+    # Documentation
+    "D", # pydocstyle - docstring style checking
+
+    # Imports and modules
+    "TID", # flake8-tidy-imports - import organization
+    "INP", # flake8-no-pep420 - no implicit namespace packages
+
+    # Type checking and annotations
+    "FA", # flake8-future-annotations - necessary for backwards compatibility
+    "TC", # flake8-type-checking - type checking best practices
+
+    # Built-ins and standard library
+    "A", # flake8-builtins - prevent overriding built-ins
+    "DTZ", # flake8-datetimez - good timezone practices
+
+    # Exception handling
+    #    "TRY", # tryceratops - exception handling antipatterns # TODO: FD-102
+
+    # Logging best practices
+    #    "G", # flake8-logging-format - logging format best practices # TODO: FD-101
+    "LOG", # flake8-logging - logging best practices
+
+    # Tests
+    #     "PT",     # flake8-pytest-style - pytest best practices # TODO: FD-59
+
+]
+
+ignore = ["W191", "D206", "D300", # https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
+    "B024", # ignore missing abstract methods
+    "D104", # Missing docstring in public package
+    "D105", # Missing docstring in magic method
+    "D205", # 1 blank line required between summary line and description
+    "D100", # Missing docstring in public module
+]
+
+
+[lint.pydocstyle]
+convention = "google"
+
+[lint.per-file-ignores]
+"examples/*" = ["D"]
+"_internal/*" = ["D"] # Private docs, be less strict

python/lib/sift_client/README.md

@@ -130,7 +130,7 @@ asset.update({
 For more complex updates, you can create update models (instead of a key-value dictionary):
 
 ```python
-from sift_client.types.asset import AssetUpdate
+from sift_client.sift_types.asset import AssetUpdate
 
 # Create an update model
 update = AssetUpdate(tags=["new", "tags"])

python/lib/sift_client/__init__.py

@@ -1,5 +1,4 @@
-"""
-!!! warning
+"""!!! warning
     The Sift Client is experimental and is subject to change.
 
 

python/lib/sift_client/_internal/gen_pyi.py

@@ -23,20 +23,16 @@ class {cls_name}:
 {methods}
 """
 
-METHOD_TEMPLATE = '''\
+METHOD_TEMPLATE = """\
     {decorator}
     def {meth_name}(self{params}){ret_ann}:
-        """
-        {meth_doc}
-        """
+{docstring_section}
         ...
-'''
+"""
 
 
 def extract_imports(path: pathlib.Path) -> list[str]:
-    """
-    Parse the given Python file and return a list of its import statements (as strings).
-    """
+    """Parse the given Python file and return a list of its import statements (as strings)."""
     source = path.read_text()
     tree = ast.parse(source, filename=str(path))
 
@@ -125,22 +121,23 @@ def generate_stubs_for_module(path_arg: str | pathlib.Path) -> dict[pathlib.Path
             async_class = matched.get("async_cls")
             if async_class is None:
                 warnings.warn(
-                    f"Could not find async class for {cls_name}. Skipping stub generation."
+                    f"Could not find async class for {cls_name}. Skipping stub generation.",
+                    stacklevel=2,
                 )
                 continue
 
             # Read imports from the original async class module
             source_file = inspect.getsourcefile(async_class)
             if source_file is None:
                 warnings.warn(
-                    f"Could not find source file for {async_class.__name__}. Skipping stub generation."
+                    f"Could not find source file for {async_class.__name__}. Skipping stub generation.",
+                    stacklevel=2,
                 )
                 continue
 
             orig_path = pathlib.Path(source_file).resolve()
             imports = extract_imports(orig_path)
-            for imp in imports:
-                new_module_imports.append(imp)
+            new_module_imports = new_module_imports + imports
 
             # Class docstring
             raw_doc = inspect.getdoc(cls) or ""
@@ -183,7 +180,7 @@ def generate_stubs_for_module(path_arg: str | pathlib.Path) -> dict[pathlib.Path
             lines.append(stub)
 
         unique_imports = list(OrderedDict.fromkeys(new_module_imports))
-        lines = [HEADER] + unique_imports + lines
+        lines = [HEADER, *unique_imports, *lines]
         pyi_file = py_file.with_suffix(".pyi")
 
         stub_files[pyi_file] = "\n".join(lines)
@@ -264,14 +261,18 @@ def generate_method_stub(name: str, f: Callable, module, decorator: str = "") ->
 
     # Method docstring
     raw_mdoc = inspect.getdoc(f) or ""
-    meth_doc = raw_mdoc.replace('"""', '\\"\\"\\"').replace("\n", "\n        ")
+    if raw_mdoc and raw_mdoc.strip():
+        meth_doc = raw_mdoc.replace('"""', '\\"\\"\\"').replace("\n", "\n        ")
+        docstring_section = f'        """\n        {meth_doc}\n        """\n'
+    else:
+        docstring_section = ""
 
     return METHOD_TEMPLATE.format(
         decorator=decorator,
         meth_name=name,
         params=params_txt,
         ret_ann=ret_txt,
-        meth_doc=meth_doc,
+        docstring_section=docstring_section,
     )
 
 

python/lib/sift_client/_internal/low_level_wrappers/assets.py

@@ -17,23 +17,21 @@
 from sift_client._internal.low_level_wrappers.base import (
     LowLevelClientBase,
 )
+from sift_client.sift_types.asset import Asset, AssetUpdate
 from sift_client.transport import GrpcClient, WithGrpcClient
-from sift_client.types.asset import Asset, AssetUpdate
 
 # Configure logging
 logger = logging.getLogger(__name__)
 
 
 class AssetsLowLevelClient(LowLevelClientBase, WithGrpcClient):
-    """
-    Low-level client for the AssetsAPI.
+    """Low-level client for the AssetsAPI.
 
     This class provides a thin wrapper around the autogenerated bindings for the AssetsAPI.
     """
 
     def __init__(self, grpc_client: GrpcClient):
-        """
-        Initialize the AssetsLowLevelClient.
+        """Initialize the AssetsLowLevelClient.
 
         Args:
             grpc_client: The gRPC client to use for making API calls.
@@ -43,7 +41,7 @@ def __init__(self, grpc_client: GrpcClient):
     async def get_asset(self, asset_id: str) -> Asset:
         request = GetAssetRequest(asset_id=asset_id)
         response = await self._grpc_client.get_stub(AssetServiceStub).GetAsset(request)
-        grpc_asset = cast(GetAssetResponse, response).asset
+        grpc_asset = cast("GetAssetResponse", response).asset
         return Asset._from_proto(grpc_asset)
 
     async def list_all_assets(
@@ -53,8 +51,7 @@ async def list_all_assets(
         max_results: int | None = None,
         page_size: int | None = None,
     ) -> list[Asset]:
-        """
-        List all results matching the given query.
+        """List all results matching the given query.
 
         Args:
             query_filter: The CEL query filter.
@@ -93,14 +90,14 @@ async def list_assets(
 
         request = ListAssetsRequest(**request_kwargs)
         response = await self._grpc_client.get_stub(AssetServiceStub).ListAssets(request)
-        response = cast(ListAssetsResponse, response)
+        response = cast("ListAssetsResponse", response)
         return [Asset._from_proto(asset) for asset in response.assets], response.next_page_token
 
     async def update_asset(self, update: AssetUpdate) -> Asset:
         grpc_asset, update_mask = update.to_proto_with_mask()
         request = UpdateAssetRequest(asset=grpc_asset, update_mask=update_mask)
         response = await self._grpc_client.get_stub(AssetServiceStub).UpdateAsset(request)
-        updated_grpc_asset = cast(UpdateAssetResponse, response).asset
+        updated_grpc_asset = cast("UpdateAssetResponse", response).asset
         return Asset._from_proto(updated_grpc_asset)
 
     async def delete_asset(self, asset_id: str, archive_runs: bool = False) -> None:

python/lib/sift_client/_internal/low_level_wrappers/base.py

@@ -8,14 +8,13 @@ class LowLevelClientBase(ABC):
     @staticmethod
     async def _handle_pagination(
         func: Callable,
-        kwargs: dict[str, Any] = {},
+        kwargs: dict[str, Any] | None = None,
         page_size: int | None = None,
         page_token: str | None = None,
         order_by: str | None = None,
         max_results: int | None = None,
     ) -> list[Any]:
-        """
-        Handle pagination for a given function by calling the function until all results are retrieved or the max_results is reached.
+        """Handle pagination for a given function by calling the function until all results are retrieved or the max_results is reached.
 
         Args:
             func: The function to call.
@@ -28,6 +27,9 @@ async def _handle_pagination(
         Returns:
             A list of all matching results.
         """
+        if kwargs is None:
+            kwargs = {}
+
         results: list[Any] = []
         if page_token is None:
             page_token = ""