Fix legacy ruff errors and enable corresponding rules for future linting (#134)

* fix ruff format

* manual ruff fixes, addressing all RET errors

* fix or ignore mypy errors

* fix tests

* fix typos and mypy

* fix linting

* revert print removal

* only ignore subset of pylint refactor rules

* fix legacy bug mentioned in #134 (comment)

Author: janosh

.pre-commit-config.yaml

@@ -3,7 +3,7 @@ default_language_version:
 exclude: ^(.github/|tests/test_data/abinit/)
 repos:
 - repo: https://github.com/charliermarsh/ruff-pre-commit
-  rev: v0.4.5
+  rev: v0.4.6
   hooks:
   - id: ruff
     args: [--fix]
@@ -39,7 +39,7 @@ repos:
     - types-paramiko
     - pydantic~=2.0
 - repo: https://github.com/codespell-project/codespell
-  rev: v2.2.6
+  rev: v2.3.0
   hooks:
   - id: codespell
     stages: [commit, commit-msg]

CODE_OF_CONDUCT.md

@@ -6,7 +6,7 @@
 We as members, contributors, and leaders pledge to make participation in our
 community a harassment-free experience for everyone, regardless of age, body
 size, visible or invisible disability, ethnicity, sex characteristics, gender
-identity and expression, level of experience, education, socio-economic status,
+identity and expression, level of experience, education, socioeconomic status,
 nationality, personal appearance, race, caste, color, religion, or sexual
 identity and orientation.
 

doc/source/conf.py

@@ -1,4 +1,4 @@
-#
+# ruff: noqa: INP001
 # Configuration file for the Sphinx documentation builder.
 #
 # This file does only contain a selection of the most common options. For a
@@ -22,7 +22,7 @@
 # -- Project information -----------------------------------------------------
 
 project = "Jobflow Remote"
-copyright = "2023, Matgenix SRL"
+copyright = "2023, Matgenix SRL"  # noqa: A001
 author = "Guido Petretto, Matthew Evans, David Waroquiers"
 
 

pyproject.toml

@@ -73,17 +73,10 @@ target-version = "py39"
 [tool.ruff.lint]
 select = ["ALL"]
 ignore = [
-    "A001",
-    "A002",
     "ANN",     # TODO fix all ANN errors
     "ARG",     # TODO fix unused method argument
-    "B007",
-    "B028",
-    "B904",
     "BLE001",
     "C408",    # Unnecessary (dict/list/tuple) call - remove call
-    "C416",
-    "C419",
     "C901",    # function too complex
     "COM812",  # trailing comma missing
     "D",
@@ -95,57 +88,34 @@ ignore = [
     "FA100",   # TODO fix FA errors
     "FBT001",
     "FBT002",
-    "FBT003",
     "FIX002",
     "G004",    # logging uses fstring
-    "G201",
-    "INP001",
     "ISC001",
     "N802",    # TODO maybe fix these
-    "N805",
-    "N818",
     "PD011",   # pandas-use-of-dot-values
-    "PERF102",
     "PERF203", # try-except-in-loop
-    "PERF401",
     "PGH003",
-    "PGH004",
-    "PLR",     # pylint-refactor
-    "PLW0602",
-    "PLW0603",
-    "PLW2901",
-    "PT003",
+    "PLR0911", # too many returns
+    "PLR0912", # too many branches
+    "PLR0913", # too many arguments
+    "PLR0915", # too many statements
+    "PLR2004", # magic value used in comparison
     "PT004",   # pytest-missing-fixture-name-underscore
     "PT006",   # pytest-parametrize-names-wrong-type
     "PT013",   # pytest-incorrect-pytest-import
     "PTH",     # prefer Pathlib to os.path
-    "PYI024",
-    "RET",
-    "RET504",
-    "RUF005",
-    "RUF012",
     "RUF013",  # implicit-optional
-    "RUF015",
     "S106",
     "S110",
-    "S112",
     "S311",
     "S324",    # use of insecure hash function
     "S507",    # paramiko auto trust
     "S602",
-    "S603",
-    "S607",
-    "SIM102",
-    "SIM105",
-    "SIM108",
-    "SIM117",
+    "SIM105",  # contextlib.suppress(Exception) instead of try-except
     "SLF",     # private member accessed outside class
-    "SLOT000",
-    "T201",
-    "TCH",
+    "T201",    # print statement
     "TD",      # TODOs
-    "TRY",     # long message outside exception class
-    "UP031",
+    "TRY003",  # long message outside exception class
 ]
 pydocstyle.convention = "numpy"
 isort.known-first-party = ["jobflow_remote"]
@@ -159,7 +129,6 @@ docstring-code-format = true
 "**/tests/*" = ["INP001", "S101"]
 "**/testing/*" = ["S101"]
 
-
 [tool.mypy]
 ignore_missing_imports = true
 strict_optional = false

src/jobflow_remote/__init__.py

@@ -1,4 +1,4 @@
-"""jobflow-remote is a python package to run jobflow workflows on remote resources"""
+"""jobflow-remote is a python package to run jobflow workflows on remote resources."""
 
 from jobflow_remote._version import __version__
 from jobflow_remote.config.jobconfig import set_run_config

src/jobflow_remote/cli/__init__.py

@@ -1,4 +1,3 @@
-# ruff: noqa: F401
 # Import the submodules with a local app to register them to the main app
 import jobflow_remote.cli.admin
 import jobflow_remote.cli.execution

src/jobflow_remote/cli/admin.py

@@ -53,7 +53,7 @@ def reset(
         ),
     ] = 25,
     force: force_opt = False,
-):
+) -> None:
     """
     Reset the jobflow database.
     WARNING: deletes all the data. These could not be retrieved anymore.
@@ -73,7 +73,7 @@ def reset(
         confirmed = Confirm.ask(text, default=False)
         if not confirmed:
             raise typer.Exit(0)
-    with loading_spinner(False) as progress:
+    with loading_spinner(processing=False) as progress:
         progress.add_task(description="Resetting the DB...", total=None)
         jc = get_job_controller()
         done = jc.reset(reset_output=reset_output, max_limit=max_limit)
@@ -94,11 +94,11 @@ def remove_lock(
     start_date: start_date_opt = None,
     end_date: end_date_opt = None,
     force: force_opt = False,
-):
+) -> None:
     """
     DEPRECATED: use unlock instead
     Forcibly removes the lock from the documents of the selected jobs.
