ci: update ruff and mypy configs

mypy.ini

@@ -1,14 +1,13 @@
 [mypy]
-namespace_packages = True
 pretty = True
 show_error_context = True
-show_error_codes = True
 show_column_numbers = True
 show_error_end = True
+warn_redundant_casts = True
 warn_unused_ignores = True
 check_untyped_defs = True
-enable_error_code = possibly-undefined
 strict_equality = True
+enable_error_code = possibly-undefined
 
 # not sure why mypy started discovering it (since 0.800??)
 [mypy-hypothesis]

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",   # suggest using from __future__ import annotations
+    "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)
@@ -17,9 +63,84 @@ lint.ignore = [
     "E402",  # Module level import not at top of file
 
 ### maybe consider these soon
-# sometimes it's useful to give a variable a name even if we don't use it as a documentation
-# on the other hand, often is a sign of error
+    # sometimes it's useful to give a variable a name even if we don't use it as a documentation
+    # on the other hand, often is a sign of error
     "F841",  # Local variable `count` is assigned to but never used
-    "F401",  # imported but unused
 ###
+
+    "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
+
+    "UP038",  # suggests using | (union) in isisntance checks.. but it results in slower code
+
+    ## 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
+    ##
+
+    "ARG001",  # ugh, kinda annoying when using pytest fixtures
+    "RUF001", "RUF002", "RUF003",  # spams about non-latin characters that we do use for testing
 ]

setup.py

@@ -37,6 +37,7 @@ def main() -> None:
             'appdirs', # for portable user directories detection
             'tzlocal',
             'more_itertools',
+            'typing-extensions',
             'pytz',
             'sqlalchemy>=2.0',  # DB api
 

src/promnesia/__init__.py

@@ -1,6 +1,7 @@
-from pathlib import Path
-from .common import PathIsh, Visit, Source, last, Loc, Results, DbVisit, Context, Res
+from .common import PathIsh, Visit, Source, last, Loc, Results, DbVisit, Context, Res  # noqa: F401
 
 # add deprecation warning so eventually this may converted to a namespace package?
 import warnings
+
+# TODO think again about it -- what are the pros and cons?
 warnings.warn("DEPRECATED! Please import directly from 'promnesia.common', e.g. 'from promnesia.common import Visit, Source, Results'", DeprecationWarning)

src/promnesia/__main__.py

@@ -6,11 +6,12 @@
 import inspect
 import os
 from pathlib import Path
+import shlex
 import shutil
 from subprocess import run, check_call, Popen
 import sys
 from tempfile import TemporaryDirectory, gettempdir
-from typing import Callable, Sequence, Iterable, Iterator, Union
+from typing import Callable, Sequence, Iterable, Iterator
 
 
 from . import config
@@ -22,7 +23,7 @@
 from .extract import extract_visits
 
 
-def iter_all_visits(sources_subset: Iterable[Union[str, int]]=()) -> Iterator[Res[DbVisit]]:
+def iter_all_visits(sources_subset: Iterable[str | int] = ()) -> Iterator[Res[DbVisit]]:
     cfg = config.get()
     output_dir = cfg.output_dir
     # not sure if belongs here??
