From 9fd4227abf7fa2619c9f1297e754eead0aee7fc3 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Wed, 28 Aug 2024 02:50:05 +0100 Subject: [PATCH] ruff: enable RET/PIE/PLW --- my/coding/commits.py | 3 +- my/core/__main__.py | 10 ++--- my/core/_deprecated/kompress.py | 2 +- my/core/error.py | 3 +- my/core/orgmode.py | 3 +- my/core/query_range.py | 60 +++++++++++++------------- my/core/serialize.py | 3 +- my/core/util.py | 2 +- my/experimental/destructive_parsing.py | 2 +- my/location/fallback/via_home.py | 19 ++++---- my/photos/main.py | 3 +- my/smscalls.py | 7 ++- my/time/tz/via_location.py | 6 +-- ruff.toml | 32 ++++++++++---- 14 files changed, 80 insertions(+), 75 deletions(-) diff --git a/my/coding/commits.py b/my/coding/commits.py index 20b66a0..d4e05b7 100644 --- a/my/coding/commits.py +++ b/my/coding/commits.py @@ -187,8 +187,7 @@ def _repo_depends_on(_repo: Path) -> int: ff = _repo / pp if ff.exists(): return int(ff.stat().st_mtime) - else: - raise RuntimeError(f"Could not find a FETCH_HEAD/HEAD file in {_repo}") + raise RuntimeError(f"Could not find a FETCH_HEAD/HEAD file in {_repo}") def _commits(_repos: List[Path]) -> Iterator[Commit]: diff --git a/my/core/__main__.py b/my/core/__main__.py index c5e4552..8553942 100644 --- a/my/core/__main__.py +++ b/my/core/__main__.py @@ -43,7 +43,7 @@ def run_mypy(cfg_path: Path) -> Optional[CompletedProcess]: cmd = mypy_cmd() if cmd is None: return None - mres = run([ # noqa: UP022 + mres = run([ # noqa: UP022,PLW1510 *cmd, '--namespace-packages', '--color-output', # not sure if works?? @@ -214,10 +214,10 @@ See https://github.com/karlicoss/HPI/blob/master/doc/SETUP.org#setting-up-module if len(errors) > 0: error(f'config check: {len(errors)} errors') return False - else: - # note: shouldn't exit here, might run something else - info('config check: success!') - return True + + # note: shouldn't exit here, might run something else + info('config check: success!') + return True from .util import HPIModule, modules diff --git a/my/core/_deprecated/kompress.py b/my/core/_deprecated/kompress.py index 803e515..cd09c06 100644 --- a/my/core/_deprecated/kompress.py +++ b/my/core/_deprecated/kompress.py @@ -87,7 +87,7 @@ def kopen(path: PathIsh, *args, mode: str='rt', **kwargs) -> IO: elif name.endswith(Ext.lz4): import lz4.frame # type: ignore return lz4.frame.open(str(pp), mode, *args, **kwargs) - elif name.endswith(Ext.zstd) or name.endswith(Ext.zst): + elif name.endswith(Ext.zstd) or name.endswith(Ext.zst): # noqa: PIE810 kwargs['mode'] = mode return _zstd_open(pp, *args, **kwargs) elif name.endswith(Ext.targz): diff --git a/my/core/error.py b/my/core/error.py index e869614..ed26dda 100644 --- a/my/core/error.py +++ b/my/core/error.py @@ -41,8 +41,7 @@ def notnone(x: Optional[T]) -> T: def unwrap(res: Res[T]) -> T: if isinstance(res, Exception): raise res - else: - return res + return res def drop_exceptions(itr: Iterator[Res[T]]) -> Iterator[T]: diff --git a/my/core/orgmode.py b/my/core/orgmode.py index c70ded6..979f288 100644 --- a/my/core/orgmode.py +++ b/my/core/orgmode.py @@ -17,8 +17,7 @@ def parse_org_datetime(s: str) -> datetime: return datetime.strptime(s, fmt) except ValueError: continue - else: - raise RuntimeError(f"Bad datetime string {s}") + raise RuntimeError(f"Bad datetime string {s}") # TODO I guess want to borrow inspiration from bs4? element type <-> tag; and similar logic for find_one, find_all diff --git a/my/core/query_range.py b/my/core/query_range.py index 0a1b321..1f4a7ff 100644 --- a/my/core/query_range.py +++ b/my/core/query_range.py @@ -341,37 +341,37 @@ def select_range( if order_by_chosen is None: raise QueryException("""Can't order by range if we have no way to order_by! Specify a type or a key to order the value by""") - else: - # force drop_unsorted=True so we can use _create_range_filter - # sort the iterable by the generated order_by_chosen function - itr = select(itr, order_by=order_by_chosen, drop_unsorted=True) - filter_func: Optional[Where] - if order_by_value_type in [datetime, date]: - filter_func = _create_range_filter( - unparsed_range=unparsed_range, - end_parser=parse_datetime_float, - within_parser=parse_timedelta_float, - attr_func=order_by_chosen, # type: ignore[arg-type] - default_before=time.time(), - value_coercion_func=_datelike_to_float) - elif order_by_value_type in [int, float]: - # allow primitives to be converted using the default int(), float() callables - filter_func = _create_range_filter( - unparsed_range=unparsed_range, - end_parser=order_by_value_type, - within_parser=order_by_value_type, - attr_func=order_by_chosen, # type: ignore[arg-type] - default_before=None, - value_coercion_func=order_by_value_type) - else: - # TODO: add additional kwargs to let the user sort by other values, by specifying the parsers? - # would need to allow passing the end_parser, within parser, default before and value_coercion_func... - # (seems like a lot?) - raise QueryException("Sorting by custom types is currently unsupported") - # use the created filter function - # we've already applied drop_exceptions and kwargs related to unsortable values above - itr = select(itr, where=filter_func, limit=limit, reverse=reverse) + # force drop_unsorted=True so we can use _create_range_filter + # sort the iterable by the generated order_by_chosen function + itr = select(itr, order_by=order_by_chosen, drop_unsorted=True) + filter_func: Optional[Where] + if order_by_value_type in [datetime, date]: + filter_func = _create_range_filter( + unparsed_range=unparsed_range, + end_parser=parse_datetime_float, + within_parser=parse_timedelta_float, + attr_func=order_by_chosen, # type: ignore[arg-type] + default_before=time.time(), + value_coercion_func=_datelike_to_float) + elif order_by_value_type in [int, float]: + # allow primitives to be converted using the default int(), float() callables + filter_func = _create_range_filter( + unparsed_range=unparsed_range, + end_parser=order_by_value_type, + within_parser=order_by_value_type, + attr_func=order_by_chosen, # type: ignore[arg-type] + default_before=None, + value_coercion_func=order_by_value_type) + else: + # TODO: add additional kwargs to let the user sort by other values, by specifying the parsers? + # would need to allow passing the end_parser, within parser, default before and value_coercion_func... + # (seems like a lot?) + raise QueryException("Sorting by custom types is currently unsupported") + + # use the created filter function + # we've already applied drop_exceptions and kwargs related to unsortable values above + itr = select(itr, where=filter_func, limit=limit, reverse=reverse) else: # wrap_unsorted may be used here if the user specified an order_key, # or manually passed a order_value function diff --git a/my/core/serialize.py b/my/core/serialize.py index b196d47..ab11a20 100644 --- a/my/core/serialize.py +++ b/my/core/serialize.py @@ -145,8 +145,7 @@ def _dumps_factory(**kwargs) -> Callable[[Any], str]: res = factory() if res is not None: return res - else: - raise RuntimeError("Should not happen!") + raise RuntimeError("Should not happen!") def dumps( diff --git a/my/core/util.py b/my/core/util.py index fb3edf8..a247f81 100644 --- a/my/core/util.py +++ b/my/core/util.py @@ -100,7 +100,7 @@ def _walk_packages(path: Iterable[str], prefix: str='', onerror=None) -> Iterabl def seen(p, m={}): # noqa: B006 if p in m: return True - m[p] = True + m[p] = True # noqa: RET503 for info in pkgutil.iter_modules(path, prefix): mname = info.name diff --git a/my/experimental/destructive_parsing.py b/my/experimental/destructive_parsing.py index 056cc0b..b389f7e 100644 --- a/my/experimental/destructive_parsing.py +++ b/my/experimental/destructive_parsing.py @@ -35,7 +35,7 @@ def is_empty(x) -> bool: elif isinstance(x, list): return all(map(is_empty, x)) else: - assert_never(x) + assert_never(x) # noqa: RET503 class Manager: diff --git a/my/location/fallback/via_home.py b/my/location/fallback/via_home.py index 199ebb0..e44c59d 100644 --- a/my/location/fallback/via_home.py +++ b/my/location/fallback/via_home.py @@ -92,13 +92,12 @@ def estimate_location(dt: DateExact) -> Iterator[FallbackLocation]: dt=datetime.fromtimestamp(d, timezone.utc), datasource='via_home') return - else: - # I guess the most reasonable is to fallback on the first location - lat, lon = hist[-1][1] - yield FallbackLocation( - lat=lat, - lon=lon, - accuracy=config.home_accuracy, - dt=datetime.fromtimestamp(d, timezone.utc), - datasource='via_home') - return + + # I guess the most reasonable is to fallback on the first location + lat, lon = hist[-1][1] + yield FallbackLocation( + lat=lat, + lon=lon, + accuracy=config.home_accuracy, + dt=datetime.fromtimestamp(d, timezone.utc), + datasource='via_home') diff --git a/my/photos/main.py b/my/photos/main.py index 63a6fea..c326405 100644 --- a/my/photos/main.py +++ b/my/photos/main.py @@ -43,8 +43,7 @@ class Photo(NamedTuple): for bp in config.paths: if self.path.startswith(bp): return self.path[len(bp):] - else: - raise RuntimeError(f"Weird path {self.path}, can't match against anything") + raise RuntimeError(f"Weird path {self.path}, can't match against anything") @property def name(self) -> str: diff --git a/my/smscalls.py b/my/smscalls.py index b56026d..78bf7ee 100644 --- a/my/smscalls.py +++ b/my/smscalls.py @@ -182,10 +182,9 @@ class MMS(NamedTuple): for (addr, _type) in self.addresses: if _type == 137: return addr - else: - # hmm, maybe return instead? but this probably shouldnt happen, means - # something is very broken - raise RuntimeError(f'No from address matching 137 found in {self.addresses}') + # hmm, maybe return instead? but this probably shouldnt happen, means + # something is very broken + raise RuntimeError(f'No from address matching 137 found in {self.addresses}') @property def from_me(self) -> bool: diff --git a/my/time/tz/via_location.py b/my/time/tz/via_location.py index 156a5db..8f521e0 100644 --- a/my/time/tz/via_location.py +++ b/my/time/tz/via_location.py @@ -63,16 +63,14 @@ def _get_user_config(): except ImportError as ie: if "'time'" not in str(ie): raise ie - else: - return empty_config + return empty_config try: user_config = time.tz.via_location except AttributeError as ae: if not ("'tz'" in str(ae) or "'via_location'" in str(ae)): raise ae - else: - return empty_config + return empty_config return user_config diff --git a/ruff.toml b/ruff.toml index 2c9c39b..3cb9f76 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,18 +1,22 @@ target-version = "py38" # NOTE: inferred from pyproject.toml if present lint.extend-select = [ - "F", # flakes rules -- default, but extend just in case - "E", # pycodestyle -- default, but extend just in case - "C4", # flake8-comprehensions -- unnecessary list/map/dict calls - "UP", # detect deprecated python stdlib stuff - "FBT", # detect use of boolean arguments - "RUF", # various ruff-specific rules - "PLR", # 'refactor' rules - "B", # 'bugbear' set -- various possible bugs - + "F", # flakes rules -- default, but extend just in case + "E", # pycodestyle -- default, but extend just in case + "C4", # flake8-comprehensions -- unnecessary list/map/dict calls + "UP", # detect deprecated python stdlib stuff + "FBT", # detect use of boolean arguments + "RUF", # various ruff-specific rules + "PLR", # 'refactor' rules + "B", # 'bugbear' set -- various possible bugs "PERF", # various potential performance speedups + "RET", # early returns + "PIE", # 'misc' lints + "PLW", # pylint warnings # "FA", # TODO enable later after we make sure cachew works? + # "PTH", # pathlib migration -- TODO enable later # "ARG", # TODO useful, but results in some false positives in pytest fixtures... maybe later + # "A", # TODO builtin shadowing -- handle later # "S", # bandit (security checks) -- tends to be not very useful, lots of nitpicks # "DTZ", # datetimes checks -- complaining about missing tz and mostly false positives ] @@ -67,6 +71,10 @@ lint.ignore = [ "B017", # pytest.raises(Exception) "B023", # seems to result in false positives? + # complains about useless pass, but has sort of a false positive if the function has a docstring? + # this is common for click entrypoints (e.g. in __main__), so disable + "PIE790", + # a bit too annoying, offers to convert for loops to list comprehension # , which may heart readability "PERF401", @@ -74,4 +82,10 @@ lint.ignore = [ # suggests no using exception in for loops # we do use this technique a lot, plus in 3.11 happy path exception handling is "zero-cost" "PERF203", + + "RET504", # unnecessary assignment before returning -- that can be useful for readability + "RET505", # unnecessary else after return -- can hurt readability + + "PLW0603", # global variable update.. we usually know why we are doing this + "PLW2901", # for loop variable overwritten, usually this is intentional ]