-    WARNING: can lead to inconsistencies if the processes is actually running
+    WARNING: can lead to inconsistencies if the processes is actually running.
     """
     out_console.print(
         "remove-lock command has been DEPRECATED. Use unlock instead.",
@@ -123,17 +123,17 @@ def unlock(
     start_date: start_date_opt = None,
     end_date: end_date_opt = None,
     force: force_opt = False,
-):
+) -> None:
     """
     Forcibly removes the lock from the documents of the selected jobs.
-    WARNING: can lead to inconsistencies if the processes is actually running
+    WARNING: can lead to inconsistencies if the processes is actually running.
     """
     job_ids_indexes = get_job_ids_indexes(job_id)
 
     jc = get_job_controller()
 
     if not force:
-        with loading_spinner(False) as progress:
+        with loading_spinner(processing=False) as progress:
             progress.add_task(
                 description="Checking the number of locked documents...", total=None
             )
@@ -158,7 +158,7 @@ def unlock(
         if not confirmed:
             raise typer.Exit(0)
 
-    with loading_spinner(False) as progress:
+    with loading_spinner(processing=False) as progress:
         progress.add_task(description="Unlocking jobs...", total=None)
 
         num_unlocked = jc.unlock_jobs(
@@ -181,17 +181,17 @@ def unlock_flow(
     start_date: start_date_opt = None,
     end_date: end_date_opt = None,
     force: force_opt = False,
-):
+) -> None:
     """
     Forcibly removes the lock from the documents of the selected jobs.