@@ -74,7 +75,7 @@ def iter_all_visits(sources_subset: Iterable[Union[str, int]]=()) -> Iterator[Re
         logger.warning("unknown --sources: %s", ", ".join(repr(i) for i in sources_subset))
 
 
-def _do_index(dry: bool=False, sources_subset: Iterable[Union[str, int]]=(), overwrite_db: bool=False) -> Iterable[Exception]:
+def _do_index(*, dry: bool = False, sources_subset: Iterable[str | int] = (), overwrite_db: bool = False) -> Iterable[Exception]:
     # also keep & return errors for further display
     errors: list[Exception] = []
     def it() -> Iterable[Res[DbVisit]]:
@@ -98,9 +99,10 @@ def it() -> Iterable[Res[DbVisit]]:
 
 def do_index(
     config_file: Path,
-    dry: bool=False,
-    sources_subset: Iterable[Union[str, int]]=(),
-    overwrite_db: bool=False,
+    *,
+    dry: bool = False,
+    sources_subset: Iterable[str | int] = (),
+    overwrite_db: bool = False,
 ) -> Sequence[Exception]:
     config.load_from(config_file) # meh.. should be cleaner
     try:
@@ -120,7 +122,8 @@ def demo_sources() -> dict[str, Callable[[], Extractor]]:
     def lazy(name: str) -> Callable[[], Extractor]:
         # helper to avoid failed imports etc, since people might be lacking necessary dependencies
         def inner() -> Extractor:
-            from . import sources
+            # TODO why this import??
+            from . import sources  # noqa: F401
             module = importlib.import_module(f'promnesia.sources.{name}')
             return getattr(module, 'index')
         return inner
@@ -145,7 +148,7 @@ def do_demo(
         config_file: Path | None,
         dry: bool=False,
         name: str='demo',
-        sources_subset: Iterable[Union[str, int]]=(),
+        sources_subset: Iterable[str | int]=(),
         overwrite_db: bool=False,
     ) -> None:
     with TemporaryDirectory() as tdir:
@@ -219,9 +222,10 @@ def _config_check(cfg: Path) -> Iterable[Exception]:
     logger.info('config: %s', cfg)
 
     def check(cmd: list[str | Path], **kwargs) -> Iterable[Exception]:
-        logger.debug(' '.join(map(str, cmd)))
-        res = run(cmd, **kwargs)
+        logger.debug(shlex.join(map(str, cmd)))
+        res = run(cmd, **kwargs)  # noqa: PLW1510
         if res.returncode > 0:
+            # TODO what's up with empty exception??
             yield Exception()
 
     logger.info('Checking syntax...')
@@ -239,7 +243,7 @@ def check(cmd: list[str | Path], **kwargs) -> Iterable[Exception]:
     # todo not sure if should be more defensive than check_call here
     logger.info('Checking type safety...')
     try:
-        import mypy
+        import mypy  # noqa: F401
     except ImportError:
         logger.warning("mypy not found, can't use it to check config!")
     else:
@@ -291,7 +295,7 @@ def cli_doctor_server(args: argparse.Namespace) -> None:
     logger.info('You should see the database path and version above!')
 
 
-def _ordinal_or_name(s: str) -> Union[str, int]:
+def _ordinal_or_name(s: str) -> str | int:
     try:
         s = int(s)  # type: ignore
     except ValueError:
@@ -328,7 +332,7 @@ def add_index_args(parser: argparse.ArgumentParser, default_config_path: PathIsh
 
     F = lambda prog: argparse.ArgumentDefaultsHelpFormatter(prog, width=120)
     p = argparse.ArgumentParser(formatter_class=F)
-    subp = p.add_subparsers(dest='mode', )
+    subp = p.add_subparsers(dest='mode' )
     ep = subp.add_parser('index', help='Create/update the link database', formatter_class=F)
     add_index_args(ep, default_config_path())
     # TODO use some way to override or provide config only via cmdline?
@@ -348,7 +352,7 @@ def add_index_args(parser: argparse.ArgumentParser, default_config_path: PathIsh
     ap.add_argument('--no-serve', action='store_const', const=None, dest='port', help='Pass to only index without running server')
     ap.add_argument(
         '--as',
-        choices=list(sorted(demo_sources().keys())),
+        choices=sorted(demo_sources().keys()),
         default='guess',
         help='Promnesia source to index as (see https://github.com/karlicoss/promnesia/tree/master/src/promnesia/sources for the full list)',
     )
@@ -359,7 +363,7 @@ def add_index_args(parser: argparse.ArgumentParser, default_config_path: PathIsh
     install_server.setup_parser(isp)
 
     cp = subp.add_parser('config', help='Config management')
-    cp.set_defaults(func=lambda *args: cp.print_help())
+    cp.set_defaults(func=lambda *_args: cp.print_help())
     scp = cp.add_subparsers()
     ccp = scp.add_parser('check', help='Check config')
     ccp.set_defaults(func=config_check)
@@ -373,7 +377,7 @@ def add_index_args(parser: argparse.ArgumentParser, default_config_path: PathIsh
 
     dp = subp.add_parser('doctor', help='Troubleshooting assistant')
     dp.add_argument('--config', type=Path, default=default_config_path(), help='Config path')
-    dp.set_defaults(func=lambda *args: dp.print_help())
+    dp.set_defaults(func=lambda *_args: dp.print_help())
     sdp = dp.add_subparsers()
     sdp.add_parser('config'  , help='Check config'    ).set_defaults(func=config_check )
     sdp.add_parser('database', help='Inspect database').set_defaults(func=cli_doctor_db)