-
Notifications
You must be signed in to change notification settings - Fork 23
Select and comply with all Ruff rules, minus a curated ignore list #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
5588928
c390810
ed6dbd7
f5a681e
8796027
716b13b
44ee64d
f36e56f
842ab5e
a181106
76c1335
2681042
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,19 @@ | ||
| import hashlib | ||
| from __future__ import annotations | ||
|
nathanjmcdougall marked this conversation as resolved.
|
||
|
|
||
| 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: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the fence on this one - I suppose there is hypothetically a slight runtime penalty, but for me it makes the code a little less readable. What rule is it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The TC ruleset from https://docs.astral.sh/ruff/rules/#flake8-type-checking-tc The runtime penalty is probably near-zero because I've gone ahead and reverted this from the PR. We can always add it back in later if there's a desire for it, it's easy to migrate with autofix. |
||
| 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: | ||
|
nathanjmcdougall marked this conversation as resolved.
Outdated
|
||
| 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 | ||
|
nathanjmcdougall marked this conversation as resolved.
Outdated
|
||
| 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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above |
||
|
|
||
| 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() | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.