-    WARNING: can lead to inconsistencies if the processes is actually running
+    WARNING: can lead to inconsistencies if the processes is actually running.
     """
     job_ids_indexes = get_job_ids_indexes(job_id)
 
     jc = get_job_controller()
 
     if not force:
-        with loading_spinner(False) as progress:
+        with loading_spinner(processing=False) as progress:
             progress.add_task(
                 description="Checking the number of locked documents...", total=None
             )
@@ -217,7 +217,7 @@ def unlock_flow(
         if not confirmed:
             raise typer.Exit(0)
 
-    with loading_spinner(False) as progress:
+    with loading_spinner(processing=False) as progress:
         progress.add_task(description="Unlocking flows...", total=None)
 
         num_unlocked = jc.unlock_flows(

src/jobflow_remote/cli/execution.py

@@ -23,10 +23,8 @@ def run(
             help="The path to the folder where the files of the job to run will be executed",
         ),
     ] = ".",
-):
-    """
-    Run the Job in the selected folder based on the
-    """
+) -> None:
+    """Run the Job in the selected folder based on the."""
     run_remote_job(run_dir)
 
 
@@ -78,10 +76,8 @@ def run_batch(
             help=("The maximum number of jobs that will be executed by the batch job"),
         ),
     ] = None,
-):
-    """
-    Run Jobs in batch mode
-    """
+) -> None:
+    """Run Jobs in batch mode."""
     run_batch_jobs(
         base_run_dir,
         files_dir,

src/jobflow_remote/cli/flow.py

@@ -61,10 +61,8 @@ def flows_list(
     max_results: max_results_opt = 100,
     sort: sort_opt = SortOption.UPDATED_ON,
     reverse_sort: reverse_sort_flag_opt = False,
-):
-    """
-    Get the list of Jobs in the database
-    """
+) -> None:
+    """Get the list of Jobs in the database."""
     check_incompatible_opt({"start_date": start_date, "days": days, "hours": hours})
     check_incompatible_opt({"end_date": end_date, "days": days, "hours": hours})
 
@@ -90,12 +88,11 @@ def flows_list(
 
         table = get_flow_info_table(flows_info, verbosity=verbosity)
 
-    if SETTINGS.cli_suggestions:
-        if max_results and len(flows_info) == max_results:
-            out_console.print(
-                f"The number of Flows printed is limited by the maximum selected: {max_results}",
-                style="yellow",
-            )
+    if SETTINGS.cli_suggestions and max_results and len(flows_info) == max_results:
+        out_console.print(
+            f"The number of Flows printed is limited by the maximum selected: {max_results}",
+            style="yellow",
+        )
 
     out_console.print(table)
 
@@ -130,10 +127,8 @@ def delete(
             help="Also delete the outputs of the Jobs in the output Store",
         ),
     ] = False,
-):
-    """
-    Permanently delete Flows from the database
-    """
+) -> None:
+    """Permanently delete Flows from the database"""
     check_incompatible_opt({"start_date": start_date, "days": days, "hours": hours})
     check_incompatible_opt({"end_date": end_date, "days": days, "hours": hours})
 
@@ -145,7 +140,7 @@ def delete(
     # the verbosity value will be decreased by one: the first is to enable
     # initial print
 
-    with loading_spinner(False) as progress:
+    with loading_spinner(processing=False) as progress:
         progress.add_task(description="Fetching data...", total=None)
         flows_info = jc.get_flows_info(
             job_ids=job_id,
@@ -180,7 +175,7 @@ def delete(
             raise typer.Exit(0)
 
     to_delete = [fi.flow_id for fi in flows_info]
-    with loading_spinner(False) as progress:
+    with loading_spinner(processing=False) as progress:
         progress.add_task(description="Deleting...", total=None)
 
         jc.delete_flows(flow_ids=to_delete, delete_output=delete_output)
@@ -194,10 +189,8 @@ def delete(
 def flow_info(
     flow_db_id: flow_db_id_arg,
     job_id_flag: job_flow_id_flag_opt = False,
-):
-    """
-    Provide detailed information on a Flow
-    """
+) -> None:
+    """Provide detailed information on a Flow."""
     db_id, jf_id = get_job_db_ids(flow_db_id, None)
     db_ids = job_ids = flow_ids = None
     if db_id is not None:
@@ -259,10 +252,8 @@ def graph(
             help="Print the mermaid graph",
         ),
     ] = False,
-):
-    """
-    Provide detailed information on a Flow
-    """
+) -> None:
+    """Provide detailed information on a Flow."""
     db_id, jf_id = get_job_db_ids(flow_db_id, None)
     db_ids = job_ids = flow_ids = None
     if db_id is not None:

src/jobflow_remote/cli/formatting.py

@@ -2,18 +2,21 @@
 
 import datetime
 import time
+from typing import TYPE_CHECKING
 
 from monty.json import jsanitize
 from rich.scope import render_scope
 from rich.table import Table
 from rich.text import Text
 
 from jobflow_remote.cli.utils import ReprStr, fmt_datetime
-from jobflow_remote.config.base import ExecutionConfig, WorkerBase
-from jobflow_remote.jobs.data import FlowInfo, JobDoc, JobInfo
 from jobflow_remote.jobs.state import JobState
 from jobflow_remote.utils.data import convert_utc_time
 
+if TYPE_CHECKING:
+    from jobflow_remote.config.base import ExecutionConfig, WorkerBase
+    from jobflow_remote.jobs.data import FlowInfo, JobDoc, JobInfo
+
 
 def get_job_info_table(jobs_info: list[JobInfo], verbosity: int):
     time_zone_str = f" [{time.tzname[0]}]"