From 5588928fa4b9bf8981988c33e18ee5051f99815a Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Fri, 19 Dec 2025 13:15:12 +1300 Subject: [PATCH 1/9] Select and comply with all Ruff rules, minus a curated ignore list --- pyproject.toml | 11 ++- src/grimp/__init__.py | 4 +- src/grimp/adaptors/caching.py | 30 +++++--- src/grimp/adaptors/filesystem.py | 11 ++- src/grimp/adaptors/modulefinder.py | 49 +++++++----- src/grimp/adaptors/packagefinder.py | 23 ++++-- src/grimp/application/config.py | 14 ++-- src/grimp/application/graph.py | 76 +++++++++++-------- src/grimp/application/ports/caching.py | 13 +++- src/grimp/application/ports/filesystem.py | 9 ++- src/grimp/application/ports/modulefinder.py | 15 ++-- src/grimp/application/ports/packagefinder.py | 6 +- src/grimp/application/ports/timing.py | 5 +- src/grimp/application/scanning.py | 14 +++- src/grimp/application/usecases.py | 44 ++++++----- src/grimp/domain/analysis.py | 8 +- src/grimp/domain/valueobjects.py | 13 ++-- src/grimp/exceptions.py | 12 ++- src/grimp/main.py | 2 +- tests/adaptors/filesystem.py | 12 ++- tests/adaptors/modulefinder.py | 11 ++- tests/adaptors/packagefinder.py | 10 ++- tests/adaptors/timing.py | 5 +- tests/benchmarking/test_benchmarking.py | 17 +++-- tests/config.py | 2 +- tests/functional/test_build_and_use_graph.py | 2 +- ...build_and_use_graph_with_multiple_roots.py | 3 +- .../test_build_graph_on_real_packages.py | 2 +- tests/functional/test_caching.py | 2 +- tests/functional/test_error_handling.py | 2 +- tests/functional/test_namespace_packages.py | 2 +- tests/unit/adaptors/test_caching.py | 4 +- tests/unit/adaptors/test_filesystem.py | 14 +++- tests/unit/adaptors/test_modulefinder.py | 9 +-- tests/unit/adaptors/test_packagefinder.py | 4 +- tests/unit/application/graph/test_chains.py | 3 +- .../application/graph/test_cycle_breakers.py | 3 +- .../application/graph/test_direct_imports.py | 5 +- .../unit/application/graph/test_hierarchy.py | 5 +- tests/unit/application/graph/test_layers.py | 11 ++- .../application/graph/test_manipulation.py | 3 +- tests/unit/application/graph/test_misc.py | 2 +- tests/unit/application/test_scanning.py | 16 ++-- tests/unit/application/test_usecases.py | 12 ++- tests/unit/conftest.py | 4 +- tests/unit/domain/test_valueobjects.py | 4 +- 46 files changed, 337 insertions(+), 191 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 51daee8f..e281961b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -84,6 +84,11 @@ exclude = [ 'tests/assets', # These are sample packages for tests to run under - we don't want ruff to mess with them. 'docs/', # Documentation configuration scripts ] -lint.select = ["A", "C4", "E4", "E7", "E9", "F", "FLY", "FURB", "INP", "PLE", "PLR", "PT", "RUF", "SIM", "UP"] -lint.ignore = ["C408", "PLR2004", "PLR0913", "PT006", "PT007", "SIM108", "SIM118", "SIM300"] -lint.per-file-ignores."tests/**/*.py" = [ "RUF012", "RUF059" ] +lint.select = [ "ALL" ] +lint.ignore = [ "A005", "ANN401" ,"ARG002", "C408", "COM812", "D1", "D2", "E501", "FBT", "FIX", "G004", "N818", "PERF203", "PLR2004", "PLR0913", "PLW1641", "PTH", "PT006", "PT007", "RET504", "SIM108", "SIM118", "SIM300", "TD", "TID252" ] +lint.per-file-ignores."tests/**/*.py" = [ "ANN", "ARG", "B007", "B018", "B033", "D", "EM", "FBT", "N806", "RET505", "RUF012", "RUF059", "S", "TRY" ] +lint.pydocstyle.convention = "google" +lint.future-annotations = true +lint.flake8-builtins.strict-checking = true +lint.flake8-type-checking.quote-annotations = true +lint.typing-extensions = false diff --git a/src/grimp/__init__.py b/src/grimp/__init__.py index e75000e1..f638d324 100644 --- a/src/grimp/__init__.py +++ b/src/grimp/__init__.py @@ -1,8 +1,8 @@ __version__ = "3.14" -from .application.graph import DetailedImport, ImportGraph, Import +from .application.graph import DetailedImport, Import, ImportGraph from .domain.analysis import PackageDependency, Route -from .domain.valueobjects import DirectImport, Module, Layer +from .domain.valueobjects import DirectImport, Layer, Module from .main import build_graph __all__ = [ diff --git a/src/grimp/adaptors/caching.py b/src/grimp/adaptors/caching.py index d6a10af4..01156197 100644 --- a/src/grimp/adaptors/caching.py +++ b/src/grimp/adaptors/caching.py @@ -1,15 +1,19 @@ -import hashlib +from __future__ import annotations +import hashlib import json import logging +from typing import TYPE_CHECKING, Any -from grimp.application.ports.filesystem import BasicFileSystem -from grimp.application.ports.modulefinder import FoundPackage, ModuleFile -from grimp.domain.valueobjects import DirectImport, Module +from grimp import _rustgrimp as rust # type: ignore[attr-defined] from ..application.ports.caching import Cache as AbstractCache from ..application.ports.caching import CacheMiss -from grimp import _rustgrimp as rust # type: ignore[attr-defined] + +if TYPE_CHECKING: + from grimp.application.ports.filesystem import BasicFileSystem + from grimp.application.ports.modulefinder import FoundPackage, ModuleFile + from grimp.domain.valueobjects import DirectImport, Module logger = logging.getLogger(__name__) PrimitiveFormat = dict[str, list[tuple[str, int | None, str]]] @@ -64,7 +68,7 @@ def make_data_file_unique_string( class Cache(AbstractCache): DEFAULT_CACHE_DIR = ".grimp_cache" - def __init__(self, *args, namer: type[CacheFileNamer], **kwargs) -> None: + def __init__(self, *args: Any, namer: type[CacheFileNamer], **kwargs: Any) -> None: """ Don't instantiate Cache directly; use Cache.setup(). """ @@ -82,7 +86,7 @@ def setup( exclude_type_checking_imports: bool = False, cache_dir: str | None = None, namer: type[CacheFileNamer] = CacheFileNamer, - ) -> "Cache": + ) -> Cache: cache = cls( file_system=file_system, found_packages=found_packages, @@ -93,7 +97,8 @@ def setup( ) cache._build_mtime_map() cache._build_data_map() - assert cache.cache_dir + if not cache.cache_dir: + raise AssertionError return cache @classmethod @@ -104,7 +109,7 @@ def read_imports(self, module_file: ModuleFile) -> set[DirectImport]: try: cached_mtime = self._mtime_map[module_file.module.name] except KeyError: - raise CacheMiss + raise CacheMiss from None if cached_mtime != module_file.mtime: raise CacheMiss @@ -113,7 +118,7 @@ def read_imports(self, module_file: ModuleFile) -> set[DirectImport]: except KeyError: # While we would expect the module to be in here, # there's no point in crashing if, for some reason, it's not. - raise CacheMiss + raise CacheMiss from None def write( self, @@ -170,12 +175,13 @@ def _read_mtime_map_file(self, found_package: FoundPackage) -> dict[str, float]: return {} try: deserialized = json.loads(serialized) - logger.info(f"Used cache meta file {meta_cache_filename}.") - return deserialized except json.JSONDecodeError: logger.warning(f"Could not use corrupt cache file {meta_cache_filename}.") return {} + logger.info(f"Used cache meta file {meta_cache_filename}.") + return deserialized + def _build_data_map(self) -> None: self._data_map = self._read_data_map_file() diff --git a/src/grimp/adaptors/filesystem.py b/src/grimp/adaptors/filesystem.py index cded5eab..ae76620b 100644 --- a/src/grimp/adaptors/filesystem.py +++ b/src/grimp/adaptors/filesystem.py @@ -1,9 +1,16 @@ +from __future__ import annotations + import os import tokenize -from collections.abc import Iterator +from typing import TYPE_CHECKING -from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem from grimp import _rustgrimp as rust # type: ignore[attr-defined] +from grimp.application.ports.filesystem import AbstractFileSystem + +if TYPE_CHECKING: + from collections.abc import Iterator + + from grimp.application.ports.filesystem import BasicFileSystem class FileSystem(AbstractFileSystem): diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index 2fae2956..d355d50e 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -1,10 +1,17 @@ +from __future__ import annotations + import logging -from collections.abc import Iterable, Set +from typing import TYPE_CHECKING from grimp.application.ports import modulefinder -from grimp.application.ports.filesystem import AbstractFileSystem from grimp.domain.valueobjects import Module +if TYPE_CHECKING: + from collections.abc import Iterable + from collections.abc import Set as AbstractSet + + from grimp.application.ports.filesystem import AbstractFileSystem + logger = logging.getLogger(__name__) @@ -44,7 +51,7 @@ def find_package( def _get_python_files_and_namespace_dirs_inside_package( self, directory: str - ) -> tuple[Iterable[str], Set[str]]: + ) -> tuple[Iterable[str], AbstractSet[str]]: """ Search the supplied package directory for Python files and namespaces. @@ -79,14 +86,16 @@ def _get_python_files_and_namespace_dirs_inside_package( for d in dirs_to_remove: dirs.remove(d) - for filename in files: - if self._is_python_file(filename, dirpath): - python_files.append(self.file_system.join(dirpath, filename)) + python_files.extend( + self.file_system.join(dirpath, filename) + for filename in files + if self._is_python_file(filename, dirpath) + ) namespace_dirs = self._determine_namespace_dirs(candidate_namespace_dirs, python_files) return python_files, namespace_dirs - def _is_in_portion(self, directory: str, portions: Set[str]) -> bool: + def _is_in_portion(self, directory: str, portions: AbstractSet[str]) -> bool: return any(directory.startswith(portion) for portion in portions) def _should_ignore_dir(self, directory: str) -> bool: @@ -112,9 +121,11 @@ def _is_python_file(self, filename: str, dirpath: str) -> bool: Files with extra dots in the name won't be treated as Python files. Args: - filename (str): the filename, excluding the path. + filename: the filename, excluding the path. + dirpath: the directory path containing the file. + Returns: - bool: whether it's a Python file. + Whether it's a Python file. """ # Ignore hidden files. if filename.startswith("."): @@ -138,12 +149,13 @@ def _module_name_from_filename( ) -> str: """ Args: - package_name (string) - the importable name of the top level package. Could + package_name: the importable name of the top level package. Could be namespaced. - filename_and_path (string) - the full name of the Python file. - package_directory (string) - the full path of the top level Python package directory. - Returns: - Absolute module name for importing (string). + filename_and_path: the full name of the Python file. + package_directory: the full path of the top level Python package directory. + + Returns: + Absolute module name for importing. """ internal_filename_and_path = filename_and_path[len(package_directory) :] internal_filename_and_path_without_extension = internal_filename_and_path[1:-3] @@ -160,11 +172,12 @@ def _namespace_from_dir( ) -> str: """ Args: - package_name (string) - the importable name of the top level package. Could + package_name: the importable name of the top level package. Could be namespaced. - namespace_dir (string) - the full name of the namespace directory. - package_directory (string) - the full path of the top level Python package directory. - Returns: + namespace_dir: - the full name of the namespace directory. + package_directory: - the full path of the top level Python package directory. + + Returns: Absolute module name for importing (string). """ parent_of_package_directory = package_directory[: -len(package_name)] diff --git a/src/grimp/adaptors/packagefinder.py b/src/grimp/adaptors/packagefinder.py index 09d459f5..5da6955b 100644 --- a/src/grimp/adaptors/packagefinder.py +++ b/src/grimp/adaptors/packagefinder.py @@ -1,13 +1,19 @@ +from __future__ import annotations + import importlib.util import logging import sys -from importlib.machinery import ModuleSpec +from typing import TYPE_CHECKING from grimp import exceptions -from grimp.application.ports.filesystem import AbstractFileSystem from grimp.application.ports.packagefinder import AbstractPackageFinder from grimp.domain.valueobjects import Module +if TYPE_CHECKING: + from importlib.machinery import ModuleSpec + + from grimp.application.ports.filesystem import AbstractFileSystem + logger = logging.getLogger(__name__) @@ -19,7 +25,8 @@ def determine_package_directories( spec = importlib.util.find_spec(package_name) if not spec: logger.debug(f"sys.path: {sys.path}") - raise ValueError(f"Could not find package '{package_name}' in your Python path.") + msg = f"Could not find package '{package_name}' in your Python path." + raise ValueError(msg) if ( spec.has_location @@ -30,11 +37,14 @@ def determine_package_directories( ): raise exceptions.NotATopLevelModule - assert spec.submodule_search_locations # This should be the case if spec.has_location. + if not spec.submodule_search_locations: + # This should never be the case the case if spec.has_location. + raise AssertionError return set(spec.submodule_search_locations) def _is_a_package(self, spec: ModuleSpec, file_system: AbstractFileSystem) -> bool: - assert spec.origin + if not spec.origin: + raise AssertionError filename = file_system.split(spec.origin)[1] return filename == "__init__.py" @@ -46,5 +56,6 @@ def _has_a_non_namespace_parent(self, spec: ModuleSpec) -> bool: return False root_spec = importlib.util.find_spec(module.parent.name) - assert root_spec + if not root_spec: + raise AssertionError return root_spec.has_location diff --git a/src/grimp/application/config.py b/src/grimp/application/config.py index 9a7879c6..6bec409e 100644 --- a/src/grimp/application/config.py +++ b/src/grimp/application/config.py @@ -1,19 +1,21 @@ +from __future__ import annotations + from typing import Any class Settings: - def __init__(self): - self._config = {} + def __init__(self) -> None: + self._config: dict[str, Any] = {} - def configure(self, **config_dict: Any): + def configure(self, **config_dict: Any) -> None: self._config.update(config_dict) - def __getattr__(self, name): + def __getattr__(self, name: str) -> Any: if name[:2] != "__": return self._config[name] - return super().__getattr__(name) + return super().__getattribute__(name) - def copy(self) -> "Settings": + def copy(self) -> Settings: new_instance = self.__class__() new_instance.configure(**self._config) return new_instance diff --git a/src/grimp/application/graph.py b/src/grimp/application/graph.py index 0847511c..07778c80 100644 --- a/src/grimp/application/graph.py +++ b/src/grimp/application/graph.py @@ -1,16 +1,20 @@ from __future__ import annotations -from typing import TypedDict -from collections.abc import Sequence + +from typing import TYPE_CHECKING, TypedDict + +from grimp import _rustgrimp as rust # type: ignore[attr-defined] from grimp.domain.analysis import PackageDependency, Route from grimp.domain.valueobjects import Layer -from grimp import _rustgrimp as rust # type: ignore[attr-defined] from grimp.exceptions import ( + InvalidImportExpression, + InvalidModuleExpression, ModuleNotPresent, NoSuchContainer, - InvalidModuleExpression, - InvalidImportExpression, ) +if TYPE_CHECKING: + from collections.abc import Sequence + class Import(TypedDict): importer: str @@ -53,24 +57,20 @@ def find_matching_modules(self, expression: str) -> set[str]: """ Find all modules matching the passed expression. + A module expression is used to refer to sets of modules. + - ``*`` stands in for a module name, without including subpackages. + - ``**`` includes subpackages too. + Args: expression: A module expression used for matching. + Returns: A set of module names matching the expression. + Raises: InvalidModuleExpression if the passed expression is invalid. - Module Expressions - ================== - - A module expression is used to refer to sets of modules. - - - ``*`` stands in for a module name, without including subpackages. - - ``**`` includes subpackages too. - - Examples - -------- - + Examples: - ``mypackage.foo``: matches ``mypackage.foo`` exactly. - ``mypackage.*``: matches ``mypackage.foo`` but not ``mypackage.foo.bar``. - ``mypackage.*.baz``: matches ``mypackage.foo.baz`` but not ``mypackage.foo.bar.baz``. @@ -118,7 +118,8 @@ def squash_module(self, module: str) -> None: """ self._cached_modules = None if not self._rustgraph.contains_module(module): - raise ModuleNotPresent(f'"{module}" not present in the graph.') + msg = f'"{module}" not present in the graph.' + raise ModuleNotPresent(msg) self._rustgraph.squash_module(module) def is_module_squashed(self, module: str) -> bool: @@ -128,7 +129,8 @@ def is_module_squashed(self, module: str) -> bool: If the module is not present in the graph, grimp.exceptions.ModuleNotPresent will be raised. """ if not self._rustgraph.contains_module(module): - raise ModuleNotPresent(f'"{module}" not present in the graph.') + msg = f'"{module}" not present in the graph.' + raise ModuleNotPresent(msg) return self._rustgraph.is_module_squashed(module) def add_import( @@ -178,7 +180,8 @@ def find_children(self, module: str) -> set[str]: # It doesn't make sense to find the children of a squashed module, as we don't store # the children in the graph. if self.is_module_squashed(module): - raise ValueError("Cannot find children of a squashed module.") + msg = "Cannot find children of a squashed module." + raise ValueError(msg) return self._rustgraph.find_children(module) def find_descendants(self, module: str) -> set[str]: @@ -192,7 +195,8 @@ def find_descendants(self, module: str) -> set[str]: # It doesn't make sense to find the descendants of a squashed module, as we don't store # the descendants in the graph. if self.is_module_squashed(module): - raise ValueError("Cannot find descendants of a squashed module.") + msg = "Cannot find descendants of a squashed module." + raise ValueError(msg) return self._rustgraph.find_descendants(module) # Direct imports @@ -225,7 +229,9 @@ def find_modules_that_directly_import(self, module: str) -> set[str]: def get_import_details(self, *, importer: str, imported: str) -> list[DetailedImport]: """ - Return available metadata relating to the direct imports between two modules, in the form: + Return available metadata relating to the direct imports between two modules. + + This returns a list of dictionaries in the following form: [ { 'importer': 'mypackage.importer', @@ -265,10 +271,12 @@ def find_matching_direct_imports(self, import_expression: str) -> list[Import]: import_expression: An expression used for matching importing modules, in the form "importer_expression -> imported_expression", where both expressions are module expressions. + Returns: A list of direct imports matching the expressions, ordered alphabetically by importer, then imported. (We return a list rather than a set purely because dictionaries aren't hashable.) + Raises: InvalidImportExpression if either of the passed expressions are invalid. @@ -277,16 +285,16 @@ def find_matching_direct_imports(self, import_expression: str) -> list[Import]: try: importer_expression, imported_expression = import_expression.split(" -> ") except ValueError: - raise InvalidImportExpression(f"{import_expression} is not a valid import expression.") + msg = f"{import_expression} is not a valid import expression." + raise InvalidImportExpression(msg) from None try: return self._rustgraph.find_matching_direct_imports( importer_expression=importer_expression, imported_expression=imported_expression ) except rust.InvalidModuleExpression as e: - raise InvalidImportExpression( - f"{import_expression} is not a valid import expression." - ) from e + msg = f"{import_expression} is not a valid import expression." + raise InvalidImportExpression(msg) from e # Indirect imports # ---------------- @@ -295,6 +303,7 @@ def find_downstream_modules(self, module: str, as_package: bool = False) -> set[ """ Return a set of the names of all the modules that import (even indirectly) the supplied module name. + Args: module: The absolute name of the upstream Module. as_package: Whether or not to treat the supplied module as an individual module, @@ -303,7 +312,6 @@ def find_downstream_modules(self, module: str, as_package: bool = False) -> set[ modules *external* to the subpackage, and won't include modules within the subpackage. Usage: - # Returns the modules downstream of mypackage.foo. import_graph.find_downstream_modules('mypackage.foo') @@ -346,7 +354,8 @@ def find_shortest_chain( """ for module in (importer, imported): if not self._rustgraph.contains_module(module): - raise ValueError(f"Module {module} is not present in the graph.") + msg = f"Module {module} is not present in the graph." + raise ValueError(msg) chain = self._rustgraph.find_shortest_chain(importer, imported, as_packages) return tuple(chain) if chain else None @@ -411,13 +420,12 @@ def find_illegal_dependencies_for_layers( all import chains from high -> low must go via mid. Arguments: - - - layers: A sequence, each element of which consists either of a `Layer`, the name + layers: A sequence, each element of which consists either of a `Layer`, the name of a layer module or a set of sibling modules. If containers are also specified, then these names must be relative to the container. The order is from higher to lower level layers. Any layers that don't exist in the graph will be ignored. - - containers: The parent modules of the layers, as absolute names that you could import, + containers: The parent modules of the layers, as absolute names that you could import, such as "mypackage.foo". (Optional.) Returns the illegal dependencies in the form of a set of PackageDependency objects. @@ -441,7 +449,7 @@ def find_illegal_dependencies_for_layers( containers=set(containers) if containers else set(), ) except rust.NoSuchContainer as e: - raise NoSuchContainer(str(e)) + raise NoSuchContainer(str(e)) from None return _dependencies_from_tuple(result) @@ -450,7 +458,8 @@ def nominate_cycle_breakers(self, package: str) -> set[ImportTuple]: Identify a set of imports that, if removed, would make the package locally acyclic. """ if not self._rustgraph.contains_module(package): - raise ModuleNotPresent(f'"{package}" not present in the graph.') + msg = f'"{package}" not present in the graph.' + raise ModuleNotPresent(msg) return self._rustgraph.nominate_cycle_breakers(package) # Dunder methods @@ -458,8 +467,9 @@ def nominate_cycle_breakers(self, package: str) -> set[ImportTuple]: def __repr__(self) -> str: """ - Display the instance in one of the following ways: + Display the instance. + This can be one of the following ways: diff --git a/src/grimp/application/ports/caching.py b/src/grimp/application/ports/caching.py index c271ddd4..38e62093 100644 --- a/src/grimp/application/ports/caching.py +++ b/src/grimp/application/ports/caching.py @@ -1,7 +1,12 @@ -from grimp.application.ports.modulefinder import FoundPackage, ModuleFile -from grimp.domain.valueobjects import DirectImport, Module +from __future__ import annotations -from .filesystem import BasicFileSystem +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from grimp.application.ports.modulefinder import FoundPackage, ModuleFile + from grimp.domain.valueobjects import DirectImport, Module + + from .filesystem import BasicFileSystem class CacheMiss(Exception): @@ -35,7 +40,7 @@ def setup( include_external_packages: bool, exclude_type_checking_imports: bool = False, cache_dir: str | None = None, - ) -> "Cache": + ) -> Cache: cache = cls( file_system=file_system, found_packages=found_packages, diff --git a/src/grimp/application/ports/filesystem.py b/src/grimp/application/ports/filesystem.py index 268b1efe..18b04cbc 100644 --- a/src/grimp/application/ports/filesystem.py +++ b/src/grimp/application/ports/filesystem.py @@ -1,7 +1,10 @@ from __future__ import annotations + import abc -from collections.abc import Iterator -from typing import Protocol +from typing import TYPE_CHECKING, Protocol + +if TYPE_CHECKING: + from collections.abc import Iterator class AbstractFileSystem(abc.ABC): @@ -12,7 +15,7 @@ class AbstractFileSystem(abc.ABC): @property @abc.abstractmethod def sep(self) -> str: - """ + r""" Return the file separator for the FileSystem. E.G. '/' for UNIX systems and '\\' for Windows systems diff --git a/src/grimp/application/ports/modulefinder.py b/src/grimp/application/ports/modulefinder.py index 779e6b7e..dcc88450 100644 --- a/src/grimp/application/ports/modulefinder.py +++ b/src/grimp/application/ports/modulefinder.py @@ -1,10 +1,15 @@ +from __future__ import annotations + import abc -from collections.abc import Set from dataclasses import dataclass +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Set as AbstractSet -from grimp.domain.valueobjects import Module + from grimp.domain.valueobjects import Module -from .filesystem import AbstractFileSystem + from .filesystem import AbstractFileSystem @dataclass(frozen=True, order=True) @@ -21,8 +26,8 @@ class FoundPackage: name: str directory: str - module_files: Set[ModuleFile] - namespace_packages: Set[str] = frozenset() + module_files: AbstractSet[ModuleFile] + namespace_packages: AbstractSet[str] = frozenset() class AbstractModuleFinder(abc.ABC): diff --git a/src/grimp/application/ports/packagefinder.py b/src/grimp/application/ports/packagefinder.py index 30b6d0ed..4ccd7e9d 100644 --- a/src/grimp/application/ports/packagefinder.py +++ b/src/grimp/application/ports/packagefinder.py @@ -1,6 +1,10 @@ +from __future__ import annotations + import abc +from typing import TYPE_CHECKING -from .filesystem import AbstractFileSystem +if TYPE_CHECKING: + from .filesystem import AbstractFileSystem class AbstractPackageFinder(abc.ABC): diff --git a/src/grimp/application/ports/timing.py b/src/grimp/application/ports/timing.py index 2ff4a8f9..2d716c0d 100644 --- a/src/grimp/application/ports/timing.py +++ b/src/grimp/application/ports/timing.py @@ -1,7 +1,10 @@ from __future__ import annotations import abc -from types import TracebackType +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from types import TracebackType class Timer(abc.ABC): diff --git a/src/grimp/application/scanning.py b/src/grimp/application/scanning.py index 09eab3c8..e20de536 100644 --- a/src/grimp/application/scanning.py +++ b/src/grimp/application/scanning.py @@ -1,10 +1,16 @@ -from collections.abc import Collection +from __future__ import annotations + +from typing import TYPE_CHECKING from grimp import _rustgrimp as rust # type: ignore[attr-defined] -from grimp.domain.valueobjects import DirectImport, Module from grimp.application.config import settings -from grimp.application.ports.filesystem import AbstractFileSystem -from grimp.application.ports.modulefinder import ModuleFile, FoundPackage + +if TYPE_CHECKING: + from collections.abc import Collection + + from grimp.application.ports.filesystem import AbstractFileSystem + from grimp.application.ports.modulefinder import FoundPackage, ModuleFile + from grimp.domain.valueobjects import DirectImport, Module def scan_imports( diff --git a/src/grimp/application/usecases.py b/src/grimp/application/usecases.py index baa30b42..efb73167 100644 --- a/src/grimp/application/usecases.py +++ b/src/grimp/application/usecases.py @@ -2,18 +2,24 @@ Use cases handle application logic. """ -from typing import cast +from __future__ import annotations + import itertools -from collections.abc import Sequence, Iterable +from typing import TYPE_CHECKING, cast -from .scanning import scan_imports from ..application.ports import caching -from ..application.ports.filesystem import AbstractFileSystem, BasicFileSystem -from ..application.graph import ImportGraph -from ..application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile -from ..application.ports.packagefinder import AbstractPackageFinder -from ..domain.valueobjects import DirectImport, Module +from ..domain.valueobjects import Module from .config import settings +from .scanning import scan_imports + +if TYPE_CHECKING: + from collections.abc import Iterable, Sequence + + from ..application.graph import ImportGraph + from ..application.ports.filesystem import AbstractFileSystem, BasicFileSystem + from ..application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile + from ..application.ports.packagefinder import AbstractPackageFinder + from ..domain.valueobjects import DirectImport class NotSupplied: @@ -21,8 +27,8 @@ class NotSupplied: def build_graph( - package_name, - *additional_package_names, + package_name: str, + *additional_package_names: str, include_external_packages: bool = False, exclude_type_checking_imports: bool = False, cache_dir: str | type[NotSupplied] | None = NotSupplied, @@ -31,13 +37,13 @@ def build_graph( Build and return an import graph for the supplied package name(s). Args: - - package_name: the name of the top level package for which to build the graph. - - additional_package_names: tuple of additional packages to build the graph from. - - include_external_packages: whether to include any external packages in the graph. - - exclude_type_checking_imports: whether to exclude imports made in type checking guards. - - cache_dir: The directory to use for caching the graph. - Examples: + package_name: the name of the top level package for which to build the graph. + additional_package_names: tuple of additional packages to build the graph from. + include_external_packages: whether to include any external packages in the graph. + exclude_type_checking_imports: whether to exclude imports made in type checking guards. + cache_dir: The directory to use for caching the graph. + Examples: # Single package. graph = build_graph("mypackage") graph = build_graph("mypackage", include_external_packages=True) @@ -99,13 +105,15 @@ def _validate_package_names_are_strings( ) -> Sequence[str]: for name in package_names: if not isinstance(name, str): - raise TypeError(f"Package names must be strings, got {name.__class__.__name__}.") - return cast(Sequence[str], package_names) + msg = f"Package names must be strings, got {name.__class__.__name__}." + raise TypeError(msg) + return cast("Sequence[str]", package_names) def _scan_packages( found_packages: set[FoundPackage], file_system: BasicFileSystem, + *, include_external_packages: bool, exclude_type_checking_imports: bool, cache_dir: str | type[NotSupplied] | None, diff --git a/src/grimp/domain/analysis.py b/src/grimp/domain/analysis.py index 88a54179..8121c3bc 100644 --- a/src/grimp/domain/analysis.py +++ b/src/grimp/domain/analysis.py @@ -1,7 +1,10 @@ from __future__ import annotations from dataclasses import dataclass -from collections.abc import Iterable, Sequence +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from collections.abc import Iterable, Sequence @dataclass(frozen=True, order=True) @@ -32,7 +35,6 @@ def new( Optional constructor for a Route with more permissive input types. Example: - Route.new( heads={"foo"}, middle=["bar", "baz"], @@ -52,7 +54,6 @@ def single_chained(cls, *modules: str) -> Route: Optional constructor for a Route with a single chain. Example: - Route.single_chained("foo", "bar", "baz") """ @@ -89,7 +90,6 @@ def new( Optional constructor for a PackageDependency with more permissive input types. Example: - PackageDependency.new( importer="foo", imported="bar", diff --git a/src/grimp/domain/valueobjects.py b/src/grimp/domain/valueobjects.py index 66bb60fb..bdc050fb 100644 --- a/src/grimp/domain/valueobjects.py +++ b/src/grimp/domain/valueobjects.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from dataclasses import dataclass @@ -18,27 +20,28 @@ def package_name(self) -> str: return self.name.split(".")[0] @property - def root(self) -> "Module": + def root(self) -> Module: """ The root package. """ return Module(self.package_name) @property - def parent(self) -> "Module": + def parent(self) -> Module: components = self.name.split(".") if len(components) == 1: - raise ValueError("Module has no parent.") + msg = "Module has no parent." + raise ValueError(msg) return Module(".".join(components[:-1])) - def is_child_of(self, module: "Module") -> bool: + def is_child_of(self, module: Module) -> bool: try: return module == self.parent except ValueError: # If this module has no parent, then it cannot be a child of the supplied module. return False - def is_descendant_of(self, module: "Module") -> bool: + def is_descendant_of(self, module: Module) -> bool: return self.name.startswith(f"{module.name}.") diff --git a/src/grimp/exceptions.py b/src/grimp/exceptions.py index 43ec9823..338b0f1e 100644 --- a/src/grimp/exceptions.py +++ b/src/grimp/exceptions.py @@ -1,3 +1,6 @@ +from __future__ import annotations + + class GrimpException(Exception): """ Base exception for all custom Grimp exceptions to inherit. @@ -47,19 +50,22 @@ def __init__(self, filename: str, lineno: int | None, text: str | None) -> None: self.lineno = lineno self.text = text - def __str__(self): + def __str__(self) -> str: lineno = self.lineno or "?" text = self.text or "" return f"Syntax error in {self.filename}, line {lineno}: {text}" - def __eq__(self, other): + def __eq__(self, other: object) -> bool: + if not isinstance(other, SourceSyntaxError): + return NotImplemented + return (self.filename, self.lineno, self.text) == ( other.filename, other.lineno, other.text, ) - def __reduce__(self): + def __reduce__(self) -> tuple[type[SourceSyntaxError], tuple[str, int | None, str | None]]: # Implement __reduce__ to make this exception pickleable, # allowing it to be sent between processes. return SourceSyntaxError, (self.filename, self.lineno, self.text) diff --git a/src/grimp/main.py b/src/grimp/main.py index 45a297a0..57d02f47 100644 --- a/src/grimp/main.py +++ b/src/grimp/main.py @@ -2,11 +2,11 @@ from .adaptors.caching import Cache from .adaptors.filesystem import FileSystem -from .application.graph import ImportGraph from .adaptors.modulefinder import ModuleFinder from .adaptors.packagefinder import ImportLibPackageFinder from .adaptors.timing import SystemClockTimer from .application.config import settings +from .application.graph import ImportGraph from .application.usecases import build_graph settings.configure( diff --git a/tests/adaptors/filesystem.py b/tests/adaptors/filesystem.py index 4903ca11..a6cb1810 100644 --- a/tests/adaptors/filesystem.py +++ b/tests/adaptors/filesystem.py @@ -1,10 +1,16 @@ -from typing import Any -from collections.abc import Generator +from __future__ import annotations + +from typing import TYPE_CHECKING, Any import yaml -from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem from grimp import _rustgrimp as rust # type: ignore[attr-defined] +from grimp.application.ports.filesystem import AbstractFileSystem + +if TYPE_CHECKING: + from collections.abc import Generator + + from grimp.application.ports.filesystem import BasicFileSystem DEFAULT_MTIME = 10000.0 diff --git a/tests/adaptors/modulefinder.py b/tests/adaptors/modulefinder.py index 423fb3db..c3fe68ea 100644 --- a/tests/adaptors/modulefinder.py +++ b/tests/adaptors/modulefinder.py @@ -1,5 +1,12 @@ -from grimp.application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile -from grimp.application.ports.filesystem import AbstractFileSystem +from __future__ import annotations + +from typing import TYPE_CHECKING + +from grimp.application.ports.modulefinder import AbstractModuleFinder, FoundPackage + +if TYPE_CHECKING: + from grimp.application.ports.filesystem import AbstractFileSystem + from grimp.application.ports.modulefinder import ModuleFile class BaseFakeModuleFinder(AbstractModuleFinder): diff --git a/tests/adaptors/packagefinder.py b/tests/adaptors/packagefinder.py index 5740677d..787f5541 100644 --- a/tests/adaptors/packagefinder.py +++ b/tests/adaptors/packagefinder.py @@ -1,7 +1,13 @@ -from collections.abc import Mapping +from __future__ import annotations + +from typing import TYPE_CHECKING + from grimp.application.ports.packagefinder import AbstractPackageFinder -from grimp.application.ports.filesystem import AbstractFileSystem +if TYPE_CHECKING: + from collections.abc import Mapping + + from grimp.application.ports.filesystem import AbstractFileSystem class BaseFakePackageFinder(AbstractPackageFinder): diff --git a/tests/adaptors/timing.py b/tests/adaptors/timing.py index 30f42ca0..f48f5280 100644 --- a/tests/adaptors/timing.py +++ b/tests/adaptors/timing.py @@ -1,9 +1,12 @@ from __future__ import annotations -from types import TracebackType +from typing import TYPE_CHECKING from grimp.application.ports.timing import Timer +if TYPE_CHECKING: + from types import TracebackType + class FakeTimer(Timer): ARBITRARY_SECONDS_SINCE_EPOCH = 1_000_000 diff --git a/tests/benchmarking/test_benchmarking.py b/tests/benchmarking/test_benchmarking.py index e5ce4167..abcf0d16 100644 --- a/tests/benchmarking/test_benchmarking.py +++ b/tests/benchmarking/test_benchmarking.py @@ -1,14 +1,15 @@ -import uuid -import random -import pytest -import json import importlib +import json +import random +import uuid +from copy import deepcopy from pathlib import Path -from grimp.application.graph import ImportGraph -from grimp import PackageDependency, Route +import pytest + import grimp -from copy import deepcopy +from grimp import PackageDependency, Route +from grimp.application.graph import ImportGraph @pytest.fixture(scope="module") @@ -354,7 +355,7 @@ def test_build_django_from_cache_a_few_misses(benchmark, number_of_misses: int): grimp.build_graph("django") # Add some modules which won't be in the cache. # (Use some real python, which will take time to parse.) - django_path = Path(importlib.util.find_spec("django").origin).parent # type: ignore + django_path = Path(importlib.util.find_spec("django").origin).parent # type: ignore[arg-type,union-attr] module_to_copy = django_path / "forms" / "forms.py" module_contents = module_to_copy.read_text() extra_modules = [ diff --git a/tests/config.py b/tests/config.py index 6a00dea6..cfae0556 100644 --- a/tests/config.py +++ b/tests/config.py @@ -1,7 +1,7 @@ from grimp.application.config import settings -class override_settings: +class override_settings: # noqa: N801 def __init__(self, **settings_to_override): self.settings_to_override = settings_to_override diff --git a/tests/functional/test_build_and_use_graph.py b/tests/functional/test_build_and_use_graph.py index ed151598..1d2f603a 100644 --- a/tests/functional/test_build_and_use_graph.py +++ b/tests/functional/test_build_and_use_graph.py @@ -1,6 +1,6 @@ -from grimp import build_graph import pytest +from grimp import build_graph """ For ease of reference, these are the imports of all the files: diff --git a/tests/functional/test_build_and_use_graph_with_multiple_roots.py b/tests/functional/test_build_and_use_graph_with_multiple_roots.py index 4c0efdcf..d8fda118 100644 --- a/tests/functional/test_build_and_use_graph_with_multiple_roots.py +++ b/tests/functional/test_build_and_use_graph_with_multiple_roots.py @@ -1,4 +1,5 @@ -import pytest # type: ignore +import pytest + from grimp import build_graph """ diff --git a/tests/functional/test_build_graph_on_real_packages.py b/tests/functional/test_build_graph_on_real_packages.py index b2da12af..63e5a4bf 100644 --- a/tests/functional/test_build_graph_on_real_packages.py +++ b/tests/functional/test_build_graph_on_real_packages.py @@ -1,4 +1,4 @@ -import pytest # type: ignore +import pytest import grimp diff --git a/tests/functional/test_caching.py b/tests/functional/test_caching.py index 1916abf9..f69ea8aa 100644 --- a/tests/functional/test_caching.py +++ b/tests/functional/test_caching.py @@ -2,7 +2,7 @@ import tempfile from pathlib import Path -import pytest # type: ignore +import pytest from grimp import build_graph diff --git a/tests/functional/test_error_handling.py b/tests/functional/test_error_handling.py index e79914ba..0bb1eca5 100644 --- a/tests/functional/test_error_handling.py +++ b/tests/functional/test_error_handling.py @@ -1,6 +1,6 @@ import os -import pytest # type: ignore +import pytest from grimp import build_graph, exceptions diff --git a/tests/functional/test_namespace_packages.py b/tests/functional/test_namespace_packages.py index ac47d2cb..53c80424 100644 --- a/tests/functional/test_namespace_packages.py +++ b/tests/functional/test_namespace_packages.py @@ -1,4 +1,4 @@ -import pytest # type: ignore +import pytest from grimp import build_graph diff --git a/tests/unit/adaptors/test_caching.py b/tests/unit/adaptors/test_caching.py index 3f6576db..289d1ae2 100644 --- a/tests/unit/adaptors/test_caching.py +++ b/tests/unit/adaptors/test_caching.py @@ -1,13 +1,13 @@ import json import logging -import pytest # type: ignore +import pytest +from grimp import _rustgrimp as rust # type: ignore[attr-defined] from grimp.adaptors.caching import Cache, CacheFileNamer from grimp.application.ports.caching import CacheMiss from grimp.application.ports.modulefinder import FoundPackage, ModuleFile from grimp.domain.valueobjects import DirectImport, Module -from grimp import _rustgrimp as rust # type: ignore[attr-defined] class SimplisticFileNamer(CacheFileNamer): diff --git a/tests/unit/adaptors/test_filesystem.py b/tests/unit/adaptors/test_filesystem.py index 27dbbf17..b8ddf2c5 100644 --- a/tests/unit/adaptors/test_filesystem.py +++ b/tests/unit/adaptors/test_filesystem.py @@ -1,9 +1,15 @@ -from typing import TypeAlias +from __future__ import annotations + from copy import copy -import pytest # type: ignore -from grimp.application.ports.filesystem import BasicFileSystem -from tests.adaptors.filesystem import FakeFileSystem +from typing import TYPE_CHECKING, TypeAlias + +import pytest + from grimp import _rustgrimp as rust # type: ignore[attr-defined] +from tests.adaptors.filesystem import FakeFileSystem + +if TYPE_CHECKING: + from grimp.application.ports.filesystem import BasicFileSystem class _Base: diff --git a/tests/unit/adaptors/test_modulefinder.py b/tests/unit/adaptors/test_modulefinder.py index 8cf2e3fc..ad5a84a6 100644 --- a/tests/unit/adaptors/test_modulefinder.py +++ b/tests/unit/adaptors/test_modulefinder.py @@ -1,8 +1,9 @@ +import pytest + from grimp.adaptors.modulefinder import ModuleFinder from grimp.application.ports.modulefinder import FoundPackage, ModuleFile from grimp.domain.valueobjects import Module from tests.adaptors.filesystem import DEFAULT_MTIME, FakeFileSystem -import pytest def test_happy_path(): @@ -131,8 +132,6 @@ def test_namespaced_packages(package_name: str, package_directory: str, expected non_python_directory/ six/ README.txt - - """ ) @@ -218,14 +217,14 @@ def test_ignores_invalid_identifier_directories(): module_files = { ModuleFile(module=Module(name), mtime=DEFAULT_MTIME) - for name in { + for name in ( "namespacepackage.foo.valid_underscore_name", "namespacepackage.foo.valid_underscore_name.mod", "namespacepackage.foo.valid_non_àscii", "namespacepackage.foo.valid_non_àscii.mod", "namespacepackage.bar", "namespacepackage.bar.mod", - } + ) } assert result == FoundPackage( name="namespacepackage", diff --git a/tests/unit/adaptors/test_packagefinder.py b/tests/unit/adaptors/test_packagefinder.py index f7f2de62..2f687a29 100644 --- a/tests/unit/adaptors/test_packagefinder.py +++ b/tests/unit/adaptors/test_packagefinder.py @@ -1,10 +1,10 @@ from pathlib import Path -import pytest # type: ignore +import pytest from grimp import exceptions -from grimp.adaptors.packagefinder import ImportLibPackageFinder from grimp.adaptors.filesystem import FileSystem +from grimp.adaptors.packagefinder import ImportLibPackageFinder from tests.adaptors.filesystem import FakeFileSystem assets = (Path(__file__).parent.parent.parent / "assets").resolve() diff --git a/tests/unit/application/graph/test_chains.py b/tests/unit/application/graph/test_chains.py index 3fa3039e..4f960c1e 100644 --- a/tests/unit/application/graph/test_chains.py +++ b/tests/unit/application/graph/test_chains.py @@ -1,5 +1,6 @@ import re -import pytest # type: ignore + +import pytest from grimp.application.graph import ImportGraph diff --git a/tests/unit/application/graph/test_cycle_breakers.py b/tests/unit/application/graph/test_cycle_breakers.py index 7e78881d..a829a6a7 100644 --- a/tests/unit/application/graph/test_cycle_breakers.py +++ b/tests/unit/application/graph/test_cycle_breakers.py @@ -1,5 +1,6 @@ -from grimp.application.graph import ImportGraph import pytest + +from grimp.application.graph import ImportGraph from grimp.exceptions import ModuleNotPresent diff --git a/tests/unit/application/graph/test_direct_imports.py b/tests/unit/application/graph/test_direct_imports.py index 871064a2..c70112ac 100644 --- a/tests/unit/application/graph/test_direct_imports.py +++ b/tests/unit/application/graph/test_direct_imports.py @@ -1,7 +1,8 @@ -import pytest # type: ignore +import re + +import pytest from grimp.application.graph import ImportGraph -import re from grimp.exceptions import InvalidImportExpression diff --git a/tests/unit/application/graph/test_hierarchy.py b/tests/unit/application/graph/test_hierarchy.py index ce373dab..fa3550c1 100644 --- a/tests/unit/application/graph/test_hierarchy.py +++ b/tests/unit/application/graph/test_hierarchy.py @@ -1,8 +1,9 @@ import re -import pytest # type: ignore + +import pytest from grimp.application.graph import ImportGraph -from grimp.exceptions import ModuleNotPresent, InvalidModuleExpression +from grimp.exceptions import InvalidModuleExpression, ModuleNotPresent @pytest.mark.parametrize( diff --git a/tests/unit/application/graph/test_layers.py b/tests/unit/application/graph/test_layers.py index 766ec2a3..210d33ea 100644 --- a/tests/unit/application/graph/test_layers.py +++ b/tests/unit/application/graph/test_layers.py @@ -3,14 +3,17 @@ import itertools import logging import re -from collections.abc import Iterable +from typing import TYPE_CHECKING -import pytest # type: ignore +import pytest from grimp import PackageDependency, Route from grimp.application.graph import ImportGraph -from grimp.exceptions import NoSuchContainer from grimp.domain.valueobjects import Layer +from grimp.exceptions import NoSuchContainer + +if TYPE_CHECKING: + from collections.abc import Iterable class TestSingleOrNoContainer: @@ -958,7 +961,7 @@ def _pairwise(iterable): """ a, b = itertools.tee(iterable) next(b, None) - return zip(a, b) + return zip(a, b, strict=False) class TestClosedLayers: diff --git a/tests/unit/application/graph/test_manipulation.py b/tests/unit/application/graph/test_manipulation.py index 8824647c..d4854855 100644 --- a/tests/unit/application/graph/test_manipulation.py +++ b/tests/unit/application/graph/test_manipulation.py @@ -1,5 +1,6 @@ import re -import pytest # type: ignore + +import pytest from grimp.application.graph import ImportGraph from grimp.exceptions import ModuleNotPresent diff --git a/tests/unit/application/graph/test_misc.py b/tests/unit/application/graph/test_misc.py index b0a4fa3a..c3668fa8 100644 --- a/tests/unit/application/graph/test_misc.py +++ b/tests/unit/application/graph/test_misc.py @@ -1,4 +1,4 @@ -import pytest # type: ignore +import pytest from grimp.application.graph import ImportGraph from grimp.exceptions import ModuleNotPresent diff --git a/tests/unit/application/test_scanning.py b/tests/unit/application/test_scanning.py index c0c1447a..4842eedd 100644 --- a/tests/unit/application/test_scanning.py +++ b/tests/unit/application/test_scanning.py @@ -1,11 +1,17 @@ -import pytest # type: ignore +from __future__ import annotations -from grimp.application.ports.modulefinder import FoundPackage, ModuleFile +from typing import TYPE_CHECKING + +import pytest + +from grimp import _rustgrimp as rust # type: ignore[attr-defined] from grimp.application import scanning +from grimp.application.ports.modulefinder import FoundPackage, ModuleFile from grimp.domain.valueobjects import DirectImport, Module from tests.config import override_settings -from grimp import _rustgrimp as rust # type: ignore[attr-defined] -from collections.abc import Set + +if TYPE_CHECKING: + from collections.abc import Set as AbstractSet @pytest.mark.parametrize( @@ -973,5 +979,5 @@ def _module_to_module_file(module: Module) -> ModuleFile: return ModuleFile(module=module, mtime=some_mtime) -def _modules_to_module_files(modules: Set[Module]) -> frozenset[ModuleFile]: +def _modules_to_module_files(modules: AbstractSet[Module]) -> frozenset[ModuleFile]: return frozenset({_module_to_module_file(module) for module in modules}) diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index e7f2ae26..96a389f1 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -1,15 +1,21 @@ +from __future__ import annotations + import re +from typing import TYPE_CHECKING from unittest.mock import sentinel -import pytest # type: ignore + +import pytest from grimp.application import usecases from grimp.application.ports.caching import Cache -from grimp.application.ports.modulefinder import ModuleFile -from grimp.domain.valueobjects import DirectImport, Module from tests.adaptors.filesystem import FakeFileSystem from tests.adaptors.packagefinder import BaseFakePackageFinder from tests.config import override_settings +if TYPE_CHECKING: + from grimp.application.ports.modulefinder import ModuleFile + from grimp.domain.valueobjects import DirectImport, Module + class TestBuildGraph: @pytest.mark.parametrize("include_external_packages", (True, False)) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 4c1dc52e..943d9522 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,8 +1,8 @@ -import pytest # type: ignore +import pytest +from grimp.adaptors.modulefinder import ModuleFinder from grimp.application.config import settings from grimp.application.graph import ImportGraph -from grimp.adaptors.modulefinder import ModuleFinder @pytest.fixture(scope="module", autouse=True) diff --git a/tests/unit/domain/test_valueobjects.py b/tests/unit/domain/test_valueobjects.py index cc883397..c6a07cd4 100644 --- a/tests/unit/domain/test_valueobjects.py +++ b/tests/unit/domain/test_valueobjects.py @@ -1,6 +1,6 @@ -import pytest # type: ignore +import pytest -from grimp.domain.valueobjects import DirectImport, Module, Layer +from grimp.domain.valueobjects import DirectImport, Layer, Module class TestModule: From c390810732164374c72f479901717ba1b64e352b Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Fri, 19 Dec 2025 13:29:09 +1300 Subject: [PATCH 2/9] Remove hyphens from docstring --- src/grimp/adaptors/modulefinder.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index d355d50e..b375cdfd 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -174,8 +174,8 @@ def _namespace_from_dir( Args: package_name: the importable name of the top level package. Could be namespaced. - namespace_dir: - the full name of the namespace directory. - package_directory: - the full path of the top level Python package directory. + namespace_dir: the full name of the namespace directory. + package_directory: the full path of the top level Python package directory. Returns: Absolute module name for importing (string). From ed6dbd73d12cf04bbd0fb5db4154a0c85ba0d9a1 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Fri, 19 Dec 2025 13:35:21 +1300 Subject: [PATCH 3/9] Improve docstring format for `find_illegal_dependencies_for_layers` --- src/grimp/application/graph.py | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/grimp/application/graph.py b/src/grimp/application/graph.py index 07778c80..54e67794 100644 --- a/src/grimp/application/graph.py +++ b/src/grimp/application/graph.py @@ -419,21 +419,23 @@ def find_illegal_dependencies_for_layers( included in the import chain. For example, given the layers high -> mid (closed) -> low then all import chains from high -> low must go via mid. - Arguments: - layers: A sequence, each element of which consists either of a `Layer`, the name - of a layer module or a set of sibling modules. If containers - are also specified, then these names must be relative to the container. - The order is from higher to lower level layers. Any layers that don't - exist in the graph will be ignored. + Args: + layers: A sequence, each element of which consists either of a `Layer`, the name + of a layer module or a set of sibling modules. If containers + are also specified, then these names must be relative to the container. + The order is from higher to lower level layers. Any layers that don't + exist in the graph will be ignored. containers: The parent modules of the layers, as absolute names that you could import, - such as "mypackage.foo". (Optional.) + such as "mypackage.foo". (Optional.) - Returns the illegal dependencies in the form of a set of PackageDependency objects. - Each package dependency is for a different permutation of two layers for which there - is a violation, and contains information about the illegal chains of imports from the - lower layer (the 'upstream') to the higher layer (the 'downstream'). + Returns: + The illegal dependencies in the form of a set of PackageDependency objects. + Each package dependency is for a different permutation of two layers for which there + is a violation, and contains information about the illegal chains of imports from the + lower layer (the 'upstream') to the higher layer (the 'downstream'). - Raises NoSuchContainer if the container is not a module in the graph. + Raises: + NoSuchContainer: if the container is not a module in the graph. """ layers = _parse_layers(layers) try: From f5a681e6f82e6b7a5103ed956ba5ea509559dfe0 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Sat, 20 Dec 2025 09:01:21 +1300 Subject: [PATCH 4/9] Migrate config to `ruff.toml` and add comments regarding the reasons for rule ignores --- .pre-commit-config.yaml | 4 ++-- pyproject.toml | 15 ------------ ruff.toml | 53 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 ruff.toml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 25d05c68..281618e3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,7 +9,7 @@ repos: language: system require_serial: true pass_filenames: false - files: "^pyproject\\.toml|^src/.*|^tests/.*|^docs/conf.py$" + files: "^pyproject\\.toml|ruff\\.toml|^src/.*|^tests/.*|^docs/conf.py$" - id: format-rust name: Format Rust @@ -24,7 +24,7 @@ repos: entry: just lint-python language: system pass_filenames: false - files: "^pyproject\\.toml|^src/.*|^tests/.*|^docs/conf.py$" + files: "^pyproject\\.toml|ruff\\.toml|^src/.*|^tests/.*|^docs/conf.py$" - id: lint-rust name: Lint Rust diff --git a/pyproject.toml b/pyproject.toml index e281961b..4a50f731 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -77,18 +77,3 @@ grimp = ["py.typed"] [tool.setuptools.packages.find] where = ["src"] namespaces = false - -[tool.ruff] -line-length = 99 -exclude = [ - 'tests/assets', # These are sample packages for tests to run under - we don't want ruff to mess with them. - 'docs/', # Documentation configuration scripts -] -lint.select = [ "ALL" ] -lint.ignore = [ "A005", "ANN401" ,"ARG002", "C408", "COM812", "D1", "D2", "E501", "FBT", "FIX", "G004", "N818", "PERF203", "PLR2004", "PLR0913", "PLW1641", "PTH", "PT006", "PT007", "RET504", "SIM108", "SIM118", "SIM300", "TD", "TID252" ] -lint.per-file-ignores."tests/**/*.py" = [ "ANN", "ARG", "B007", "B018", "B033", "D", "EM", "FBT", "N806", "RET505", "RUF012", "RUF059", "S", "TRY" ] -lint.pydocstyle.convention = "google" -lint.future-annotations = true -lint.flake8-builtins.strict-checking = true -lint.flake8-type-checking.quote-annotations = true -lint.typing-extensions = false diff --git a/ruff.toml b/ruff.toml new file mode 100644 index 00000000..e4e9c174 --- /dev/null +++ b/ruff.toml @@ -0,0 +1,53 @@ +line-length = 99 +exclude = [ + 'tests/assets', # These are sample packages for tests to run under - we don't want ruff to mess with them. + 'docs/', # Documentation configuration scripts +] +lint.select = [ "ALL" ] +lint.ignore = [ + "A005", # [stdlib-module-shadowing] Backward Compat. Violated by `modulefinder.py`. + "ANN401", # [any-type] Controversial. We use `Any` type annotations in several places intentionally. + "ARG002", # [unused-method-argument] Backward Compat. + "C408", # [unnecessary-collection-call] Controversial. A micro-optimization like `dict(a=1)` -> `{"a": 1}` that hurts readability. + "COM812", # [missing-trailing-comma] TODO enable. Has a large diff to fix all occurrences. + "D1", # [pydocstyle : Undocumented] TODO enable. Not a priority to have every public interface documented yet. + "D2", # [pydocstyle : Whitespace issues] TODO enable. Has a large diff to fix all occurrences. + "E501", # [line-too-long] Controversial. Long comments and docstrings are sometimes clearer. The formatter already addresses most violations. + "FBT", # [flake8-boolean-trap] Backward Compat. + "FIX", # [flake8-fixme] Controversial. Different projects use different conventions for FIXME/TODO comments. + "G004", # [logging-f-string] Controversial. A micro-optimization but hurts readability. + "N818", # [error-suffix-on-exception-name] Backward Compat. + "PERF203", # [try-except-in-loop] Controversial. A micro-optimization but increases code complexity. + "PLR0913", # [too-many-arguments] Backward Compat. + "PLW1641", # [eq-without-hash] TODO enable. This is low-priority, and basically an enhancement to make classes more ergonomic. + "PTH", # [flake8-use-pathlib] Controversial. `pathlib` adoption is subtle and might be too disruptive for this codebase. e.g. https://github.com/python-grimp/grimp/pull/289#discussion_r2636012008 + "PT007", # [pytest-parametrize-values-wrong-type] TODO enable. Inconsistent use of `@pytest.mark.parametrize("value", ("a", "b", "c"))` versus `...["a", "b", "c"])` in parameterizations. + "RET504", # [unnecessary-assign] Controversial. A micro-optimization but hurts readability. + "RET505", # [superfluous-else-return] Controversial. Explicit `else` blocks can improve readability. + "SIM108", # [if-else-block-instead-of-if-exp] Controversial. Ternary/Binary in-line expressions often hurt readability. + "SIM118", # [in-dict-keys] Controversial. `if key in dict.keys()` sometimes has clearer intent than `if key in dict`. + "TD", # [missing-space-after-todo-colon] Controversial. Different projects use different conventions for FIXME/TODO comments. + "TID252", # [relative-imports] Controversial. There are pros and cons to both relative and absolute imports in a packaging context. +] +lint.per-file-ignores."tests/**/*.py" = [ + "ANN", # [flake8-annotations] Type annotations not needed in a test suite + "ARG", # [flake8-unused-arguments] Interface design doesn't need attention in a test suite + "B007", # [unused-loop-control-variable] Unused variables are common in tests + "B018", # [useless-expression] Unused variables are common in tests + "B033", # [duplicate-value] Can be deliberate in a test suite + "D", # [pydocstyle] Docstrings not needed in a test suite + "EM", # [flake8-errmsg] Error message style doesn't need attention in a test suite + "N806", # [non-lowercase-variable-in-function] Variable naming conventions don't need attention in a test suite + "PLR2004", # [magic-value-comparison] Magic values are common in tests + "RUF012", # [mutable-class-default] Type annotations not needed in a test suite + "RUF059", # [unused-unpacked-variable] Unused variables are common in tests + "S", # [flake8-bandit] Security patterns don't need attention in a test suite + "SIM300", # [yoda-conditions] TODO enable. + "TRY", # [tryceratops] Exception handling patterns don't need attention in a test suite +] +lint.pydocstyle.convention = "google" +lint.future-annotations = true +lint.flake8-builtins.strict-checking = true +lint.flake8-pytest-style.parametrize-names-type = "csv" +lint.flake8-type-checking.quote-annotations = true +lint.typing-extensions = false From 716b13ba2a5c7c75cde11253b89f9f90ebe9097c Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Sat, 20 Dec 2025 09:10:45 +1300 Subject: [PATCH 5/9] Ignore `S101` rule and revert associated changes --- ruff.toml | 1 + src/grimp/adaptors/caching.py | 3 +-- src/grimp/adaptors/packagefinder.py | 10 +++------- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/ruff.toml b/ruff.toml index e4e9c174..15e3f46e 100644 --- a/ruff.toml +++ b/ruff.toml @@ -24,6 +24,7 @@ lint.ignore = [ "PT007", # [pytest-parametrize-values-wrong-type] TODO enable. Inconsistent use of `@pytest.mark.parametrize("value", ("a", "b", "c"))` versus `...["a", "b", "c"])` in parameterizations. "RET504", # [unnecessary-assign] Controversial. A micro-optimization but hurts readability. "RET505", # [superfluous-else-return] Controversial. Explicit `else` blocks can improve readability. + "S101", # [assert] Controversial. `assert` statements are sometimes appropriate for type-checking purposes. "SIM108", # [if-else-block-instead-of-if-exp] Controversial. Ternary/Binary in-line expressions often hurt readability. "SIM118", # [in-dict-keys] Controversial. `if key in dict.keys()` sometimes has clearer intent than `if key in dict`. "TD", # [missing-space-after-todo-colon] Controversial. Different projects use different conventions for FIXME/TODO comments. diff --git a/src/grimp/adaptors/caching.py b/src/grimp/adaptors/caching.py index 01156197..b85a62f8 100644 --- a/src/grimp/adaptors/caching.py +++ b/src/grimp/adaptors/caching.py @@ -97,8 +97,7 @@ def setup( ) cache._build_mtime_map() cache._build_data_map() - if not cache.cache_dir: - raise AssertionError + assert cache.cache_dir return cache @classmethod diff --git a/src/grimp/adaptors/packagefinder.py b/src/grimp/adaptors/packagefinder.py index 5da6955b..539f81ee 100644 --- a/src/grimp/adaptors/packagefinder.py +++ b/src/grimp/adaptors/packagefinder.py @@ -37,14 +37,11 @@ def determine_package_directories( ): raise exceptions.NotATopLevelModule - if not spec.submodule_search_locations: - # This should never be the case the case if spec.has_location. - raise AssertionError + assert spec.submodule_search_locations # This should be the case if spec.has_location. return set(spec.submodule_search_locations) def _is_a_package(self, spec: ModuleSpec, file_system: AbstractFileSystem) -> bool: - if not spec.origin: - raise AssertionError + assert spec.origin filename = file_system.split(spec.origin)[1] return filename == "__init__.py" @@ -56,6 +53,5 @@ def _has_a_non_namespace_parent(self, spec: ModuleSpec) -> bool: return False root_spec = importlib.util.find_spec(module.parent.name) - if not root_spec: - raise AssertionError + assert root_spec return root_spec.has_location From 44ee64df3fed9b0c3d0a69eafeab5265e36b318f Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Sat, 20 Dec 2025 09:12:40 +1300 Subject: [PATCH 6/9] Use exception chaining in some cases, use consistent naming `e` -> `exc` --- src/grimp/adaptors/caching.py | 8 ++++---- src/grimp/application/graph.py | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/grimp/adaptors/caching.py b/src/grimp/adaptors/caching.py index b85a62f8..fc3181e2 100644 --- a/src/grimp/adaptors/caching.py +++ b/src/grimp/adaptors/caching.py @@ -107,17 +107,17 @@ def cache_dir_or_default(cls, cache_dir: str | None) -> str: def read_imports(self, module_file: ModuleFile) -> set[DirectImport]: try: cached_mtime = self._mtime_map[module_file.module.name] - except KeyError: - raise CacheMiss from None + except KeyError as exc: + raise CacheMiss from exc if cached_mtime != module_file.mtime: raise CacheMiss try: return self._data_map[module_file.module] - except KeyError: + except KeyError as exc: # While we would expect the module to be in here, # there's no point in crashing if, for some reason, it's not. - raise CacheMiss from None + raise CacheMiss from exc def write( self, diff --git a/src/grimp/application/graph.py b/src/grimp/application/graph.py index 54e67794..841bad1a 100644 --- a/src/grimp/application/graph.py +++ b/src/grimp/application/graph.py @@ -82,8 +82,8 @@ def find_matching_modules(self, expression: str) -> set[str]: """ try: return self._rustgraph.find_matching_modules(expression) - except rust.InvalidModuleExpression as e: - raise InvalidModuleExpression(str(e)) from e + except rust.InvalidModuleExpression as exc: + raise InvalidModuleExpression(str(exc)) from exc def add_module(self, module: str, is_squashed: bool = False) -> None: """ @@ -284,17 +284,17 @@ def find_matching_direct_imports(self, import_expression: str) -> list[Import]: """ try: importer_expression, imported_expression = import_expression.split(" -> ") - except ValueError: + except ValueError as exc: msg = f"{import_expression} is not a valid import expression." - raise InvalidImportExpression(msg) from None + raise InvalidImportExpression(msg) from exc try: return self._rustgraph.find_matching_direct_imports( importer_expression=importer_expression, imported_expression=imported_expression ) - except rust.InvalidModuleExpression as e: + except rust.InvalidModuleExpression as exc: msg = f"{import_expression} is not a valid import expression." - raise InvalidImportExpression(msg) from e + raise InvalidImportExpression(msg) from exc # Indirect imports # ---------------- From f36e56fbe1b123d4d3acaa4ffbd8b776c81168f0 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Sat, 20 Dec 2025 09:15:00 +1300 Subject: [PATCH 7/9] Disable `PYI025` rule and revert aliasing i.e. `AbstractSet` -> `Set` --- ruff.toml | 1 + src/grimp/adaptors/modulefinder.py | 7 +++---- src/grimp/application/ports/modulefinder.py | 6 +++--- tests/unit/application/test_scanning.py | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ruff.toml b/ruff.toml index 15e3f46e..7b690419 100644 --- a/ruff.toml +++ b/ruff.toml @@ -22,6 +22,7 @@ lint.ignore = [ "PLW1641", # [eq-without-hash] TODO enable. This is low-priority, and basically an enhancement to make classes more ergonomic. "PTH", # [flake8-use-pathlib] Controversial. `pathlib` adoption is subtle and might be too disruptive for this codebase. e.g. https://github.com/python-grimp/grimp/pull/289#discussion_r2636012008 "PT007", # [pytest-parametrize-values-wrong-type] TODO enable. Inconsistent use of `@pytest.mark.parametrize("value", ("a", "b", "c"))` versus `...["a", "b", "c"])` in parameterizations. + "PYI025", # [unaliased-collections-abc-set-import] Controversial. `collections.abc.Set` is not necessarily confused with the built-in `set`. "RET504", # [unnecessary-assign] Controversial. A micro-optimization but hurts readability. "RET505", # [superfluous-else-return] Controversial. Explicit `else` blocks can improve readability. "S101", # [assert] Controversial. `assert` statements are sometimes appropriate for type-checking purposes. diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index b375cdfd..2a94438f 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -7,8 +7,7 @@ from grimp.domain.valueobjects import Module if TYPE_CHECKING: - from collections.abc import Iterable - from collections.abc import Set as AbstractSet + from collections.abc import Iterable, Set from grimp.application.ports.filesystem import AbstractFileSystem @@ -51,7 +50,7 @@ def find_package( def _get_python_files_and_namespace_dirs_inside_package( self, directory: str - ) -> tuple[Iterable[str], AbstractSet[str]]: + ) -> tuple[Iterable[str], Set[str]]: """ Search the supplied package directory for Python files and namespaces. @@ -95,7 +94,7 @@ def _get_python_files_and_namespace_dirs_inside_package( namespace_dirs = self._determine_namespace_dirs(candidate_namespace_dirs, python_files) return python_files, namespace_dirs - def _is_in_portion(self, directory: str, portions: AbstractSet[str]) -> bool: + def _is_in_portion(self, directory: str, portions: Set[str]) -> bool: return any(directory.startswith(portion) for portion in portions) def _should_ignore_dir(self, directory: str) -> bool: diff --git a/src/grimp/application/ports/modulefinder.py b/src/grimp/application/ports/modulefinder.py index dcc88450..8303146e 100644 --- a/src/grimp/application/ports/modulefinder.py +++ b/src/grimp/application/ports/modulefinder.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from collections.abc import Set as AbstractSet + from collections.abc import Set from grimp.domain.valueobjects import Module @@ -26,8 +26,8 @@ class FoundPackage: name: str directory: str - module_files: AbstractSet[ModuleFile] - namespace_packages: AbstractSet[str] = frozenset() + module_files: Set[ModuleFile] + namespace_packages: Set[str] = frozenset() class AbstractModuleFinder(abc.ABC): diff --git a/tests/unit/application/test_scanning.py b/tests/unit/application/test_scanning.py index 4842eedd..f72ca337 100644 --- a/tests/unit/application/test_scanning.py +++ b/tests/unit/application/test_scanning.py @@ -11,7 +11,7 @@ from tests.config import override_settings if TYPE_CHECKING: - from collections.abc import Set as AbstractSet + from collections.abc import Set @pytest.mark.parametrize( @@ -979,5 +979,5 @@ def _module_to_module_file(module: Module) -> ModuleFile: return ModuleFile(module=module, mtime=some_mtime) -def _modules_to_module_files(modules: AbstractSet[Module]) -> frozenset[ModuleFile]: +def _modules_to_module_files(modules: Set[Module]) -> frozenset[ModuleFile]: return frozenset({_module_to_module_file(module) for module in modules}) From 842ab5e2943eda2d9ede90d0a39312118ca25f54 Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Sat, 20 Dec 2025 09:28:58 +1300 Subject: [PATCH 8/9] Revert TC001, TC002, TC003 rule compliance (TYPE_CHECKING guards) --- ruff.toml | 3 +++ src/grimp/adaptors/caching.py | 10 ++++------ src/grimp/adaptors/filesystem.py | 9 ++------- src/grimp/adaptors/modulefinder.py | 8 ++------ src/grimp/adaptors/packagefinder.py | 8 ++------ src/grimp/application/graph.py | 6 ++---- src/grimp/application/ports/caching.py | 9 +++------ src/grimp/application/ports/filesystem.py | 6 ++---- src/grimp/application/ports/modulefinder.py | 9 +++------ src/grimp/application/ports/packagefinder.py | 4 +--- src/grimp/application/ports/timing.py | 5 +---- src/grimp/application/scanning.py | 12 ++++-------- src/grimp/application/usecases.py | 18 +++++++----------- src/grimp/domain/analysis.py | 5 +---- tests/adaptors/filesystem.py | 10 +++------- tests/adaptors/modulefinder.py | 9 ++------- tests/adaptors/packagefinder.py | 8 ++------ tests/adaptors/timing.py | 5 +---- tests/unit/adaptors/test_filesystem.py | 6 ++---- tests/unit/application/graph/test_layers.py | 5 +---- tests/unit/application/test_scanning.py | 5 +---- tests/unit/application/test_usecases.py | 7 ++----- 22 files changed, 51 insertions(+), 116 deletions(-) diff --git a/ruff.toml b/ruff.toml index 7b690419..be40bf40 100644 --- a/ruff.toml +++ b/ruff.toml @@ -28,6 +28,9 @@ lint.ignore = [ "S101", # [assert] Controversial. `assert` statements are sometimes appropriate for type-checking purposes. "SIM108", # [if-else-block-instead-of-if-exp] Controversial. Ternary/Binary in-line expressions often hurt readability. "SIM118", # [in-dict-keys] Controversial. `if key in dict.keys()` sometimes has clearer intent than `if key in dict`. + "TC001", # [typing-only-first-party-import] Controversial. This package has no/few dependencies so TYPE_CHECKING guards offer no material benefit and hurt readability. + "TC002", # [typing-only-third-party-import] Controversial. This package has no/few dependencies so TYPE_CHECKING guards offer no material benefit and hurt readability. + "TC003", # [typing-only-standard-library-import] Controversial. This package has no/few dependencies so TYPE_CHECKING guards offer no material benefit and hurt readability. "TD", # [missing-space-after-todo-colon] Controversial. Different projects use different conventions for FIXME/TODO comments. "TID252", # [relative-imports] Controversial. There are pros and cons to both relative and absolute imports in a packaging context. ] diff --git a/src/grimp/adaptors/caching.py b/src/grimp/adaptors/caching.py index fc3181e2..d717063d 100644 --- a/src/grimp/adaptors/caching.py +++ b/src/grimp/adaptors/caching.py @@ -3,18 +3,16 @@ import hashlib import json import logging -from typing import TYPE_CHECKING, Any +from typing import Any from grimp import _rustgrimp as rust # type: ignore[attr-defined] +from grimp.application.ports.filesystem import BasicFileSystem +from grimp.application.ports.modulefinder import FoundPackage, ModuleFile +from grimp.domain.valueobjects import DirectImport, Module from ..application.ports.caching import Cache as AbstractCache from ..application.ports.caching import CacheMiss -if TYPE_CHECKING: - from grimp.application.ports.filesystem import BasicFileSystem - from grimp.application.ports.modulefinder import FoundPackage, ModuleFile - from grimp.domain.valueobjects import DirectImport, Module - logger = logging.getLogger(__name__) PrimitiveFormat = dict[str, list[tuple[str, int | None, str]]] diff --git a/src/grimp/adaptors/filesystem.py b/src/grimp/adaptors/filesystem.py index ae76620b..0a396cee 100644 --- a/src/grimp/adaptors/filesystem.py +++ b/src/grimp/adaptors/filesystem.py @@ -2,15 +2,10 @@ import os import tokenize -from typing import TYPE_CHECKING +from collections.abc import Iterator from grimp import _rustgrimp as rust # type: ignore[attr-defined] -from grimp.application.ports.filesystem import AbstractFileSystem - -if TYPE_CHECKING: - from collections.abc import Iterator - - from grimp.application.ports.filesystem import BasicFileSystem +from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem class FileSystem(AbstractFileSystem): diff --git a/src/grimp/adaptors/modulefinder.py b/src/grimp/adaptors/modulefinder.py index 2a94438f..00f77374 100644 --- a/src/grimp/adaptors/modulefinder.py +++ b/src/grimp/adaptors/modulefinder.py @@ -1,16 +1,12 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING +from collections.abc import Iterable, Set from grimp.application.ports import modulefinder +from grimp.application.ports.filesystem import AbstractFileSystem from grimp.domain.valueobjects import Module -if TYPE_CHECKING: - from collections.abc import Iterable, Set - - from grimp.application.ports.filesystem import AbstractFileSystem - logger = logging.getLogger(__name__) diff --git a/src/grimp/adaptors/packagefinder.py b/src/grimp/adaptors/packagefinder.py index 539f81ee..7da528dd 100644 --- a/src/grimp/adaptors/packagefinder.py +++ b/src/grimp/adaptors/packagefinder.py @@ -3,17 +3,13 @@ import importlib.util import logging import sys -from typing import TYPE_CHECKING +from importlib.machinery import ModuleSpec from grimp import exceptions +from grimp.application.ports.filesystem import AbstractFileSystem from grimp.application.ports.packagefinder import AbstractPackageFinder from grimp.domain.valueobjects import Module -if TYPE_CHECKING: - from importlib.machinery import ModuleSpec - - from grimp.application.ports.filesystem import AbstractFileSystem - logger = logging.getLogger(__name__) diff --git a/src/grimp/application/graph.py b/src/grimp/application/graph.py index 841bad1a..21533f18 100644 --- a/src/grimp/application/graph.py +++ b/src/grimp/application/graph.py @@ -1,6 +1,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING, TypedDict +from collections.abc import Sequence +from typing import TypedDict from grimp import _rustgrimp as rust # type: ignore[attr-defined] from grimp.domain.analysis import PackageDependency, Route @@ -12,9 +13,6 @@ NoSuchContainer, ) -if TYPE_CHECKING: - from collections.abc import Sequence - class Import(TypedDict): importer: str diff --git a/src/grimp/application/ports/caching.py b/src/grimp/application/ports/caching.py index 38e62093..46f10997 100644 --- a/src/grimp/application/ports/caching.py +++ b/src/grimp/application/ports/caching.py @@ -1,12 +1,9 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from grimp.application.ports.modulefinder import FoundPackage, ModuleFile +from grimp.domain.valueobjects import DirectImport, Module -if TYPE_CHECKING: - from grimp.application.ports.modulefinder import FoundPackage, ModuleFile - from grimp.domain.valueobjects import DirectImport, Module - - from .filesystem import BasicFileSystem +from .filesystem import BasicFileSystem class CacheMiss(Exception): diff --git a/src/grimp/application/ports/filesystem.py b/src/grimp/application/ports/filesystem.py index 18b04cbc..92e5504c 100644 --- a/src/grimp/application/ports/filesystem.py +++ b/src/grimp/application/ports/filesystem.py @@ -1,10 +1,8 @@ from __future__ import annotations import abc -from typing import TYPE_CHECKING, Protocol - -if TYPE_CHECKING: - from collections.abc import Iterator +from collections.abc import Iterator +from typing import Protocol class AbstractFileSystem(abc.ABC): diff --git a/src/grimp/application/ports/modulefinder.py b/src/grimp/application/ports/modulefinder.py index 8303146e..1d59f936 100644 --- a/src/grimp/application/ports/modulefinder.py +++ b/src/grimp/application/ports/modulefinder.py @@ -1,15 +1,12 @@ from __future__ import annotations import abc +from collections.abc import Set from dataclasses import dataclass -from typing import TYPE_CHECKING -if TYPE_CHECKING: - from collections.abc import Set +from grimp.domain.valueobjects import Module - from grimp.domain.valueobjects import Module - - from .filesystem import AbstractFileSystem +from .filesystem import AbstractFileSystem @dataclass(frozen=True, order=True) diff --git a/src/grimp/application/ports/packagefinder.py b/src/grimp/application/ports/packagefinder.py index 4ccd7e9d..317f5318 100644 --- a/src/grimp/application/ports/packagefinder.py +++ b/src/grimp/application/ports/packagefinder.py @@ -1,10 +1,8 @@ from __future__ import annotations import abc -from typing import TYPE_CHECKING -if TYPE_CHECKING: - from .filesystem import AbstractFileSystem +from .filesystem import AbstractFileSystem class AbstractPackageFinder(abc.ABC): diff --git a/src/grimp/application/ports/timing.py b/src/grimp/application/ports/timing.py index 2d716c0d..2ff4a8f9 100644 --- a/src/grimp/application/ports/timing.py +++ b/src/grimp/application/ports/timing.py @@ -1,10 +1,7 @@ from __future__ import annotations import abc -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from types import TracebackType +from types import TracebackType class Timer(abc.ABC): diff --git a/src/grimp/application/scanning.py b/src/grimp/application/scanning.py index e20de536..47b85506 100644 --- a/src/grimp/application/scanning.py +++ b/src/grimp/application/scanning.py @@ -1,16 +1,12 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from collections.abc import Collection from grimp import _rustgrimp as rust # type: ignore[attr-defined] from grimp.application.config import settings - -if TYPE_CHECKING: - from collections.abc import Collection - - from grimp.application.ports.filesystem import AbstractFileSystem - from grimp.application.ports.modulefinder import FoundPackage, ModuleFile - from grimp.domain.valueobjects import DirectImport, Module +from grimp.application.ports.filesystem import AbstractFileSystem +from grimp.application.ports.modulefinder import FoundPackage, ModuleFile +from grimp.domain.valueobjects import DirectImport, Module def scan_imports( diff --git a/src/grimp/application/usecases.py b/src/grimp/application/usecases.py index efb73167..1e2fec61 100644 --- a/src/grimp/application/usecases.py +++ b/src/grimp/application/usecases.py @@ -5,22 +5,18 @@ from __future__ import annotations import itertools -from typing import TYPE_CHECKING, cast +from collections.abc import Iterable, Sequence +from typing import cast +from ..application.graph import ImportGraph from ..application.ports import caching -from ..domain.valueobjects import Module +from ..application.ports.filesystem import AbstractFileSystem, BasicFileSystem +from ..application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile +from ..application.ports.packagefinder import AbstractPackageFinder +from ..domain.valueobjects import DirectImport, Module from .config import settings from .scanning import scan_imports -if TYPE_CHECKING: - from collections.abc import Iterable, Sequence - - from ..application.graph import ImportGraph - from ..application.ports.filesystem import AbstractFileSystem, BasicFileSystem - from ..application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile - from ..application.ports.packagefinder import AbstractPackageFinder - from ..domain.valueobjects import DirectImport - class NotSupplied: pass diff --git a/src/grimp/domain/analysis.py b/src/grimp/domain/analysis.py index 8121c3bc..0e03011a 100644 --- a/src/grimp/domain/analysis.py +++ b/src/grimp/domain/analysis.py @@ -1,10 +1,7 @@ from __future__ import annotations +from collections.abc import Iterable, Sequence from dataclasses import dataclass -from typing import TYPE_CHECKING - -if TYPE_CHECKING: - from collections.abc import Iterable, Sequence @dataclass(frozen=True, order=True) diff --git a/tests/adaptors/filesystem.py b/tests/adaptors/filesystem.py index a6cb1810..db471f68 100644 --- a/tests/adaptors/filesystem.py +++ b/tests/adaptors/filesystem.py @@ -1,16 +1,12 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any +from collections.abc import Generator +from typing import Any import yaml from grimp import _rustgrimp as rust # type: ignore[attr-defined] -from grimp.application.ports.filesystem import AbstractFileSystem - -if TYPE_CHECKING: - from collections.abc import Generator - - from grimp.application.ports.filesystem import BasicFileSystem +from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem DEFAULT_MTIME = 10000.0 diff --git a/tests/adaptors/modulefinder.py b/tests/adaptors/modulefinder.py index c3fe68ea..50acdd2c 100644 --- a/tests/adaptors/modulefinder.py +++ b/tests/adaptors/modulefinder.py @@ -1,12 +1,7 @@ from __future__ import annotations -from typing import TYPE_CHECKING - -from grimp.application.ports.modulefinder import AbstractModuleFinder, FoundPackage - -if TYPE_CHECKING: - from grimp.application.ports.filesystem import AbstractFileSystem - from grimp.application.ports.modulefinder import ModuleFile +from grimp.application.ports.filesystem import AbstractFileSystem +from grimp.application.ports.modulefinder import AbstractModuleFinder, FoundPackage, ModuleFile class BaseFakeModuleFinder(AbstractModuleFinder): diff --git a/tests/adaptors/packagefinder.py b/tests/adaptors/packagefinder.py index 787f5541..81d1b70b 100644 --- a/tests/adaptors/packagefinder.py +++ b/tests/adaptors/packagefinder.py @@ -1,14 +1,10 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from collections.abc import Mapping +from grimp.application.ports.filesystem import AbstractFileSystem from grimp.application.ports.packagefinder import AbstractPackageFinder -if TYPE_CHECKING: - from collections.abc import Mapping - - from grimp.application.ports.filesystem import AbstractFileSystem - class BaseFakePackageFinder(AbstractPackageFinder): directory_map: Mapping[str, set[str]] = {} diff --git a/tests/adaptors/timing.py b/tests/adaptors/timing.py index f48f5280..30f42ca0 100644 --- a/tests/adaptors/timing.py +++ b/tests/adaptors/timing.py @@ -1,12 +1,9 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from types import TracebackType from grimp.application.ports.timing import Timer -if TYPE_CHECKING: - from types import TracebackType - class FakeTimer(Timer): ARBITRARY_SECONDS_SINCE_EPOCH = 1_000_000 diff --git a/tests/unit/adaptors/test_filesystem.py b/tests/unit/adaptors/test_filesystem.py index b8ddf2c5..3526e55c 100644 --- a/tests/unit/adaptors/test_filesystem.py +++ b/tests/unit/adaptors/test_filesystem.py @@ -1,16 +1,14 @@ from __future__ import annotations from copy import copy -from typing import TYPE_CHECKING, TypeAlias +from typing import TypeAlias import pytest from grimp import _rustgrimp as rust # type: ignore[attr-defined] +from grimp.application.ports.filesystem import BasicFileSystem from tests.adaptors.filesystem import FakeFileSystem -if TYPE_CHECKING: - from grimp.application.ports.filesystem import BasicFileSystem - class _Base: """ diff --git a/tests/unit/application/graph/test_layers.py b/tests/unit/application/graph/test_layers.py index eb31aace..91da4f75 100644 --- a/tests/unit/application/graph/test_layers.py +++ b/tests/unit/application/graph/test_layers.py @@ -3,7 +3,7 @@ import itertools import logging import re -from typing import TYPE_CHECKING +from collections.abc import Iterable import pytest @@ -12,9 +12,6 @@ from grimp.domain.valueobjects import Layer from grimp.exceptions import NoSuchContainer -if TYPE_CHECKING: - from collections.abc import Iterable - class TestSingleOrNoContainer: @pytest.mark.parametrize("specify_container", (True, False)) diff --git a/tests/unit/application/test_scanning.py b/tests/unit/application/test_scanning.py index f72ca337..28289220 100644 --- a/tests/unit/application/test_scanning.py +++ b/tests/unit/application/test_scanning.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from collections.abc import Set import pytest @@ -10,9 +10,6 @@ from grimp.domain.valueobjects import DirectImport, Module from tests.config import override_settings -if TYPE_CHECKING: - from collections.abc import Set - @pytest.mark.parametrize( "include_external_packages, expected_result", diff --git a/tests/unit/application/test_usecases.py b/tests/unit/application/test_usecases.py index 96a389f1..7c3da83e 100644 --- a/tests/unit/application/test_usecases.py +++ b/tests/unit/application/test_usecases.py @@ -1,21 +1,18 @@ from __future__ import annotations import re -from typing import TYPE_CHECKING from unittest.mock import sentinel import pytest from grimp.application import usecases from grimp.application.ports.caching import Cache +from grimp.application.ports.modulefinder import ModuleFile +from grimp.domain.valueobjects import DirectImport, Module from tests.adaptors.filesystem import FakeFileSystem from tests.adaptors.packagefinder import BaseFakePackageFinder from tests.config import override_settings -if TYPE_CHECKING: - from grimp.application.ports.modulefinder import ModuleFile - from grimp.domain.valueobjects import DirectImport, Module - class TestBuildGraph: @pytest.mark.parametrize("include_external_packages", (True, False)) From a1811064fe9c686b379c39795b85763fa632b7ba Mon Sep 17 00:00:00 2001 From: Nathan McDougall Date: Sat, 20 Dec 2025 09:41:32 +1300 Subject: [PATCH 9/9] Add `match` arg in `pytest.warns` for `DeprecationWarning` of `NamespacePackageEncountered` --- tests/unit/test_exceptions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py index 4a2c7186..2ffd8fa5 100644 --- a/tests/unit/test_exceptions.py +++ b/tests/unit/test_exceptions.py @@ -60,5 +60,5 @@ def test_different_texts_are_not_equal(self): class TestNamespacePackageEncountered: def test_deprecated_access(self): - with pytest.warns(DeprecationWarning): + with pytest.warns(DeprecationWarning, match="NamespacePackageEncountered is deprecated"): exceptions.NamespacePackageEncountered