Add linting with ruff

ruff (https://github.com/charliermarsh/ruff) is a new and extremely fast
Python linter, written in Rust.
Ruff can be used to replace Flake8 (plus a variety of plugins), isort,
pydocstyle, yesqa, eradicate, pyupgrade, and autoflake.
Here ruff is added to our current linters (flake8, flake8-bugbear and isort)
while we assess its reliability.
I've not enabled the isort rules because many of the options we use are not
implemented yet. I have instead enabled the pyupgrade rules (a tool which I
run manually every few months).

Also:
- Fix new linting errors reported by ruff.

.flake8

@@ -3,7 +3,7 @@
 # B008 Do not perform function calls in argument defaults (for FastAPI Depends and Body)
 # E203 is whitespace before ':'; we follow black's formatting here. See https://black.readthedocs.io/en/stable/faq.html#why-are-flake8-s-e203-and-w503-violated
 # E402 module level import not at top of file # TODO, we would like to improve this.
-# E501 is line length
+# E501 is line length (delegated to black)
 # W503 is line breaks before binary operators, which has been reversed in PEP 8.
 # D** are docstring linting - which we mostly ignore except D302. (Hopefully we will solve more over time).
 ignore = B008,E203,E402,E501,W503,D100,D101,D102,D103,D104,D105,D106,D107,D200,D201,D202,D204,D205,D206,D207,D208,D209,D210,D211,D300,D301,D400,D401,D402,D403,D412,D413

CONTRIBUTING.md

@@ -157,8 +157,8 @@ https://help.github.com/en/github/getting-started-with-github/git-and-github-lea
 - Galaxy follows [PEP-8](https://www.python.org/dev/peps/pep-0008/), with
   particular emphasis on readability being the ultimate goal:
   - 4 spaces (not tabs!) per indentation level
-  - divergences from PEP-8 are listed in the `[flake8]` section of the
-    `.flake8` file
+  - divergences from PEP-8 are listed in the `[flake8]` section of the `.flake8`
+    file and in the `[tool.ruff]` section of the `pyproject.toml` file.
   - The Python code base is automatically formatted using
     [isort](https://pycqa.github.io/isort/) (for imports) and
     [black](https://black.readthedocs.io). To easily format your Python code

Makefile

@@ -1,6 +1,6 @@
 # Location of virtualenv used for development.
 VENV?=.venv
-# Source virtualenv to execute command (flake8, sphinx, twine, etc...)
+# Source virtualenv to execute command (darker, sphinx, twine, etc...)
 IN_VENV=if [ -f "$(VENV)/bin/activate" ]; then . "$(VENV)/bin/activate"; fi;
 RELEASE_CURR:=23.0
 RELEASE_UPSTREAM:=upstream

contrib/galaxy_config_merger.py

@@ -30,7 +30,7 @@ def main():
 
     for option in ["sample", "config"]:
         if getattr(options, option) is None:
-            print("Please supply a --%s parameter.\n" % (option))
+            print(f"Please supply a --{option} parameter.\n")
             parser.print_help()
             sys.exit()
 
@@ -45,11 +45,11 @@ def main():
     logging.info("---------- DIFFERENCE ANALYSIS BEGIN ----------")
     for section in config.sections():
         if not config_sample.has_section(section):
-            logging.warning("-MISSING- section [%s] not found in sample file. It will be ignored." % section)
+            logging.warning("-MISSING- section [%s] not found in sample file. It will be ignored.", section)
         else:
             for (name, value) in config.items(section):
                 if not config_sample.has_option(section, name):
-                    if not "#%s" % name in config_sample_content:
+                    if f"#{name}" not in config_sample_content:
                         logging.warning(
                             f"-MISSING- section [{section}] option '{name}' not found in sample file. It will be ignored."
                         )

lib/galaxy/celery/tasks.py

@@ -221,12 +221,11 @@ def is_aborted(session: galaxy_scoped_session, job_id: int):
     ).scalar()
 
 
-def abort_when_job_stops(function: Callable, session: galaxy_scoped_session, job_id: int, *args, **kwargs) -> Any:
+def abort_when_job_stops(function: Callable, session: galaxy_scoped_session, job_id: int, **kwargs) -> Any:
     if not is_aborted(session, job_id):
         future = celery_app.fork_pool.submit(
             function,
             timeout=None,
-            *args,
             **kwargs,
         )
         while True:

lib/galaxy/datatypes/genetics.py

@@ -1108,7 +1108,7 @@ def sniff_prefix(self, file_prefix: FilePrefix) -> bool:
                 try:
                     int(chrm)
                 except ValueError:
-                    if not chrm.lower()[0] in ("x", "y", "m"):
+                    if chrm.lower()[0] not in ("x", "y", "m"):
                         return False
 
             except ValueError:

lib/galaxy/datatypes/proteomics.py

@@ -173,7 +173,7 @@ def sniff_prefix(self, file_prefix: FilePrefix) -> bool:
                     mandatory_field = self._man_mtd[columns[1]]
                     if mandatory_field is None or columns[2].lower() in mandatory_field:
                         found_man_mtd.add(columns[1])
-            elif not columns[0] in self._sections:
+            elif columns[0] not in self._sections:
                 return False
         return has_version and found_man_mtd == set(self._man_mtd.keys())
 

lib/galaxy/datatypes/tabular.py

@@ -825,8 +825,8 @@ def set_meta(
                             break
                 else:
                     # Otherwise, read the whole thing and set num data lines.
-                    for i, l in enumerate(dataset_fh):  # noqa: B007
-                        if l.startswith("@"):
+                    for i, line in enumerate(dataset_fh):  # noqa: B007
+                        if line.startswith("@"):
                             comment_lines += 1
                     dataset.metadata.data_lines = i + 1 - comment_lines
             dataset.metadata.comment_lines = comment_lines

lib/galaxy/dependencies/lint-requirements.txt

@@ -1,2 +1,3 @@
 flake8
 flake8-bugbear
+ruff

lib/galaxy/dependencies/pinned-lint-requirements.txt

@@ -5,5 +5,6 @@ importlib-metadata==4.2.0
 mccabe==0.7.0
 pycodestyle==2.9.1
 pyflakes==2.5.0
+ruff==0.0.223
 typing_extensions==4.4.0
 zipp==3.11.0

lib/galaxy/managers/configuration.py

@@ -159,7 +159,6 @@ def _config_is_truthy(item, key, **context):
             "simplified_workflow_run_ui": _use_config,
             "simplified_workflow_run_ui_target_history": _use_config,
             "simplified_workflow_run_ui_job_cache": _use_config,
-            "simplified_workflow_run_ui": _use_config,
             "has_user_tool_filters": _defaults_to(False),
             # TODO: is there no 'correct' way to get an api url? controller='api', action='tools' is a hack
             # at any rate: the following works with path_prefix but is still brittle

lib/galaxy/tool_util/deps/brew_exts.py

@@ -29,6 +29,10 @@
 import string
 import subprocess
 import sys
+from typing import (
+    List,
+    Tuple,
+)
 
 WHITESPACE_PATTERN = re.compile(r"[\s]+")
 
@@ -76,7 +80,7 @@ def cellar_path(self):
         return recipe_cellar_path(self.brew_context.homebrew_cellar, self.recipe, self.version)
 
     @property
-    def tap_path(self):
+    def tap_path(self) -> str:
         return os.path.join(self.brew_context.homebrew_prefix, "Library", "Taps", self.__tap_path(self.recipe))
 
     def __tap_path(self, recipe):
@@ -234,7 +238,7 @@ def versioned_install(recipe_context, package=None, version=None, installed_deps
             attempt_unlink_all(package, deps)
 
 
-def commit_for_version(recipe_context, package, version):
+def commit_for_version(recipe_context: RecipeContext, package, version):
     tap_path = recipe_context.tap_path
     commit = None
     with brew_head_at_commit("master", tap_path):
@@ -490,8 +494,8 @@ def extended_brew_info(recipe):
     return extra_info
 
 
-def brew_versions_info(package, tap_path):
-    def versioned(recipe_path):
+def brew_versions_info(package, tap_path: str) -> List[Tuple[str, str, bool]]:
+    def versioned(recipe_path: str):
         if not os.path.isabs(recipe_path):
             recipe_path = os.path.join(os.getcwd(), recipe_path)
         # Dependencies in the same repository should be versioned,
@@ -502,7 +506,7 @@ def versioned(recipe_path):
     # TODO: Also use tags.
     stdout = brew_execute(["versions", package])
     version_parts = [line for line in stdout.split("\n") if line and "git checkout" in line]
-    version_parts = map(lambda l: WHITESPACE_PATTERN.split(l), version_parts)
+    version_parts = [WHITESPACE_PATTERN.split(line) for line in version_parts]
     info = [(p[0], p[3], versioned(p[4])) for p in version_parts]
     return info
 

lib/galaxy/tool_util/linters/_util.py

@@ -8,4 +8,4 @@ def is_datasource(tool_xml):
 
 def is_valid_cheetah_placeholder(name):
     """Returns true if name is a valid Cheetah placeholder"""
-    return not re.match(r"^[a-zA-Z_]\w*$", name) is None
+    return re.match(r"^[a-zA-Z_]\w*$", name) is not None

lib/galaxy/tool_util/provided_metadata.py

@@ -167,7 +167,7 @@ def get_unnamed_outputs(self):
         return []
 
     def rewrite(self):
-        with open(self.meta_file, "wt") as job_metadata_fh:
+        with open(self.meta_file, "w") as job_metadata_fh:
             for meta in self.tool_provided_job_metadata:
                 job_metadata_fh.write(f"{json.dumps(meta)}\n")
 
@@ -230,5 +230,5 @@ def get_unnamed_outputs(self):
         return self.tool_provided_job_metadata.get("__unnamed_outputs", [])
 
     def rewrite(self):
-        with open(self.meta_file, "wt") as job_metadata_fh:
+        with open(self.meta_file, "w") as job_metadata_fh:
             json.dump(self.tool_provided_job_metadata, job_metadata_fh)

lib/galaxy/tool_util/verify/asserts/text.py

@@ -64,8 +64,8 @@ def assert_has_line(
         min,
         max,
         negate,
-        lambda o, l: re.search(f"^{re.escape(l)}$", o, flags=re.MULTILINE) is not None,
-        lambda o, l: len(re.findall(f"^{re.escape(l)}$", o, flags=re.MULTILINE)),
+        lambda o, t: re.search(f"^{re.escape(t)}$", o, flags=re.MULTILINE) is not None,
+        lambda o, t: len(re.findall(f"^{re.escape(t)}$", o, flags=re.MULTILINE)),
         "{expected} line '{text}' in output ('{output}')",
         "{expected} {n}+-{delta} lines '{text}' in output ('{output}')",
         "{expected} that the number of lines '{text}' in output is in [{min}:{max}] ('{output}')",

lib/galaxy/tools/parameters/dynamic_options.py

@@ -113,7 +113,7 @@ def filter_options(self, options, trans, other_values):
             pass
         filter_pattern = re.compile(filter_value)
         for fields in options:
-            if self.keep == (not filter_pattern.match(fields[self.column]) is None):
+            if self.keep == (filter_pattern.match(fields[self.column]) is not None):
                 rval.append(fields)
         return rval
 

lib/galaxy/util/xml_macros.py

@@ -131,14 +131,14 @@ def _expand_tokens_for_el(element, tokens):
     value = element.text
     if value:
         new_value = _expand_tokens_str(element.text, tokens)
-        if not (new_value is value):
+        if new_value is not value:
             element.text = new_value
     for key, value in element.attrib.items():
         new_value = _expand_tokens_str(value, tokens)
-        if not (new_value is value):
+        if new_value is not value:
             element.attrib[key] = new_value
         new_key = _expand_tokens_str(key, tokens)
-        if not (new_key is key):
+        if new_key is not key:
             element.attrib[new_key] = element.attrib[key]
             del element.attrib[key]
     # recursively expand in childrens

lib/galaxy_test/api/test_jobs.py

@@ -975,7 +975,7 @@ def __uploads_with_state(self, *states):
         jobs_response = self._get("jobs", data=dict(state=states))
         self._assert_status_code_is(jobs_response, 200)
         jobs = jobs_response.json()
-        assert not [j for j in jobs if not j["state"] in states]
+        assert not [j for j in jobs if j["state"] not in states]
         return [j for j in jobs if j["tool_id"] == "__DATA_FETCH__"]
 
     def __history_with_new_dataset(self, history_id):

pyproject.toml

@@ -146,3 +146,27 @@ testfixtures = "*"
 tuspy = "*"
 twill = "*"
 watchdog = "*"
+
+[tool.ruff]
+select = ["E", "F", "B", "UP"]
+target-version = "py37"
+# Exceptions:
+# B008 Do not perform function calls in argument defaults (for FastAPI Depends and Body)
+# B9 flake8-bugbear opinionated warnings
+# E402 module level import not at top of file # TODO, we would like to improve this.
+# E501 is line length (delegated to black)
+ignore = ["B008", "B9", "E402", "E501"]
+
+[tool.ruff.isort]
+# We are not selecting "I" rules in ruff yet because support for all the isort
+# options we need is not complete, but these are the one currently implemented.
+combine-as-imports = true
+relative-imports-order = "closest-to-furthest"
+
+[tool.ruff.per-file-ignores]
+# Don't check pyupgrade rules on tool scripts, which may use different Python versions
+"test/functional/tools/*" = ["UP"]
+"tools/*" = ["UP"]
+
+[tool.ruff.pyupgrade]
+keep-runtime-typing = true  # https://github.com/charliermarsh/ruff/issues/1617

test/unit/app/managers/test_HDAManager.py

@@ -495,9 +495,9 @@ def test_file_name_serializers(self):
         self.log("file_name should be skipped for non-admin when not exposed by config")
         self.app.config.expose_dataset_path = False
         serialized = self.hda_serializer.serialize(hda, keys, user=None)
-        assert not ("file_name" in serialized)
+        assert "file_name" not in serialized
         serialized = self.hda_serializer.serialize(hda, keys, user=owner)
-        assert not ("file_name" in serialized)
+        assert "file_name" not in serialized
 
         self.log("file_name should be sent for admin in either case")
         serialized = self.hda_serializer.serialize(hda, keys, user=self.admin_user)

test/unit/webapps/test_webapp_base.py

@@ -48,7 +48,7 @@ def assert_cors_header_equals(self, headers, should_be):
         assert headers.get("access-control-allow-origin", None) == should_be
 
     def assert_cors_header_missing(self, headers):
-        assert not ("access-control-allow-origin" in headers)
+        assert "access-control-allow-origin" not in headers
 
     def test_parse_allowed_origin_hostnames(self) -> None:
         """Should return a list of (possibly) mixed strings and regexps"""

tools/data_export/export_remote.py

@@ -8,7 +8,7 @@
 
 def get_file_sources(file_sources_path):
     assert os.path.exists(file_sources_path), f"file sources path [{file_sources_path}] does not exist"
-    with open(file_sources_path, "r") as f:
+    with open(file_sources_path) as f:
         file_sources_as_dict = json.load(f)
     file_sources = ConfiguredFileSources.from_dict(file_sources_as_dict)
     return file_sources

tools/stats/filtering.py

@@ -288,7 +288,7 @@ def stop_err(msg):
 invalid_line = None
 lines_kept = 0
 total_lines = 0
-out = open(out_fname, "wt")
+out = open(out_fname, "w")
 
 # Read and filter input file, skipping invalid lines
 code = """

tox.ini

@@ -7,6 +7,9 @@ skipsdist = True
 [testenv]
 commands =
     first_startup: bash .ci/first_startup.sh
+    lint: ruff .
+    # Once we are happy to replace flake8 with ruff, we can replace the next line with `black --check --diff`
+    # (since ruff delegates to black for checking/fixing many rules).
     lint: bash .ci/flake8_wrapper.sh
     lint_docstring: bash .ci/flake8_wrapper_docstrings.sh --exclude
     lint_docstring_include_list: bash .ci/flake8_wrapper_docstrings.sh --include