From 6ddc81013be16242fef0d4c8e17fa3e99ae6aad8 Mon Sep 17 00:00:00 2001 From: Dima Gerasimov Date: Thu, 29 Aug 2024 23:35:28 +0100 Subject: [PATCH] ruff: update config --- ruff.toml | 122 ++++++++++++++++++++++++++++++ src/orger/__init__.py | 10 ++- src/orger/atomic_append.py | 16 ++-- src/orger/common.py | 12 +-- src/orger/inorganic.py | 80 +++++++++++--------- src/orger/logging_helper.py | 10 ++- src/orger/modules/krill.py | 2 +- src/orger/modules/pocket_demo.py | 2 +- src/orger/modules/roamresearch.py | 4 +- src/orger/modules/zotero.py | 2 +- src/orger/org_view.py | 43 +++++------ src/orger/pandoc.py | 8 +- src/orger/state.py | 20 ++--- 13 files changed, 232 insertions(+), 99 deletions(-) diff --git a/ruff.toml b/ruff.toml index 54f621c..5b03323 100644 --- a/ruff.toml +++ b/ruff.toml @@ -1,4 +1,50 @@ +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 + "W", # various warnings + + "B", # 'bugbear' set -- various possible bugs + "C4", # flake8-comprehensions -- unnecessary list/map/dict calls + "COM", # trailing commas + "EXE", # various checks wrt executable files + "I", # sort imports + "ICN", # various import conventions + "FBT", # detect use of boolean arguments + "FURB", # various rules + "PERF", # various potential performance speedups + "PD", # pandas rules + "PIE", # 'misc' lints + "PLC", # pylint convention rules + "PLR", # pylint refactor rules + "PLW", # pylint warnings + "PT", # pytest stuff + "PYI", # various type hinting rules + "RET", # early returns + "RUF", # various ruff-specific rules + "TID", # various imports suggestions + "TRY", # various exception handling rules + "UP", # detect deprecated python stdlib stuff + # "FA", # TODO enable later after we make sure cachew works? + "PTH", # pathlib migration + "ARG", # unused argument checks + "A", # builtin shadowing + # "EM", # TODO hmm could be helpful to prevent duplicate err msg in traceback.. but kinda annoying + + # "ALL", # uncomment this to check for new rules! +] + lint.ignore = [ + "D", # annoying nags about docstrings + "N", # pep naming + "TCH", # type checking rules, mostly just suggests moving imports under TYPE_CHECKING + "S", # bandit (security checks) -- tends to be not very useful, lots of nitpicks + "DTZ", # datetimes checks -- complaining about missing tz and mostly false positives + "FIX", # complains about fixmes/todos -- annoying + "TD", # complains about todo formatting -- too annoying + "ANN", # missing type annotations? seems way to strict though + ### too opinionated style checks "E501", # too long lines "E702", # Multiple statements on one line (semicolon) @@ -22,4 +68,80 @@ lint.ignore = [ "F841", # Local variable `count` is assigned to but never used "F401", # imported but unused ### + +### TODO should be fine to use these with from __future__ import annotations? +### there was some issue with cachew though... double check this? + "UP006", # use type instead of Type + "UP007", # use X | Y instead of Union +### + "RUF100", # unused noqa -- handle later + "RUF012", # mutable class attrs should be annotated with ClassVar... ugh pretty annoying for user configs + +### these are just nitpicky, we usually know better + "PLR0911", # too many return statements + "PLR0912", # too many branches + "PLR0913", # too many function arguments + "PLR0915", # too many statements + "PLR1714", # consider merging multiple comparisons + "PLR2044", # line with empty comment + "PLR5501", # use elif instead of else if + "PLR2004", # magic value in comparison -- super annoying in tests +### + "PLR0402", # import X.Y as Y -- TODO maybe consider enabling it, but double check + + "B009", # calling gettattr with constant attribute -- this is useful to convince mypy + "B010", # same as above, but setattr + "B011", # complains about assert False + "B017", # pytest.raises(Exception) + "B023", # seems to result in false positives? + "B028", # suggest using explicit stacklevel? TODO double check later, but not sure it's useful + + # 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", + + # 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 + + "PT004", # deprecated rule, will be removed later + "PT011", # pytest raises should is too broad + "PT012", # pytest raises should contain a single statement + + "COM812", # trailing comma missing -- mostly just being annoying with long multiline strings + + "PD901", # generic variable name df + + "TRY003", # suggests defining exception messages in exception class -- kinda annoying + "TRY004", # prefer TypeError -- don't see the point + "TRY201", # raise without specifying exception name -- sometimes hurts readability + "TRY400", # TODO double check this, might be useful + "TRY401", # redundant exception in logging.exception call? TODO double check, might result in excessive logging + + "PGH", # TODO force error code in mypy instead + + "TID252", # Prefer absolute imports over relative imports from parent modules + + ## too annoying + "T20", # just complains about prints and pprints + "Q", # flake quotes, too annoying + "C90", # some complexity checking + "G004", # logging statement uses f string + "ERA001", # commented out code + "SLF001", # private member accessed + "BLE001", # do not catch 'blind' Exception + "INP001", # complains about implicit namespace packages + "SIM", # some if statements crap + "RSE102", # complains about missing parens in exceptions + ## ] diff --git a/src/orger/__init__.py b/src/orger/__init__.py index d9608ab..7912219 100644 --- a/src/orger/__init__.py +++ b/src/orger/__init__.py @@ -1,5 +1,7 @@ -from .org_view import StaticView, OrgWithKey -from .org_view import Mirror, Queue +from typing import TYPE_CHECKING -# TODO deprecate properly? -InteractiveView = Queue +from .org_view import Mirror, OrgWithKey, Queue, StaticView + +if not TYPE_CHECKING: + # TODO deprecate properly? + InteractiveView = Queue diff --git a/src/orger/atomic_append.py b/src/orger/atomic_append.py index fec82ec..5f774f8 100644 --- a/src/orger/atomic_append.py +++ b/src/orger/atomic_append.py @@ -1,6 +1,6 @@ -from pathlib import Path -from os.path import lexists import logging +from os.path import lexists +from pathlib import Path from typing import Union PathIsh = Union[str, Path] @@ -26,7 +26,7 @@ def assert_not_edited(path: Path) -> None: for x in [vim, emacs]: lf = path.parent / x if lexists(lf): # lexist is necessary because emacs uses symlink for lock file - raise RuntimeError('File is being edited: {}'.format(lf)) + raise RuntimeError(f'File is being edited: {lf}') def atomic_append_check( @@ -45,19 +45,19 @@ def atomic_append_check( def test_atomic_append_check(tmp_path: Path) -> None: - import pytest import platform + import pytest + if platform.system() == 'Windows': pytest.skip("this test doesn't work on windows for now") of = tmp_path / 'test.org' of.touch() - from subprocess import Popen, PIPE, check_call - from time import sleep - from contextlib import contextmanager + from subprocess import PIPE, Popen, check_call + from time import sleep @contextmanager def tmp_popen(*args, **kwargs): with Popen(*args, **kwargs) as p: @@ -71,7 +71,7 @@ def tmp_popen(*args, **kwargs): assert of.read_text() == 'data1data2' with tmp_popen(['vi', '-c', 'startinsert', str(of)], stdin=PIPE, stdout=PIPE, stderr=PIPE) as p: # enter insert mode - for attempt in range(10): + for _attempt in range(10): # ugh, needs long pause for some reason sleep(1) swapfiles = list(tmp_path.glob('.*.swp')) diff --git a/src/orger/common.py b/src/orger/common.py index fcf53ed..c2ef4be 100644 --- a/src/orger/common.py +++ b/src/orger/common.py @@ -1,8 +1,10 @@ +import traceback +import warnings from datetime import datetime -from typing import Optional from pathlib import Path +from typing import TYPE_CHECKING, Optional -from .inorganic import OrgNode, timestamp, timestamp_with_style, TimestampStyle +from .inorganic import OrgNode, TimestampStyle, timestamp, timestamp_with_style # todo add error policy here? @@ -23,7 +25,6 @@ def dt_heading(dt: Optional[datetime], heading: str) -> str: tz = dt.tzinfo # todo come up with a better way of reporting this.. if tz not in _timezones and len(_timezones) > 0: - import warnings warnings.warn(f"Seems that a mixture of timezones is used. Org-mode doesn't support timezones, so this might end up confusing: {_timezones} {tz} {heading}") _timezones.add(tz) @@ -31,7 +32,6 @@ def dt_heading(dt: Optional[datetime], heading: str) -> str: def error(e: Exception) -> OrgNode: - import traceback return OrgNode( heading="ERROR!", body='\n'.join(traceback.format_exception(Exception, e, None)), @@ -58,4 +58,6 @@ def orger_user_dir() -> Path: return Path(appdirs.user_config_dir('orger')) -from .logging_helper import LazyLogger, setup_logger # legacy imports for bwd compatibility +if not TYPE_CHECKING: + # legacy imports for bwd compatibility + from .logging_helper import LazyLogger, setup_logger diff --git a/src/orger/inorganic.py b/src/orger/inorganic.py index d530b54..954776c 100644 --- a/src/orger/inorganic.py +++ b/src/orger/inorganic.py @@ -1,15 +1,29 @@ -from datetime import datetime, date import logging -from pathlib import Path -import re import os -from collections import OrderedDict -from typing import NamedTuple, Optional, Sequence, Dict, Mapping, Any, Tuple, TypeVar, Callable, Union, List, TYPE_CHECKING +import re +import textwrap import warnings +from collections import OrderedDict +from dataclasses import dataclass +from datetime import date, datetime +from enum import Enum +from pathlib import Path +from typing import ( + TYPE_CHECKING, + Any, + Callable, + Dict, + List, + Mapping, + Optional, + Sequence, + Tuple, + TypeVar, + Union, +) # todo use mypy literals later? -from enum import Enum class TimestampStyle(Enum): INACTIVE = ('[', ']') ACTIVE = ('<', '>') @@ -21,6 +35,7 @@ class TimestampStyle(Enum): PathIsh = Union[str, Path] + def link(*, url: PathIsh, title: Optional[str]) -> str: """ >>> link(url='http://reddit.com', title='[R]eddit!') @@ -51,7 +66,7 @@ def docview_link(*, path: PathIsh, title: Optional[str], page1: Optional[int]=No return link(url=url, title=title) -def timestamp(dt: Dateish, inactive: bool=False, active: bool=False) -> str: +def timestamp(dt: Dateish, *, inactive: bool=False, active: bool=False) -> str: """ default is active >>> dt = datetime.strptime('19920110 04:45', '%Y%m%d %H:%M') @@ -89,16 +104,16 @@ def timestamp_with_style(dt: Dateish, style: TimestampStyle) -> str: return beg + r + end -import textwrap def literal(text: str) -> str: """ Quick way of 'quoting' the text https://orgmode.org/manual/Literal-Examples.html """ - return textwrap.indent(text, ': ', lambda line: True) + return textwrap.indent(text, ': ', lambda _line: True) -class Quoted(NamedTuple): +@dataclass +class Quoted: ''' Special object to markup 'literal' paragraphs https://orgmode.org/manual/Literal-Examples.html @@ -115,15 +130,16 @@ def quoted(self) -> str: # TODO priority # TODO for sanitizing, have two strategies: error and replace? def asorgoutline( - heading: Optional[str] = None, - todo: Optional[str] = None, - tags: Sequence[str] = [], - scheduled: Optional[Dateish] = None, - # TODO perhaps allow list of tuples? properties might be repeating - properties: Optional[Mapping[str, str]]=None, - body: Optional[Body] = None, - level: int=1, - escaped: bool=False, + heading: Optional[str] = None, + *, + todo: Optional[str] = None, + tags: Sequence[str] = [], + scheduled: Optional[Dateish] = None, + # TODO perhaps allow list of tuples? properties might be repeating + properties: Optional[Mapping[str, str]] = None, + body: Optional[Body] = None, + level: int = 1, + escaped: bool = False, ) -> str: r""" Renders Org mode outline (apart from children) @@ -215,22 +231,12 @@ def asorgoutline( Lazy = Union[T, Callable[[], T]] -# jeez... -# otherwise it struggles to resolve recursive type -# could as well make it dataclass... just dunno if should introduce dependency for py36 -if TYPE_CHECKING: - from dataclasses import dataclass as mypy_dataclass - Base = object -else: - mypy_dataclass = lambda x: x - Base = NamedTuple - - -@mypy_dataclass -class OrgNode(Base): +@dataclass +class OrgNode: """ Meant to be somewhat compatible with https://orgparse.readthedocs.io/en/latest/#orgparse.node.OrgNode """ + heading: Lazy[str] todo: Optional[str] = None tags: Sequence[str] = () @@ -241,7 +247,7 @@ class OrgNode(Base): children: Sequence['OrgNode'] = () # NOTE: this is a 'private' attribute - escaped: bool=False + escaped: bool = False def _render_self(self) -> str: return asorgoutline( @@ -263,7 +269,7 @@ def _render_hier(self) -> List[Tuple[int, str]]: res.extend((l + 1, x) for l, x in ch._render_hier()) return res - def render(self, level: int=1) -> str: + def render(self, level: int = 1) -> str: r""" >>> OrgNode('something', todo='TODO').render() '* TODO something' @@ -278,11 +284,13 @@ def render(self, level: int=1) -> str: rh = [(level + l, x) for l, x in rh] return '\n'.join('*' * l + (' ' if l > 0 else '') + x for l, x in rh) + node = OrgNode ## helper functions + def asorgdate(t: Dateish) -> str: return t.strftime("%Y-%m-%d %a") @@ -299,7 +307,6 @@ def _from_lazy(x: Lazy[T]) -> T: return x -from typing import Mapping def maketrans(d: Dict[str, str]) -> Dict[int, str]: # make mypy happy... https://github.com/python/mypy/issues/4374 return str.maketrans(d) @@ -343,8 +350,7 @@ def _sanitize_tag(tag: str) -> str: 'test_d@shes' """ # https://orgmode.org/manual/Tags.html - # Tags are normal words containing letters, numbers, ‘_’, and ‘@’. + # Tags are normal words containing letters, numbers, '_', and '@'. # TODO not sure, perhaps we want strict mode for formatting? # TODO reuse orgparse regexes? return re.sub(r'[^@\w]', '_', tag) - diff --git a/src/orger/logging_helper.py b/src/orger/logging_helper.py index 154f977..f796645 100644 --- a/src/orger/logging_helper.py +++ b/src/orger/logging_helper.py @@ -1,11 +1,11 @@ from __future__ import annotations -from functools import lru_cache import logging import os import sys -from typing import Union import warnings +from functools import lru_cache +from typing import Union def test() -> None: @@ -15,7 +15,7 @@ def test() -> None: ## prepare exception for later try: - None.whatever # type: ignore[attr-defined] + None.whatever # type: ignore[attr-defined] # noqa: B018 except Exception as e: ex = e ## @@ -209,7 +209,9 @@ def make_logger(name: str, *, level: LevelIsh = None) -> logging.Logger: # OK, when stdout is not a tty, enlighten doesn't log anything, good def get_enlighten(): # TODO could add env variable to disable enlighten for a module? - from unittest.mock import Mock # Mock to return stub so cients don't have to think about it + from unittest.mock import ( + Mock, # Mock to return stub so cients don't have to think about it + ) # for now hidden behind the flag since it's a little experimental if os.environ.get('ENLIGHTEN_ENABLE', None) is None: diff --git a/src/orger/modules/krill.py b/src/orger/modules/krill.py index 867f8b1..88d65c6 100755 --- a/src/orger/modules/krill.py +++ b/src/orger/modules/krill.py @@ -30,7 +30,7 @@ def is_drill(i: Highlight) -> bool: def get_drill_items(): - return list(sorted(filter(is_drill, get_highlights()), key=lambda h: h.dt)) + return sorted(filter(is_drill, get_highlights()), key=lambda h: h.dt) class Krill(Queue): diff --git a/src/orger/modules/pocket_demo.py b/src/orger/modules/pocket_demo.py index b0a8054..e1ff73f 100755 --- a/src/orger/modules/pocket_demo.py +++ b/src/orger/modules/pocket_demo.py @@ -128,7 +128,7 @@ def setup_parser(p): * [2016-05-31 Tue 18:24] [[https://app.getpocket.com/read/1188624587][pocket]] · [[https://packetzoom.com/blog/how-to-test-your-app-in-different-network-conditions.html][How to test your app in different network conditions -]] * [2016-05-31 Tue 18:24] [[https://app.getpocket.com/read/1191143185][pocket]] · [[http://www.schibsted.pl/2016/02/hood-okhttps-cache/][What's under the hood of the OkHttp's cache?]] * [2016-03-15 Tue 17:27] [[https://app.getpocket.com/read/1187239791][pocket]] · [[http://joeduffyblog.com/2016/02/07/the-error-model/][Joe Duffy - The Error Model]] -** [2019-09-25 Wed 18:20] A bug is a kind of error the programmer didn’t expect. Inputs weren’t validated correctly, logic was written wrong, or any host of problems have arisen. +** [2019-09-25 Wed 18:20] A bug is a kind of error the programmer didn't expect. Inputs weren't validated correctly, logic was written wrong, or any host of problems have arisen. ** [2019-09-25 Wed 18:19] First, throwing an exception is usually ridiculously expensive. This is almost always due to the gathering of a stack trace. ** [2019-09-25 Wed 18:20] In other words, an exception, as with error codes, is just a different kind of return value! """ diff --git a/src/orger/modules/roamresearch.py b/src/orger/modules/roamresearch.py index fe49364..f0b1c93 100755 --- a/src/orger/modules/roamresearch.py +++ b/src/orger/modules/roamresearch.py @@ -24,7 +24,7 @@ def roam_text_to_org(text: str) -> str: return org -def roam_note_to_org(node: roamresearch.Node, top=False) -> Iterable[OrgNode]: +def roam_note_to_org(node: roamresearch.Node, *, top: bool = False) -> Iterable[OrgNode]: """ Converts Roam node into Org-mode representation """ @@ -84,7 +84,7 @@ def get_items(self): items = list(chain.from_iterable(pool.map(roam_note_to_org, rr.notes))) # move the ones with no children to the bottom - items = list(sorted(items, key=lambda n: len(n.children), reverse=True)) + items = sorted(items, key=lambda n: len(n.children), reverse=True) yield from items diff --git a/src/orger/modules/zotero.py b/src/orger/modules/zotero.py index 3968c1b..0beb374 100755 --- a/src/orger/modules/zotero.py +++ b/src/orger/modules/zotero.py @@ -30,7 +30,7 @@ class config: class Zotero(Mirror): - DEFAULT_HEADER = Mirror.DEFAULT_HEADER + indent(colors_doc, '# ', lambda line: True) + DEFAULT_HEADER = Mirror.DEFAULT_HEADER + indent(colors_doc, '# ', lambda _line: True) def get_items(self) -> Mirror.Results: from my import zotero diff --git a/src/orger/org_view.py b/src/orger/org_view.py index 95159a9..309d6e2 100644 --- a/src/orger/org_view.py +++ b/src/orger/org_view.py @@ -1,20 +1,18 @@ -#!/usr/bin/env python3 import argparse -from argparse import ArgumentParser, Namespace -from collections import Counter import inspect import json +import sys +from argparse import ArgumentParser, Namespace +from collections import Counter from pathlib import Path from subprocess import check_call -import sys -from typing import Any, List, Tuple, Iterable, Optional, Union, Callable, Dict +from typing import Any, Callable, Dict, Iterable, List, Optional, Tuple, Union -from .inorganic import OrgNode, TimestampStyle -from .state import JsonState -from .atomic_append import atomic_append_check, assert_not_edited +from .atomic_append import assert_not_edited, atomic_append_check from .common import orger_user_dir +from .inorganic import OrgNode, TimestampStyle from .logging_helper import make_logger - +from .state import JsonState # TODO tests for determinism? not sure where should they be... # think of some generic thing to test that? @@ -119,7 +117,7 @@ def main(cls, setup_parser=None) -> None: def get_items(self) -> Iterable: raise NotImplementedError - def _run(self, to: Path, stdout: bool) -> None: + def _run(self, to: Path, *, stdout: bool) -> None: org_tree = self.make_tree() rtree = org_tree.render(level=0) @@ -164,11 +162,10 @@ def pick_heading(root: OrgNode, text: str) -> Optional[OrgNode]: if text in _from_lazy(root.heading): return root for ch in root.children: - chr = pick_heading(ch, text) - if chr is not None: - return chr - else: - return None + ch_res = pick_heading(ch, text) + if ch_res is not None: + return ch_res + return None def test(): tree = cls().make_tree() # TODO make sure it works on both static and interactive? @@ -197,12 +194,13 @@ class Queue(OrgView): Results = Iterable[OrgWithKey] def _run( - self, - to: Path, - stdout: bool, - state_path: Path, - init: bool=False, - dry_run: bool=False, + self, + to: Path, + *, + stdout: bool, + state_path: Path, + init: bool = False, + dry_run: bool = False, ) -> None: self.logger.info('Using state file %s', state_path) @@ -316,8 +314,7 @@ def __init__(self, items: List[OrgWithKey], *args, **kwargs) -> None: self.items = items def get_items(self): - for i in self.items: - yield i + yield from self.items rpath = tmp_path / 'res.org' spath = tmp_path / 'state.json' diff --git a/src/orger/pandoc.py b/src/orger/pandoc.py index 9996dc2..aa6cc02 100644 --- a/src/orger/pandoc.py +++ b/src/orger/pandoc.py @@ -1,15 +1,15 @@ """ Helper for converting stuff to pandoc """ -from functools import lru_cache import logging import shutil -from subprocess import run, PIPE +from functools import lru_cache +from subprocess import PIPE, run from typing import Optional - from .common import settings + @lru_cache(1) def should_use_pandoc() -> bool: if not settings.USE_PANDOC: @@ -32,7 +32,7 @@ def to_org(data: str, *, from_: str, logger=logging) -> str: # meh. for some reason they are converted to \\ otherwise if from_ == 'html': data = data.replace('
', '') - + try: r = run( ['pandoc', '-f', from_, '-t', 'org', '--wrap=none'], diff --git a/src/orger/state.py b/src/orger/state.py index 939c90f..7e17900 100644 --- a/src/orger/state.py +++ b/src/orger/state.py @@ -1,25 +1,27 @@ import json -from pathlib import Path -from typing import List, Union, Dict, Any, Optional, Callable import logging import os import sys import warnings +from pathlib import Path +from typing import Any, Callable, Dict, List, Optional, Union + +from atomicwrites import atomic_write # type: ignore[import-untyped] PathIsh = Union[str, Path] State = Dict[str, Any] -from atomicwrites import atomic_write # type: ignore[import-untyped] # TODO hmm. state should be ordered ideally? so it's easy to add/remove items? # would require storing as list of lists? or use that https://stackoverflow.com/a/6921760/706389 class JsonState: def __init__( - self, - path: PathIsh, - dry_run: bool=False, - default: Optional[State]=None, - logger: logging.Logger=logging.getLogger('orger'), + self, + path: PathIsh, + *, + dry_run: bool = False, + default: Optional[State] = None, + logger: logging.Logger=logging.getLogger('orger'), # noqa: B008 ) -> None: self.path = Path(path) self.dry_run = dry_run @@ -59,7 +61,7 @@ def get(self) -> State: def feed(self, key: str, value: Any, action: Callable[[], None]) -> None: # just to be safe so we don't dump int by accident - assert isinstance(key, str), f"key/id has to be a str! key: {repr(key)}, value: {repr(value)}" + assert isinstance(key, str), f"key/id has to be a str! key: {key!r}, value: {value!r}" if key in self: self.logger.debug('already handled: %s: %s', key, value)