From ba8cf92614e669107c0c1cde56d45087e232698b Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 10:58:06 -0400 Subject: [PATCH 01/24] Enable all pycodestyle checks --- pyproject.toml | 8 ++------ src/stpipe/config_parser.py | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1dbbdda8..4e9f4319 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -79,12 +79,8 @@ filterwarnings = [ [tool.ruff] select = [ - 'E402', # module level import not at top of file - 'E501', # line too long - 'E711', # comparison to None should be ‘if cond is None:’ - 'E722', # do not use bare except, specify exception instead - 'F', # flakes - 'W', # whitespace / deprecation + 'F', # Pyflakes + 'W', 'E', # pycodestyle ] line-length = 88 extend-exclude = [ diff --git a/src/stpipe/config_parser.py b/src/stpipe/config_parser.py index 09065be1..a5c3eed3 100644 --- a/src/stpipe/config_parser.py +++ b/src/stpipe/config_parser.py @@ -355,7 +355,7 @@ def validate( else: section_list.append("[missing section]") section_string = "/".join(section_list) - if err == False: + if err is False: if allow_missing: config[key] = spec[key] continue From 838111cfa4167a1b9e3382c3b191482b0b87ab13 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 10:59:01 -0400 Subject: [PATCH 02/24] Switch to ruff for isort --- .pre-commit-config.yaml | 5 ----- pyproject.toml | 1 + tests/cli/test_list.py | 1 - tests/cli/test_main.py | 1 - tests/test_abstract_datamodel.py | 1 - tests/test_config_parser.py | 1 - tests/test_format_template.py | 1 - tests/test_integration.py | 1 - tests/test_logger.py | 1 - tests/test_step.py | 1 - 10 files changed, 1 insertion(+), 13 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 73c88920..bbc3e00c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -53,11 +53,6 @@ repos: args: ["--fix"] exclude: "scripts/strun" -- repo: https://github.com/pycqa/isort - rev: 5.12.0 - hooks: - - id: isort - - repo: https://github.com/psf/black rev: 23.10.1 hooks: diff --git a/pyproject.toml b/pyproject.toml index 4e9f4319..d020dee9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,6 +81,7 @@ filterwarnings = [ select = [ 'F', # Pyflakes 'W', 'E', # pycodestyle + 'I', # isort ] line-length = 88 extend-exclude = [ diff --git a/tests/cli/test_list.py b/tests/cli/test_list.py index 8ba721f0..64df6f06 100644 --- a/tests/cli/test_list.py +++ b/tests/cli/test_list.py @@ -1,5 +1,4 @@ import pytest - from stpipe import entry_points from stpipe.cli import handle_args from stpipe.entry_points import StepInfo diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index b941b121..cb1324e3 100644 --- a/tests/cli/test_main.py +++ b/tests/cli/test_main.py @@ -1,7 +1,6 @@ import subprocess import pytest - import stpipe from stpipe import entry_points from stpipe.cli import handle_args diff --git a/tests/test_abstract_datamodel.py b/tests/test_abstract_datamodel.py index 803cf62c..95c1bd1c 100644 --- a/tests/test_abstract_datamodel.py +++ b/tests/test_abstract_datamodel.py @@ -3,7 +3,6 @@ """ import pytest - from stpipe.datamodel import AbstractDataModel diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index bef530c0..626d81f2 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -2,7 +2,6 @@ from collections.abc import Mapping import pytest - from stpipe import config_parser from stpipe.extern.configobj.configobj import ConfigObj, Section from stpipe.utilities import _not_set diff --git a/tests/test_format_template.py b/tests/test_format_template.py index 47f771b1..9e93f6f0 100644 --- a/tests/test_format_template.py +++ b/tests/test_format_template.py @@ -2,7 +2,6 @@ from functools import partial import pytest - from stpipe.format_template import FormatTemplate diff --git a/tests/test_integration.py b/tests/test_integration.py index 757488fe..c9255e82 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -3,7 +3,6 @@ import asdf import yaml - from stpipe.integration import SCHEMAS_PATH diff --git a/tests/test_logger.py b/tests/test_logger.py index 03817dc8..fde9d57d 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -2,7 +2,6 @@ import logging import pytest - from stpipe import log as stpipe_log diff --git a/tests/test_step.py b/tests/test_step.py index e61a0d59..e9980190 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -3,7 +3,6 @@ import asdf import pytest - import stpipe.config_parser as cp from stpipe import cmdline from stpipe.pipeline import Pipeline From 223fe0bed083893077d87200a509af9c5df6f498 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 11:03:14 -0400 Subject: [PATCH 03/24] Enable pep8 naming enforcement --- pyproject.toml | 1 + src/stpipe/datamodel.py | 4 ++-- src/stpipe/exceptions.py | 4 ++-- src/stpipe/log.py | 4 ++-- src/stpipe/step.py | 1 + 5 files changed, 8 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d020dee9..65f1d47b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,6 +82,7 @@ select = [ 'F', # Pyflakes 'W', 'E', # pycodestyle 'I', # isort + 'N', # pep8-naming ] line-length = 88 extend-exclude = [ diff --git a/src/stpipe/datamodel.py b/src/stpipe/datamodel.py index 4ee7259f..e6dee69c 100644 --- a/src/stpipe/datamodel.py +++ b/src/stpipe/datamodel.py @@ -16,12 +16,12 @@ class without requiring that they inherit this class. """ @classmethod - def __subclasshook__(cls, C): + def __subclasshook__(cls, c_): """ Pseudo subclass check based on these attributes and methods """ if cls is AbstractDataModel: - mro = C.__mro__ + mro = c_.__mro__ if ( any([hasattr(CC, "crds_observatory") for CC in mro]) and any([hasattr(CC, "get_crds_parameters") for CC in mro]) diff --git a/src/stpipe/exceptions.py b/src/stpipe/exceptions.py index ba579662..7c49f43c 100644 --- a/src/stpipe/exceptions.py +++ b/src/stpipe/exceptions.py @@ -1,4 +1,4 @@ -class StpipeException(Exception): +class StpipeException(Exception): # noqa: N818 """ Base class for exceptions from the stpipe package. """ @@ -6,7 +6,7 @@ class StpipeException(Exception): pass -class StpipeExitException(StpipeException): +class StpipeExitException(StpipeException): # noqa: N818 """ An exception that carries an exit status that is returned by stpipe CLI tools. diff --git a/src/stpipe/log.py b/src/stpipe/log.py index c0dbbb16..ba4f6ce3 100644 --- a/src/stpipe/log.py +++ b/src/stpipe/log.py @@ -32,7 +32,7 @@ # LOGS AS EXCEPTIONS -class LoggedException(Exception): +class LoggedException(Exception): # noqa: N818 """ This is an exception used when a log record is converted into an exception. @@ -203,7 +203,7 @@ def _level_check(value): cfg.match_and_apply(log) -def getLogger(name=None): +def getLogger(name=None): # noqa: N802 log = logging.getLogger(name) return log diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 2145eca2..b512c346 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -561,6 +561,7 @@ def finalize_result(self, result, reference_files_used): """ pass + @staticmethod def remove_suffix(name): """ Remove a known Step filename suffix from a filename From ac866d3e8ec27655a8bd6ae35cace1e5598fa72e Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 31 Oct 2023 12:40:10 -0400 Subject: [PATCH 04/24] Switch to pyupgrade to ruff and target python 3.9 --- .pre-commit-config.yaml | 6 ------ pyproject.toml | 2 ++ src/stpipe/format_template.py | 8 ++------ 3 files changed, 4 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bbc3e00c..889c7211 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -40,12 +40,6 @@ repos: - id: flynt exclude: "src/stpipe/extern/.*" -- repo: https://github.com/asottile/pyupgrade - rev: 'v3.15.0' - hooks: - - id: pyupgrade - args: ["--py38-plus"] - - repo: https://github.com/astral-sh/ruff-pre-commit rev: 'v0.1.4' hooks: diff --git a/pyproject.toml b/pyproject.toml index 65f1d47b..1a628ef8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -78,11 +78,13 @@ filterwarnings = [ ] [tool.ruff] +target-version = "py39" select = [ 'F', # Pyflakes 'W', 'E', # pycodestyle 'I', # isort 'N', # pep8-naming + 'UP', # pyupgrade ] line-length = 88 extend-exclude = [ diff --git a/src/stpipe/format_template.py b/src/stpipe/format_template.py index f2b8eb77..5391348a 100644 --- a/src/stpipe/format_template.py +++ b/src/stpipe/format_template.py @@ -164,12 +164,8 @@ def format(self, format_string, **kwargs): break else: raise RuntimeError( - "No suitable formatting for {key}: {value} found. Given" - " formatting options:\n\t{formats}".format( - key=key, - value=value, - formats=self.key_formats[key], - ) + f"No suitable formatting for {key}: {value} found. Given" + f" formatting options:\n\t{self.key_formats[key]}" ) formatted_kwargs[key] = value result = super().format(format_string, **formatted_kwargs) From 0f926c5a073633a54d4850d1621f5624033da55c Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 11:35:15 -0400 Subject: [PATCH 05/24] Switch to ruff for bandit checks --- .pre-commit-config.yaml | 6 ------ pyproject.toml | 10 +++++----- src/stpipe/cmdline.py | 3 ++- src/stpipe/hooks.py | 9 +++------ src/stpipe/log.py | 6 ++++-- src/stpipe/step.py | 7 ++----- 6 files changed, 16 insertions(+), 25 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 889c7211..ab0e440a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -51,9 +51,3 @@ repos: rev: 23.10.1 hooks: - id: black - -- repo: https://github.com/PyCQA/bandit - rev: 1.7.5 - hooks: - - id: bandit - args: ["-r", "-ll"] diff --git a/pyproject.toml b/pyproject.toml index 1a628ef8..1b92c7d2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -85,6 +85,7 @@ select = [ 'I', # isort 'N', # pep8-naming 'UP', # pyupgrade + 'S', # flake8-bandit ] line-length = 88 extend-exclude = [ @@ -96,11 +97,10 @@ extend-ignore = [ 'W605', # invalid escape sequence ] -[tool.isort] -profile = "black" -filter_files = true -line_length = 88 -extend_skip_glob = ["src/stpipe/extern/*"] +[tool.ruff.extend-per-file-ignores] +"tests/*.py" = ["S101", "S603", "S607"] +"src/stpipe/tests/*.py" = ["S101"] + [tool.black] line-length = 88 diff --git a/src/stpipe/cmdline.py b/src/stpipe/cmdline.py index 63b17537..e88ca903 100644 --- a/src/stpipe/cmdline.py +++ b/src/stpipe/cmdline.py @@ -406,6 +406,7 @@ def step_from_cmdline(args, cls=None): def step_script(cls): import sys - assert issubclass(cls, Step) + if not issubclass(cls, Step): + raise AssertionError("cls must be a subclass of Step") return step_from_cmdline(sys.argv[1:], cls=cls) diff --git a/src/stpipe/hooks.py b/src/stpipe/hooks.py index 5f979426..0a08325e 100644 --- a/src/stpipe/hooks.py +++ b/src/stpipe/hooks.py @@ -1,6 +1,7 @@ """ Pre- and post-hooks """ +import contextlib import types from . import utilities @@ -11,21 +12,17 @@ def hook_from_string(step, type, num, command): name = f"{type}_hook{num:d}" step_class = None - try: + with contextlib.suppress(Exception): step_class = utilities.import_class(command, Step, step.config_file) - except Exception: - pass if step_class is not None: return step_class(name, parent=step, config_file=step.config_file) step_func = None - try: + with contextlib.suppress(Exception): step_func = utilities.import_class( command, types.FunctionType, step.config_file ) - except Exception: - pass if step_func is not None: from . import function_wrapper diff --git a/src/stpipe/log.py b/src/stpipe/log.py index ba4f6ce3..87eef81d 100644 --- a/src/stpipe/log.py +++ b/src/stpipe/log.py @@ -252,9 +252,11 @@ def log(self): @log.setter def log(self, log): - assert log is None or ( + if log is not None and not ( isinstance(log, logging.Logger) and log.name.startswith(STPIPE_ROOT_LOGGER) - ) + ): + raise AssertionError("Can't set the log to a root logger") + self._logs[threading.current_thread()] = log diff --git a/src/stpipe/step.py b/src/stpipe/step.py index b512c346..b22e09b7 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -5,7 +5,7 @@ import os import sys from collections.abc import Sequence -from contextlib import contextmanager +from contextlib import contextmanager, suppress from functools import partial from os.path import ( abspath, @@ -1238,12 +1238,9 @@ def _set_input_dir(self, input, exclusive=True): """ if not exclusive or self.search_attr("_input_dir") is None: - try: + with suppress(Exception): if isfile(input): self.input_dir = split(input)[0] - except Exception: - # Not a file-checkable object. Ignore. - pass def get_pars(self, full_spec=True): """Retrieve the configuration parameters of a step From 35aa96f44926b476c15c4219244ec7d2e242d051 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 11:39:31 -0400 Subject: [PATCH 06/24] Enforce flake8 bugbear --- pyproject.toml | 2 ++ src/stpipe/cmdline.py | 14 +++++++++----- src/stpipe/config.py | 4 ++-- src/stpipe/config_parser.py | 2 +- src/stpipe/entry_points.py | 3 ++- src/stpipe/log.py | 5 +++-- src/stpipe/pipeline.py | 2 +- src/stpipe/step.py | 4 +++- 8 files changed, 23 insertions(+), 13 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 1b92c7d2..c98551ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -86,6 +86,8 @@ select = [ 'N', # pep8-naming 'UP', # pyupgrade 'S', # flake8-bandit + # 'BLE', # flake8-blind-except + 'B', # flake8-bugbear ] line-length = 88 extend-exclude = [ diff --git a/src/stpipe/cmdline.py b/src/stpipe/cmdline.py index e88ca903..c05c9ce6 100644 --- a/src/stpipe/cmdline.py +++ b/src/stpipe/cmdline.py @@ -45,10 +45,10 @@ def _get_config_and_class(identifier): step_class = utilities.import_class( utilities.resolve_step_class_alias(identifier), Step ) - except (ImportError, AttributeError, TypeError): + except (ImportError, AttributeError, TypeError) as err: raise ValueError( f"{identifier!r} is not a path to a config file or a Python Step class" - ) + ) from err # Don't validate yet config = config_parser.config_from_dict({}) name = None @@ -82,7 +82,9 @@ def _build_arg_parser_from_spec(spec, step_class, parent=None): description=step_class.__doc__, ) - def build_from_spec(subspec, parts=[]): + def build_from_spec(subspec, parts=None): + if parts is None: + parts = [] for key, val in subspec.items(): if isinstance(val, dict): build_from_spec(val, parts + [key]) @@ -263,7 +265,9 @@ def just_the_step_from_cmdline(args, cls=None): try: log.load_configuration(log_config) except Exception as e: - raise ValueError(f"Error parsing logging config {log_config!r}:\n{e}") + raise ValueError( + f"Error parsing logging config {log_config!r}:\n{e}" + ) from e except Exception as e: _print_important_message("ERROR PARSING CONFIGURATION:", str(e)) parser1.print_help() @@ -336,7 +340,7 @@ def just_the_step_from_cmdline(args, cls=None): # If the configobj validator failed, print usage information. _print_important_message("ERROR PARSING CONFIGURATION:", str(e)) parser2.print_help() - raise ValueError(str(e)) + raise ValueError(str(e)) from e # Define the primary input file. # Always have an output_file set on the outermost step diff --git a/src/stpipe/config.py b/src/stpipe/config.py index a6fb3932..b7c79a09 100644 --- a/src/stpipe/config.py +++ b/src/stpipe/config.py @@ -149,10 +149,10 @@ def from_asdf(cls, asdf_file): try: _validate_asdf(asdf_file, _LEGACY_CONFIG_SCHEMA_URI) return cls._from_legacy_tree(asdf_file.tree) - except asdf.ValidationError: + except asdf.ValidationError as err: # Raise the original error so that we encourage # use of the new config file format. - raise e + raise e from err @classmethod def _from_tree(cls, tree): diff --git a/src/stpipe/config_parser.py b/src/stpipe/config_parser.py index a5c3eed3..5af47794 100644 --- a/src/stpipe/config_parser.py +++ b/src/stpipe/config_parser.py @@ -187,7 +187,7 @@ def load_spec_file(cls, preserve_comments=_not_set): """ if preserve_comments is not _not_set: msg = "preserve_comments is deprecated" - warnings.warn(msg, DeprecationWarning) + warnings.warn(msg, DeprecationWarning, stacklevel=2) # Don't use 'hasattr' here, because we don't want to inherit spec # from the base class. if not isclass(cls): diff --git a/src/stpipe/entry_points.py b/src/stpipe/entry_points.py index 061f67c2..02bfb261 100644 --- a/src/stpipe/entry_points.py +++ b/src/stpipe/entry_points.py @@ -40,7 +40,8 @@ class alias, and the third is a bool indicating whether the class is to be warnings.warn( f"{STEPS_GROUP} plugin from package {package_name}=={package_version} " "failed to load:\n\n" - f"{e.__class__.__name__}: {e}" + f"{e.__class__.__name__}: {e}", + stacklevel=2, ) steps.extend(package_steps) diff --git a/src/stpipe/log.py b/src/stpipe/log.py index 87eef81d..a715e422 100644 --- a/src/stpipe/log.py +++ b/src/stpipe/log.py @@ -180,10 +180,11 @@ def _level_check(value): value = int(value) except ValueError: pass + try: value = logging._checkLevel(value) - except ValueError: - raise validate.VdtTypeError(value) + except ValueError as err: + raise validate.VdtTypeError(value) from err return value spec = config_parser.load_spec_file(LogConfig) diff --git a/src/stpipe/pipeline.py b/src/stpipe/pipeline.py index 357cc0a0..c69421d9 100644 --- a/src/stpipe/pipeline.py +++ b/src/stpipe/pipeline.py @@ -344,7 +344,7 @@ def get_pars(self, full_spec=True): """ pars = super().get_pars(full_spec=full_spec) pars["steps"] = {} - for step_name, step_class in self.step_defs.items(): + for step_name, _step_class in self.step_defs.items(): pars["steps"][step_name] = getattr(self, step_name).get_pars( full_spec=full_spec ) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index b22e09b7..8ee34ae5 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -479,7 +479,9 @@ def run(self, *args): step_result = self.process(*args) except TypeError as e: if "process() takes exactly" in str(e): - raise TypeError("Incorrect number of arguments to step") + raise TypeError( + "Incorrect number of arguments to step" + ) from e raise # Warn if returning a discouraged object From 739d82014e64d422969a1f3451594959bfd96020 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 11:48:01 -0400 Subject: [PATCH 07/24] Use flake8 builtin shadowing linting --- pyproject.toml | 25 +++++++++++++++++++++++++ src/stpipe/config_parser.py | 6 +++--- src/stpipe/format_template.py | 2 +- src/stpipe/hooks.py | 6 +++--- src/stpipe/log.py | 4 ++-- src/stpipe/step.py | 12 ++++++------ src/stpipe/utilities.py | 4 ++-- 7 files changed, 42 insertions(+), 17 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c98551ba..c4f5ccc4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -88,6 +88,31 @@ select = [ 'S', # flake8-bandit # 'BLE', # flake8-blind-except 'B', # flake8-bugbear + 'A', # flake8-builtins (prevent shadowing of builtins) + # 'C4', # flake8-comprehensions (best practices for comprehensions) + # 'T10', # flake8-debugger (prevent debugger statements in code) + # 'ISC', # flake8-implicit-str-concat (prevent implicit string concat) + # 'ICN', # flake8-import-conventions (enforce import conventions) + # 'INP', # flake8-no-pep420 (prevent use of PEP420, i.e. implicit name spaces) + # 'G', # flake8-logging-format (best practices for logging) + # 'PIE', # flake8-pie (misc suggested improvement linting) + # 'T20', # flake8-print (prevent print statements in code) + # 'PT', # flake8-pytest-style (best practices for pytest) + # 'Q', # flake8-quotes (best practices for quotes) + # 'RSE', # flake8-raise (best practices for raising exceptions) + # 'RET', # flake8-return (best practices for return statements) + # # 'SLF', # flake8-self (prevent private member access) + # 'TID', # flake8-tidy-imports (prevent banned api and best import practices) + # 'INT', # flake8-gettext (when to use printf style strings) + # 'ARG', # flake8-unused-arguments (prevent unused arguments) + # 'PTH', # flake8-use-pathlib (prefer pathlib over os.path) + # 'ERA', # eradicate (remove commented out code) + # 'PGH', # pygrep (simple grep checks) + # 'PL', # pylint (general linting, flake8 alternative) + # 'FLY', # flynt (f-string conversion where possible) + # 'NPY', # NumPy-specific checks (recommendations from NumPy) + # 'PERF', # Perflint (performance linting) + # 'RUF', # ruff specific checks ] line-length = 88 extend-exclude = [ diff --git a/src/stpipe/config_parser.py b/src/stpipe/config_parser.py index 5af47794..b97c0866 100644 --- a/src/stpipe/config_parser.py +++ b/src/stpipe/config_parser.py @@ -70,9 +70,9 @@ def _output_file_check(path): path = os.path.join(root_dir, path) path = os.path.abspath(path) - dir = os.path.dirname(path) - if dir and not os.path.exists(dir): - os.makedirs(dir) + dir_ = os.path.dirname(path) + if dir_ and not os.path.exists(dir_): + os.makedirs(dir_) return path diff --git a/src/stpipe/format_template.py b/src/stpipe/format_template.py index 5391348a..c51f5c63 100644 --- a/src/stpipe/format_template.py +++ b/src/stpipe/format_template.py @@ -127,7 +127,7 @@ def __init__(self, separator="_", key_formats=None, remove_unused=False): if key_formats: self.key_formats.update(key_formats) - def format(self, format_string, **kwargs): + def format(self, format_string, **kwargs): # noqa: A003 """Perform the formatting Parameters diff --git a/src/stpipe/hooks.py b/src/stpipe/hooks.py index 0a08325e..2ab16fc1 100644 --- a/src/stpipe/hooks.py +++ b/src/stpipe/hooks.py @@ -8,7 +8,7 @@ from .step import Step -def hook_from_string(step, type, num, command): +def hook_from_string(step, type, num, command): # noqa: A002 name = f"{type}_hook{num:d}" step_class = None @@ -36,5 +36,5 @@ def hook_from_string(step, type, num, command): return SystemCall(name, parent=step, command=command) -def get_hook_objects(step, type, hooks): - return [hook_from_string(step, type, i, hook) for i, hook in enumerate(hooks)] +def get_hook_objects(step, type_, hooks): + return [hook_from_string(step, type_, i, hook) for i, hook in enumerate(hooks)] diff --git a/src/stpipe/log.py b/src/stpipe/log.py index a715e422..b9de4a2f 100644 --- a/src/stpipe/log.py +++ b/src/stpipe/log.py @@ -83,7 +83,7 @@ def __init__( handler=None, level=logging.NOTSET, break_level=logging.NOTSET, - format=None, + format=None, # noqa: A002 ): if name in ("", ".", "root"): name = "*" @@ -97,7 +97,7 @@ def __init__( self.level = level self.break_level = break_level if format is None: - format = DEFAULT_FORMAT + format = DEFAULT_FORMAT # noqa: A001 self.format = format def match(self, log_name): diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 8ee34ae5..60bd86e0 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -926,7 +926,7 @@ def save_model( idx=None, output_file=None, force=False, - format=None, + format=None, # noqa: A002 **components, ): """ @@ -1223,7 +1223,7 @@ def make_input_path(self, file_path): return full_path - def _set_input_dir(self, input, exclusive=True): + def _set_input_dir(self, input_, exclusive=True): """Set the input directory If sufficient information is at hand, set a value @@ -1231,7 +1231,7 @@ def _set_input_dir(self, input, exclusive=True): Parameters ---------- - input : str + input_ : str Input to determine path from. exclusive : bool @@ -1241,8 +1241,8 @@ def _set_input_dir(self, input, exclusive=True): """ if not exclusive or self.search_attr("_input_dir") is None: with suppress(Exception): - if isfile(input): - self.input_dir = split(input)[0] + if isfile(input_): + self.input_dir = split(input_)[0] def get_pars(self, full_spec=True): """Retrieve the configuration parameters of a step @@ -1334,7 +1334,7 @@ def update_pars(self, parameters): ) @classmethod - def build_config(cls, input, **kwargs): + def build_config(cls, input, **kwargs): # noqa: A002 """Build the ConfigObj to initialize a Step A Step config is built in the following order: diff --git a/src/stpipe/utilities.py b/src/stpipe/utilities.py index d267edc4..6e37b8dc 100644 --- a/src/stpipe/utilities.py +++ b/src/stpipe/utilities.py @@ -100,8 +100,8 @@ def get_spec_file_path(step_class): # Since `step_class` could be defined in a file called whatever, # we need the source file basedir and the class name. - dir = os.path.dirname(step_source_file) - return os.path.join(dir, step_class.__name__ + ".spec") + dir_ = os.path.dirname(step_source_file) + return os.path.join(dir_, step_class.__name__ + ".spec") def find_spec_file(step_class): From 9c9b6e145516c9c3f6279dd13022e4aba5b621c4 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 11:49:20 -0400 Subject: [PATCH 08/24] Add flake8 comprehension linting This will make comprehensions marginally faster --- pyproject.toml | 2 +- src/stpipe/datamodel.py | 6 +++--- tests/test_step.py | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index c4f5ccc4..a9be77a9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -89,7 +89,7 @@ select = [ # 'BLE', # flake8-blind-except 'B', # flake8-bugbear 'A', # flake8-builtins (prevent shadowing of builtins) - # 'C4', # flake8-comprehensions (best practices for comprehensions) + 'C4', # flake8-comprehensions (best practices for comprehensions) # 'T10', # flake8-debugger (prevent debugger statements in code) # 'ISC', # flake8-implicit-str-concat (prevent implicit string concat) # 'ICN', # flake8-import-conventions (enforce import conventions) diff --git a/src/stpipe/datamodel.py b/src/stpipe/datamodel.py index e6dee69c..06b7dd21 100644 --- a/src/stpipe/datamodel.py +++ b/src/stpipe/datamodel.py @@ -23,9 +23,9 @@ def __subclasshook__(cls, c_): if cls is AbstractDataModel: mro = c_.__mro__ if ( - any([hasattr(CC, "crds_observatory") for CC in mro]) - and any([hasattr(CC, "get_crds_parameters") for CC in mro]) - and any([hasattr(CC, "save") for CC in mro]) + any(hasattr(CC, "crds_observatory") for CC in mro) + and any(hasattr(CC, "get_crds_parameters") for CC in mro) + and any(hasattr(CC, "save") for CC in mro) ): return True return False diff --git a/tests/test_step.py b/tests/test_step.py index e9980190..a7b65088 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -393,7 +393,7 @@ def test_logcfg_routing(tmpdir): handler.close() with open(tmpdir / "myrun.log") as f: - fulltext = "\n".join([line for line in f]) + fulltext = "\n".join(list(f)) assert "called out a warning" in fulltext From a13f2352dac0dfe02e3f1b97f9f91b5c94d60893 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 11:50:17 -0400 Subject: [PATCH 09/24] Lint for debugger statements --- pyproject.toml | 2 +- src/stpipe/cmdline.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index a9be77a9..be2e410c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,7 +90,7 @@ select = [ 'B', # flake8-bugbear 'A', # flake8-builtins (prevent shadowing of builtins) 'C4', # flake8-comprehensions (best practices for comprehensions) - # 'T10', # flake8-debugger (prevent debugger statements in code) + 'T10', # flake8-debugger (prevent debugger statements in code) # 'ISC', # flake8-implicit-str-concat (prevent implicit string concat) # 'ICN', # flake8-import-conventions (enforce import conventions) # 'INP', # flake8-no-pep420 (prevent use of PEP420, i.e. implicit name spaces) diff --git a/src/stpipe/cmdline.py b/src/stpipe/cmdline.py index c05c9ce6..2625248e 100644 --- a/src/stpipe/cmdline.py +++ b/src/stpipe/cmdline.py @@ -398,7 +398,7 @@ def step_from_cmdline(args, cls=None): _print_important_message(f"ERROR RUNNING STEP {step_class.__name__!r}:", str(e)) if debug_on_exception: - import pdb + import pdb # noqa: T100 pdb.post_mortem() else: From b4c82fb39bc4720ffbb523375f3ff0db018b35c2 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:07:26 -0400 Subject: [PATCH 10/24] Add unnecessary pass linting --- pyproject.toml | 10 +++++----- src/stpipe/cli/command.py | 3 --- src/stpipe/cmdline.py | 2 -- src/stpipe/datamodel.py | 2 -- src/stpipe/exceptions.py | 2 -- src/stpipe/step.py | 2 -- src/stpipe/utilities.py | 2 -- 7 files changed, 5 insertions(+), 18 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index be2e410c..ec3211b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -91,11 +91,11 @@ select = [ 'A', # flake8-builtins (prevent shadowing of builtins) 'C4', # flake8-comprehensions (best practices for comprehensions) 'T10', # flake8-debugger (prevent debugger statements in code) - # 'ISC', # flake8-implicit-str-concat (prevent implicit string concat) - # 'ICN', # flake8-import-conventions (enforce import conventions) - # 'INP', # flake8-no-pep420 (prevent use of PEP420, i.e. implicit name spaces) + 'ISC', # flake8-implicit-str-concat (prevent implicit string concat) + 'ICN', # flake8-import-conventions (enforce import conventions) + 'INP', # flake8-no-pep420 (prevent use of PEP420, i.e. implicit name spaces) # 'G', # flake8-logging-format (best practices for logging) - # 'PIE', # flake8-pie (misc suggested improvement linting) + 'PIE', # flake8-pie (misc suggested improvement linting) # 'T20', # flake8-print (prevent print statements in code) # 'PT', # flake8-pytest-style (best practices for pytest) # 'Q', # flake8-quotes (best practices for quotes) @@ -125,7 +125,7 @@ extend-ignore = [ ] [tool.ruff.extend-per-file-ignores] -"tests/*.py" = ["S101", "S603", "S607"] +"tests/*.py" = ["S101", "S603", "S607", "INP001"] "src/stpipe/tests/*.py" = ["S101"] diff --git a/src/stpipe/cli/command.py b/src/stpipe/cli/command.py index e78b8477..47de9600 100644 --- a/src/stpipe/cli/command.py +++ b/src/stpipe/cli/command.py @@ -16,7 +16,6 @@ def get_name(cls): ------- str """ - pass @abc.abstractclassmethod def add_subparser(cls, subparsers): @@ -27,7 +26,6 @@ def add_subparser(cls, subparsers): ---------- subparsers : argparse._SubParsersAction """ - pass @abc.abstractclassmethod def run(cls, args): @@ -44,4 +42,3 @@ def run(cls, args): int Exit status code. """ - pass diff --git a/src/stpipe/cmdline.py b/src/stpipe/cmdline.py index 2625248e..8ece498e 100644 --- a/src/stpipe/cmdline.py +++ b/src/stpipe/cmdline.py @@ -132,8 +132,6 @@ class FromCommandLine(str): use isinstance to see where the values came from. """ - pass - def _override_config_from_args(config, args): """ diff --git a/src/stpipe/datamodel.py b/src/stpipe/datamodel.py index 06b7dd21..7ec90b04 100644 --- a/src/stpipe/datamodel.py +++ b/src/stpipe/datamodel.py @@ -34,7 +34,6 @@ def __subclasshook__(cls, c_): @abc.abstractmethod def crds_observatory(self): """This should return a string identifying the observatory as CRDS expects it""" - pass @abc.abstractmethod def get_crds_parameters(self): @@ -65,4 +64,3 @@ def save(self, path, dir_path=None, *args, **kwargs): output_path: str The file path the model was saved in. """ - pass diff --git a/src/stpipe/exceptions.py b/src/stpipe/exceptions.py index 7c49f43c..0ed4bda9 100644 --- a/src/stpipe/exceptions.py +++ b/src/stpipe/exceptions.py @@ -3,8 +3,6 @@ class StpipeException(Exception): # noqa: N818 Base class for exceptions from the stpipe package. """ - pass - class StpipeExitException(StpipeException): # noqa: N818 """ diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 60bd86e0..b5c996dd 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -561,7 +561,6 @@ def finalize_result(self, result, reference_files_used): List of reference files used when running the step, each a tuple in the form (str reference type, str reference URI). """ - pass @staticmethod def remove_suffix(name): @@ -722,7 +721,6 @@ def _precache_references(self, input_file): true precache operations and avoids having to override the more complex Step.run() instead. """ - pass def get_ref_override(self, reference_file_type): """Determine and return any override for `reference_file_type`. diff --git a/src/stpipe/utilities.py b/src/stpipe/utilities.py index 6e37b8dc..6863f128 100644 --- a/src/stpipe/utilities.py +++ b/src/stpipe/utilities.py @@ -144,7 +144,5 @@ class _NotSet: below """ - pass - _not_set = _NotSet() From 321d5c5bc926d7a23a0f3cf8cd05c546d4947422 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:25:45 -0400 Subject: [PATCH 11/24] Add pytest linting --- pyproject.toml | 6 ++-- tests/cli/test_list.py | 2 +- tests/cli/test_main.py | 2 +- tests/test_format_template.py | 2 +- tests/test_logger.py | 2 +- tests/test_step.py | 64 +++++++++++++++++++---------------- 6 files changed, 42 insertions(+), 36 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ec3211b0..74f9d973 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,8 +96,8 @@ select = [ 'INP', # flake8-no-pep420 (prevent use of PEP420, i.e. implicit name spaces) # 'G', # flake8-logging-format (best practices for logging) 'PIE', # flake8-pie (misc suggested improvement linting) - # 'T20', # flake8-print (prevent print statements in code) - # 'PT', # flake8-pytest-style (best practices for pytest) + 'T20', # flake8-print (prevent print statements in code) + 'PT', # flake8-pytest-style (best practices for pytest) # 'Q', # flake8-quotes (best practices for quotes) # 'RSE', # flake8-raise (best practices for raising exceptions) # 'RET', # flake8-return (best practices for return statements) @@ -127,6 +127,8 @@ extend-ignore = [ [tool.ruff.extend-per-file-ignores] "tests/*.py" = ["S101", "S603", "S607", "INP001"] "src/stpipe/tests/*.py" = ["S101"] +"src/stpipe/cli/*.py" = ["T201"] +"src/stpipe/cmdline.py" = ["T201"] [tool.black] diff --git a/tests/cli/test_list.py b/tests/cli/test_list.py index 64df6f06..cb977e81 100644 --- a/tests/cli/test_list.py +++ b/tests/cli/test_list.py @@ -5,7 +5,7 @@ @pytest.fixture(autouse=True) -def monkey_patch_get_steps(monkeypatch): +def _monkey_patch_get_steps(monkeypatch): def _get_steps(): return [ StepInfo( diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index cb1324e3..f659e494 100644 --- a/tests/cli/test_main.py +++ b/tests/cli/test_main.py @@ -8,7 +8,7 @@ @pytest.fixture(autouse=True) -def monkey_patch_get_steps(monkeypatch): +def _monkey_patch_get_steps(monkeypatch): def _get_steps(): return [ StepInfo( diff --git a/tests/test_format_template.py b/tests/test_format_template.py index 9e93f6f0..f737652f 100644 --- a/tests/test_format_template.py +++ b/tests/test_format_template.py @@ -6,7 +6,7 @@ @pytest.mark.parametrize( - "key_formats, template, expected, fields, errors", + ("key_formats", "template", "expected", "fields", "errors"), [ # No replacement at all. ( diff --git a/tests/test_logger.py b/tests/test_logger.py index fde9d57d..b28918ae 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -6,7 +6,7 @@ @pytest.fixture(autouse=True) -def clean_up_logging(): +def _clean_up_logging(): yield logging.shutdown() stpipe_log.load_configuration(io.BytesIO(stpipe_log.DEFAULT_CONFIGURATION)) diff --git a/tests/test_step.py b/tests/test_step.py index a7b65088..9eff34e1 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -1,5 +1,6 @@ """Test step.Step""" import logging +import re import asdf import pytest @@ -137,8 +138,8 @@ def config_file_list_arg_step(tmpdir): return config_file -@pytest.fixture -def mock_step_crds(monkeypatch): +@pytest.fixture() +def _mock_step_crds(monkeypatch): """Mock various crds calls from Step""" def mock_get_config_from_reference_pipe(dataset, disable=None): @@ -184,7 +185,8 @@ def mock_get_config_from_reference_list_arg_step(dataset, disable=None): # ##### # Tests # ##### -def test_build_config_pipe_config_file(mock_step_crds, config_file_pipe): +@pytest.mark.usefixtures("_mock_step_crds") +def test_build_config_pipe_config_file(config_file_pipe): """Test that local config overrides defaults and CRDS-supplied file""" config, returned_config_file = SimplePipe.build_config( "science.fits", config_file=config_file_pipe @@ -198,7 +200,8 @@ def test_build_config_pipe_config_file(mock_step_crds, config_file_pipe): assert config["steps"]["step1"]["str3"] == "from crds" -def test_build_config_pipe_crds(mock_step_crds): +@pytest.mark.usefixtures("_mock_step_crds") +def test_build_config_pipe_crds(): """Test that CRDS param reffile overrides a default CRDS configuration""" config, config_file = SimplePipe.build_config("science.fits") assert not config_file @@ -217,7 +220,8 @@ def test_build_config_pipe_default(): assert len(config) == 0 -def test_build_config_pipe_kwarg(mock_step_crds, config_file_pipe): +@pytest.mark.usefixtures("_mock_step_crds") +def test_build_config_pipe_kwarg(config_file_pipe): """Test that kwargs override CRDS and local param reffiles""" config, returned_config_file = SimplePipe.build_config( "science.fits", @@ -234,7 +238,8 @@ def test_build_config_pipe_kwarg(mock_step_crds, config_file_pipe): assert config["steps"]["step1"]["str3"] == "from crds" -def test_build_config_step_config_file(mock_step_crds, config_file_step): +@pytest.mark.usefixtures("_mock_step_crds") +def test_build_config_step_config_file(config_file_step): """Test that local config overrides defaults and CRDS-supplied file""" config, returned_config_file = SimpleStep.build_config( "science.fits", config_file=config_file_step @@ -245,7 +250,8 @@ def test_build_config_step_config_file(mock_step_crds, config_file_step): assert config["str3"] == "from crds" -def test_build_config_step_crds(mock_step_crds): +@pytest.mark.usefixtures("_mock_step_crds") +def test_build_config_step_crds(): """Test override of a CRDS configuration""" config, config_file = SimpleStep.build_config("science.fits") assert config_file is None @@ -262,7 +268,8 @@ def test_build_config_step_default(): assert len(config) == 0 -def test_build_config_step_kwarg(mock_step_crds, config_file_step): +@pytest.mark.usefixtures("_mock_step_crds") +def test_build_config_step_kwarg(config_file_step): """Test that kwargs override everything""" config, returned_config_file = SimpleStep.build_config( "science.fits", config_file=config_file_step, str1="from kwarg" @@ -273,7 +280,8 @@ def test_build_config_step_kwarg(mock_step_crds, config_file_step): assert config["str3"] == "from crds" -def test_step_list_args(mock_step_crds, config_file_list_arg_step): +@pytest.mark.usefixtures("_mock_step_crds") +def test_step_list_args(config_file_list_arg_step): """Test that list arguments, provided as comma-separated values are parsed correctly. """ @@ -300,7 +308,11 @@ def test_step_list_args(mock_step_crds, config_file_list_arg_step): assert c.output_shape == [1500, 1300] assert c.crpix == [123, 456] - with pytest.raises(ValueError) as e: + msg = re.escape( + "Config parameter 'output_shape': the value \"['1500', '1300', '90']\" " + "is too long." + ) + with pytest.raises(ValueError, match=msg): cmdline.just_the_step_from_cmdline( [ "filename.fits", @@ -313,13 +325,11 @@ def test_step_list_args(mock_step_crds, config_file_list_arg_step): ], ListArgStep, ) - assert ( - e.value.args[0] - == "Config parameter 'output_shape': the value \"['1500', '1300', '90']\" is" - " too long." - ) - with pytest.raises(ValueError) as e: + msg = re.escape( + "Config parameter 'output_shape': the value \"['1500']\" is too short." + ) + with pytest.raises(ValueError, match=msg): cmdline.just_the_step_from_cmdline( [ "filename.fits", @@ -332,12 +342,11 @@ def test_step_list_args(mock_step_crds, config_file_list_arg_step): ], ListArgStep, ) - assert ( - e.value.args[0] - == "Config parameter 'output_shape': the value \"['1500']\" is too short." - ) - with pytest.raises(ValueError) as e: + msg = re.escape( + "Config parameter 'output_shape': the value \"1500\" is of the wrong type." + ) + with pytest.raises(ValueError, match=msg): cmdline.just_the_step_from_cmdline( [ "filename.fits", @@ -350,12 +359,11 @@ def test_step_list_args(mock_step_crds, config_file_list_arg_step): ], ListArgStep, ) - assert ( - e.value.args[0] - == "Config parameter 'output_shape': the value \"1500\" is of the wrong type." - ) - with pytest.raises(ValueError) as e: + msg = re.escape( + "Config parameter 'output_shape': the value \"1500.5\" is of the wrong type." + ) + with pytest.raises(ValueError, match=msg): cmdline.just_the_step_from_cmdline( [ "filename.fits", @@ -368,10 +376,6 @@ def test_step_list_args(mock_step_crds, config_file_list_arg_step): ], ListArgStep, ) - assert ( - e.value.args[0] - == "Config parameter 'output_shape': the value \"1500.5\" is of the wrong type." - ) def test_logcfg_routing(tmpdir): From c8ff9c09914c0e9dfb9cba8c10d118c9b68dcb9e Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:30:49 -0400 Subject: [PATCH 12/24] Simplify if/else returns --- pyproject.toml | 6 ++--- src/stpipe/config_parser.py | 22 +++++++++------- src/stpipe/crds_client.py | 6 ++--- src/stpipe/format_template.py | 4 +-- src/stpipe/log.py | 20 +++++++-------- src/stpipe/step.py | 47 +++++++++++++++++------------------ src/stpipe/utilities.py | 7 +++--- tests/test_step.py | 9 +++---- 8 files changed, 59 insertions(+), 62 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 74f9d973..f70b2366 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,9 +98,9 @@ select = [ 'PIE', # flake8-pie (misc suggested improvement linting) 'T20', # flake8-print (prevent print statements in code) 'PT', # flake8-pytest-style (best practices for pytest) - # 'Q', # flake8-quotes (best practices for quotes) - # 'RSE', # flake8-raise (best practices for raising exceptions) - # 'RET', # flake8-return (best practices for return statements) + 'Q', # flake8-quotes (best practices for quotes) + 'RSE', # flake8-raise (best practices for raising exceptions) + 'RET', # flake8-return (best practices for return statements) # # 'SLF', # flake8-self (prevent private member access) # 'TID', # flake8-tidy-imports (prevent banned api and best import practices) # 'INT', # flake8-gettext (when to use printf style strings) diff --git a/src/stpipe/config_parser.py b/src/stpipe/config_parser.py index b97c0866..d7f7b259 100644 --- a/src/stpipe/config_parser.py +++ b/src/stpipe/config_parser.py @@ -83,8 +83,8 @@ def _is_datamodel(value, default=None): """Verify that value is either is a DataModel.""" if isinstance(value, AbstractDataModel): return value - else: - raise VdtTypeError(value) + + raise VdtTypeError(value) def _is_string_or_datamodel(value, default=None): @@ -93,10 +93,11 @@ def _is_string_or_datamodel(value, default=None): """ if isinstance(value, AbstractDataModel): return value - elif isinstance(value, str): + + if isinstance(value, str): return value - else: - raise VdtTypeError(value) + + raise VdtTypeError(value) def load_config_file(config_file): @@ -211,7 +212,7 @@ def load_spec_file(cls, preserve_comments=_not_set): raise_errors=True, list_values=False, ) - return + return None def merge_config(into, new): @@ -359,8 +360,8 @@ def validate( if allow_missing: config[key] = spec[key] continue - else: - err = "missing" + + err = "missing" messages.append(f"Config parameter {section_string!r}: {err}") @@ -401,12 +402,15 @@ def _parse(val): """ if val.lower() == "true": return True - elif val.lower() == "false": + + if val.lower() == "false": return False + try: return int(val) except ValueError: pass + try: return float(val) except ValueError: diff --git a/src/stpipe/crds_client.py b/src/stpipe/crds_client.py index 24b44ad8..2e48b517 100644 --- a/src/stpipe/crds_client.py +++ b/src/stpipe/crds_client.py @@ -49,8 +49,7 @@ def get_multiple_reference_paths(parameters, reference_file_types, observatory): raise TypeError("First argument must be a dict of parameters") log.set_log_time(True) - refpaths = _get_refpaths(parameters, tuple(reference_file_types), observatory) - return refpaths + return _get_refpaths(parameters, tuple(reference_file_types), observatory) def _get_refpaths(data_dict, reference_file_types, observatory): @@ -67,11 +66,10 @@ def _get_refpaths(data_dict, reference_file_types, observatory): reftypes=reference_file_types, observatory=observatory, ) - refpaths = { + return { filetype: filepath if "N/A" not in filepath.upper() else "N/A" for (filetype, filepath) in bestrefs.items() } - return refpaths def check_reference_open(refpath): diff --git a/src/stpipe/format_template.py b/src/stpipe/format_template.py index c51f5c63..f8d57bbb 100644 --- a/src/stpipe/format_template.py +++ b/src/stpipe/format_template.py @@ -178,9 +178,7 @@ def format(self, format_string, **kwargs): # noqa: A003 if formatted_kwargs[unused] is not None ] result_parts = [result] + unused_values - result = self.separator.join(result_parts) - - return result + return self.separator.join(result_parts) # Make the instance callable __call__ = format diff --git a/src/stpipe/log.py b/src/stpipe/log.py index b9de4a2f..5d0a9fc5 100644 --- a/src/stpipe/log.py +++ b/src/stpipe/log.py @@ -117,14 +117,17 @@ def get_handler(self, handler_str): """ if handler_str.startswith("file:"): return logging.FileHandler(handler_str[5:], "w", "utf-8", True) - elif handler_str.startswith("append:"): + + if handler_str.startswith("append:"): return logging.FileHandler(handler_str[7:], "a", "utf-8", True) - elif handler_str == "stdout": + + if handler_str == "stdout": return logging.StreamHandler(sys.stdout) - elif handler_str == "stderr": + + if handler_str == "stderr": return logging.StreamHandler(sys.stderr) - else: - raise ValueError(f"Can't parse handler {handler_str!r}") + + raise ValueError(f"Can't parse handler {handler_str!r}") def apply(self, log): """ @@ -205,9 +208,7 @@ def _level_check(value): def getLogger(name=None): # noqa: N802 - log = logging.getLogger(name) - - return log + return logging.getLogger(name) def _find_logging_config_file(): @@ -218,8 +219,7 @@ def _find_logging_config_file(): if os.path.exists(file): return os.path.abspath(file) - buffer = io.BytesIO(DEFAULT_CONFIGURATION) - return buffer + return io.BytesIO(DEFAULT_CONFIGURATION) ########################################################################### diff --git a/src/stpipe/step.py b/src/stpipe/step.py index b5c996dd..8cacc284 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -285,7 +285,7 @@ def from_config_section( else: kwargs[k] = config[k] - step = cls( + return cls( name=name, parent=parent, config_file=config_file, @@ -293,8 +293,6 @@ def from_config_section( **kwargs, ) - return step - def __init__( self, name=None, @@ -702,16 +700,16 @@ def search_attr(self, attribute, default=None, parent_first=False): if value is None: value = getattr(self, attribute, default) return value - else: - value = getattr(self, attribute, None) - if value is None: - try: - value = self.parent.search_attr(attribute) - except AttributeError: - pass - if value is None: - value = default - return value + + value = getattr(self, attribute, None) + if value is None: + try: + value = self.parent.search_attr(attribute) + except AttributeError: + pass + if value is None: + value = default + return value def _precache_references(self, input_file): """Because Step precaching precedes calls to get_reference_file() almost @@ -733,8 +731,8 @@ def get_ref_override(self, reference_file_type): path = getattr(self, override_name, None) if isinstance(path, AbstractDataModel): return path - else: - return abspath(path) if path and path != "N/A" else path + + return abspath(path) if path and path != "N/A" else path def get_reference_file(self, input_file, reference_file_type): """ @@ -766,7 +764,8 @@ def get_reference_file(self, input_file, reference_file_type): (reference_file_type, override.override_handle) ) return override - elif override.strip() != "": + + if override.strip() != "": self._reference_files_used.append( (reference_file_type, basename(override)) ) @@ -869,9 +868,9 @@ def get_config_from_reference(cls, dataset, disable=None, crds_observatory=None) ) return ref - else: - logger.debug(f"No {reftype.upper()} reference files found.") - return config_parser.ConfigObj() + + logger.debug(f"No {reftype.upper()} reference files found.") + return config_parser.ConfigObj() @classmethod def reference_uri_to_cache_path(cls, reference_uri, observatory): @@ -969,7 +968,7 @@ def save_model( # Check if saving is even specified. if not force and not self.save_results and not output_file: - return + return None if isinstance(model, Sequence): save_model_func = partial( @@ -1123,9 +1122,7 @@ def _make_output_path( output_dir = step.search_attr("output_dir", default="") output_dir = expandvars(expanduser(output_dir)) - full_output_path = join(output_dir, basename) - - return full_output_path + return join(output_dir, basename) def closeout(self, to_close=None, to_del=None): """Close out step processing @@ -1459,8 +1456,10 @@ def get_disable_crds_steppars(default=None): if default: if isinstance(default, bool): return default - elif isinstance(default, str): + + if isinstance(default, str): return default in truths + raise ValueError(f"default must be string or boolean: {default}") flag = os.environ.get("STPIPE_DISABLE_CRDS_STEPPARS", "") diff --git a/src/stpipe/utilities.py b/src/stpipe/utilities.py index 6863f128..b28dc21d 100644 --- a/src/stpipe/utilities.py +++ b/src/stpipe/utilities.py @@ -73,7 +73,8 @@ def import_class(full_name, subclassof=object, config_file=None): raise TypeError( f"Object {class_name} from package {package_name} is not a class" ) - elif not issubclass(step_class, subclassof): + + if not issubclass(step_class, subclassof): raise TypeError( f"Class {class_name} from package {package_name} is not a subclass of" f" {subclassof.__name__}" @@ -133,8 +134,8 @@ def get_fully_qualified_class_name(cls_or_obj): module = cls.__module__ if module is None or module == str.__class__.__module__: return cls.__name__ # Avoid reporting __builtin__ - else: - return module + "." + cls.__name__ + + return module + "." + cls.__name__ class _NotSet: diff --git a/tests/test_step.py b/tests/test_step.py index 9eff34e1..08c23a1c 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -143,7 +143,7 @@ def _mock_step_crds(monkeypatch): """Mock various crds calls from Step""" def mock_get_config_from_reference_pipe(dataset, disable=None): - config = cp.config_from_dict( + return cp.config_from_dict( { "str1": "from crds", "str2": "from crds", @@ -157,17 +157,14 @@ def mock_get_config_from_reference_pipe(dataset, disable=None): }, } ) - return config def mock_get_config_from_reference_step(dataset, disable=None): - config = cp.config_from_dict( + return cp.config_from_dict( {"str1": "from crds", "str2": "from crds", "str3": "from crds"} ) - return config def mock_get_config_from_reference_list_arg_step(dataset, disable=None): - config = cp.config_from_dict({"rotation": "15", "pixel_scale": "0.85"}) - return config + return cp.config_from_dict({"rotation": "15", "pixel_scale": "0.85"}) monkeypatch.setattr( SimplePipe, "get_config_from_reference", mock_get_config_from_reference_pipe From d291c495b4889ab1127ac74adf9d36314db059f6 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:33:25 -0400 Subject: [PATCH 13/24] Add import linting --- pyproject.toml | 4 ++-- src/stpipe/cli/list.py | 3 ++- src/stpipe/cli/main.py | 6 +++--- src/stpipe/subproc.py | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index f70b2366..ac78f51a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -101,8 +101,8 @@ select = [ 'Q', # flake8-quotes (best practices for quotes) 'RSE', # flake8-raise (best practices for raising exceptions) 'RET', # flake8-return (best practices for return statements) - # # 'SLF', # flake8-self (prevent private member access) - # 'TID', # flake8-tidy-imports (prevent banned api and best import practices) + # 'SLF', # flake8-self (prevent private member access) + 'TID', # flake8-tidy-imports (prevent banned api and best import practices) # 'INT', # flake8-gettext (when to use printf style strings) # 'ARG', # flake8-unused-arguments (prevent unused arguments) # 'PTH', # flake8-use-pathlib (prefer pathlib over os.path) diff --git a/src/stpipe/cli/list.py b/src/stpipe/cli/list.py index 0b229ad7..43616c60 100644 --- a/src/stpipe/cli/list.py +++ b/src/stpipe/cli/list.py @@ -6,7 +6,8 @@ import re import sys -from .. import entry_points +from stpipe import entry_points + from .command import Command diff --git a/src/stpipe/cli/main.py b/src/stpipe/cli/main.py index cccf1b37..290503e0 100644 --- a/src/stpipe/cli/main.py +++ b/src/stpipe/cli/main.py @@ -5,7 +5,8 @@ import sys import traceback -from ..exceptions import StpipeExitException +from stpipe.exceptions import StpipeExitException + from .list import ListCommand # New subclasses of Command must be imported @@ -90,8 +91,7 @@ def _print_versions(): that register an stpipe.steps entry point. """ import stpipe - - from .. import entry_points + from stpipe import entry_points packages = sorted( {(s.package_name, s.package_version) for s in entry_points.get_steps()}, diff --git a/src/stpipe/subproc.py b/src/stpipe/subproc.py index fb83e861..487f2a98 100644 --- a/src/stpipe/subproc.py +++ b/src/stpipe/subproc.py @@ -29,7 +29,7 @@ class SystemCall(Step): """ # noqa: E501 def process(self, *args): - from .. import datamodels + from .. import datamodels # noqa: TID252 newargs = [] for i, arg in enumerate(args): From 84dc153b55901922b013d015a9221379a66a9f84 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:39:18 -0400 Subject: [PATCH 14/24] Add unused argument checks --- pyproject.toml | 6 +++--- src/stpipe/config_parser.py | 4 ++-- src/stpipe/format_template.py | 2 +- src/stpipe/step.py | 6 ++++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ac78f51a..08a730b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -103,8 +103,8 @@ select = [ 'RET', # flake8-return (best practices for return statements) # 'SLF', # flake8-self (prevent private member access) 'TID', # flake8-tidy-imports (prevent banned api and best import practices) - # 'INT', # flake8-gettext (when to use printf style strings) - # 'ARG', # flake8-unused-arguments (prevent unused arguments) + 'INT', # flake8-gettext (when to use printf style strings) + 'ARG', # flake8-unused-arguments (prevent unused arguments) # 'PTH', # flake8-use-pathlib (prefer pathlib over os.path) # 'ERA', # eradicate (remove commented out code) # 'PGH', # pygrep (simple grep checks) @@ -125,7 +125,7 @@ extend-ignore = [ ] [tool.ruff.extend-per-file-ignores] -"tests/*.py" = ["S101", "S603", "S607", "INP001"] +"tests/*.py" = ["S101", "S603", "S607", "INP001", "ARG001"] "src/stpipe/tests/*.py" = ["S101"] "src/stpipe/cli/*.py" = ["T201"] "src/stpipe/cmdline.py" = ["T201"] diff --git a/src/stpipe/config_parser.py b/src/stpipe/config_parser.py index d7f7b259..1c71d96e 100644 --- a/src/stpipe/config_parser.py +++ b/src/stpipe/config_parser.py @@ -79,7 +79,7 @@ def _output_file_check(path): return _output_file_check -def _is_datamodel(value, default=None): +def _is_datamodel(value): """Verify that value is either is a DataModel.""" if isinstance(value, AbstractDataModel): return value @@ -87,7 +87,7 @@ def _is_datamodel(value, default=None): raise VdtTypeError(value) -def _is_string_or_datamodel(value, default=None): +def _is_string_or_datamodel(value): """Verify that value is either a string (nominally a reference file path) or a DataModel (possibly one with no corresponding file.) """ diff --git a/src/stpipe/format_template.py b/src/stpipe/format_template.py index f8d57bbb..fd486fa9 100644 --- a/src/stpipe/format_template.py +++ b/src/stpipe/format_template.py @@ -183,7 +183,7 @@ def format(self, format_string, **kwargs): # noqa: A003 # Make the instance callable __call__ = format - def get_value(self, key, args, kwargs): + def get_value(self, key, args, kwargs): # noqa: ARG002 """Return a given field value Parameters diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 8cacc284..e49ac967 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -87,7 +87,7 @@ def get_config_reftype(cls): return f"pars-{cls.__name__.lower()}" @classmethod - def merge_config(cls, config, config_file): + def merge_config(cls, config, config_file): # noqa: ARG003 return config @classmethod @@ -187,7 +187,9 @@ def from_cmdline(args): return cmdline.step_from_cmdline(args) @classmethod - def _parse_class_and_name(cls, config, parent=None, name=None, config_file=None): + def _parse_class_and_name( + cls, config, parent=None, name=None, config_file=None # noqa: ARG003 + ): if "class" in config: step_class = utilities.import_class( utilities.resolve_step_class_alias(config["class"]), From f3640f021e08e43261d88601833a41c8e4c95408 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:41:54 -0400 Subject: [PATCH 15/24] Add removal of commented out code --- .pre-commit-config.yaml | 2 -- pyproject.toml | 4 ++-- src/stpipe/config_parser.py | 1 - src/stpipe/utilities.py | 2 +- tests/test_step.py | 1 - 5 files changed, 3 insertions(+), 7 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ab0e440a..395a8648 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -20,8 +20,6 @@ repos: - repo: https://github.com/pre-commit/pygrep-hooks rev: v1.10.0 hooks: - - id: python-check-blanket-noqa - - id: python-check-mock-methods - id: rst-directive-colons - id: rst-inline-touching-normal - id: text-unicode-replacement-char diff --git a/pyproject.toml b/pyproject.toml index 08a730b0..1d451dca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -106,8 +106,8 @@ select = [ 'INT', # flake8-gettext (when to use printf style strings) 'ARG', # flake8-unused-arguments (prevent unused arguments) # 'PTH', # flake8-use-pathlib (prefer pathlib over os.path) - # 'ERA', # eradicate (remove commented out code) - # 'PGH', # pygrep (simple grep checks) + 'ERA', # eradicate (remove commented out code) + 'PGH', # pygrep (simple grep checks) # 'PL', # pylint (general linting, flake8 alternative) # 'FLY', # flynt (f-string conversion where possible) # 'NPY', # NumPy-specific checks (recommendations from NumPy) diff --git a/src/stpipe/config_parser.py b/src/stpipe/config_parser.py index 1c71d96e..7981896a 100644 --- a/src/stpipe/config_parser.py +++ b/src/stpipe/config_parser.py @@ -393,7 +393,6 @@ def string_to_python_type(section, key): else: typed_val = _parse(val) section[key] = typed_val - return def _parse(val): diff --git a/src/stpipe/utilities.py b/src/stpipe/utilities.py index b28dc21d..cf615a1a 100644 --- a/src/stpipe/utilities.py +++ b/src/stpipe/utilities.py @@ -45,7 +45,7 @@ def import_class(full_name, subclassof=object, config_file=None): # package.subPackage.subsubpackage.className # in the input parameter `full_name`. This means that # 1. We HAVE to be able to say - # from package.subPackage.subsubpackage import className + # `from package.subPackage.subsubpackage import className` # 2. If `subclassof` is defined, the newly imported Python class MUST be a # subclass of `subclassof`, which HAS to be a Python class. diff --git a/tests/test_step.py b/tests/test_step.py index 08c23a1c..baf3f4c3 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -53,7 +53,6 @@ def process(self): self.log.warning("This step has called out a warning.") self.log.warning(f"{self.log} {self.log.handlers}") - return def _datamodels_open(self, **kwargs): pass From aef6ccf65a9ae2d7909508e3e37326a549a7211e Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:42:44 -0400 Subject: [PATCH 16/24] Apply flint using ruff --- .pre-commit-config.yaml | 6 ------ pyproject.toml | 2 +- src/stpipe/step.py | 4 ++-- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 395a8648..01ae1096 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,12 +32,6 @@ repos: additional_dependencies: - tomli -- repo: https://github.com/ikamensh/flynt/ - rev: '1.0.1' - hooks: - - id: flynt - exclude: "src/stpipe/extern/.*" - - repo: https://github.com/astral-sh/ruff-pre-commit rev: 'v0.1.4' hooks: diff --git a/pyproject.toml b/pyproject.toml index 1d451dca..120033d6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -109,7 +109,7 @@ select = [ 'ERA', # eradicate (remove commented out code) 'PGH', # pygrep (simple grep checks) # 'PL', # pylint (general linting, flake8 alternative) - # 'FLY', # flynt (f-string conversion where possible) + 'FLY', # flynt (f-string conversion where possible) # 'NPY', # NumPy-specific checks (recommendations from NumPy) # 'PERF', # Perflint (performance linting) # 'RUF', # ruff specific checks diff --git a/src/stpipe/step.py b/src/stpipe/step.py index e49ac967..b2873222 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -345,9 +345,9 @@ def __init__( name = self.__class__.__name__ self.name = name if parent is None: - self.qualified_name = ".".join([log.STPIPE_ROOT_LOGGER, self.name]) + self.qualified_name = f"{log.STPIPE_ROOT_LOGGER}.{self.name}" else: - self.qualified_name = ".".join([parent.qualified_name, self.name]) + self.qualified_name = f"{parent.qualified_name}.{self.name}" self.parent = parent # Set the parameters as member variables From cd5baebdc397f7e19cff3dde58307aef5cc43aac Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 12:46:04 -0400 Subject: [PATCH 17/24] Add performance linting --- pyproject.toml | 4 ++-- src/stpipe/entry_points.py | 6 ++++-- src/stpipe/pipeline.py | 2 +- src/stpipe/step.py | 6 +++--- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 120033d6..966850d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -110,8 +110,8 @@ select = [ 'PGH', # pygrep (simple grep checks) # 'PL', # pylint (general linting, flake8 alternative) 'FLY', # flynt (f-string conversion where possible) - # 'NPY', # NumPy-specific checks (recommendations from NumPy) - # 'PERF', # Perflint (performance linting) + 'NPY', # NumPy-specific checks (recommendations from NumPy) + 'PERF', # Perflint (performance linting) # 'RUF', # ruff specific checks ] line-length = 88 diff --git a/src/stpipe/entry_points.py b/src/stpipe/entry_points.py index 02bfb261..133fa6ac 100644 --- a/src/stpipe/entry_points.py +++ b/src/stpipe/entry_points.py @@ -33,9 +33,11 @@ class alias, and the third is a bool indicating whether the class is to be try: elements = entry_point.load()() + package_steps = [ + StepInfo(*element, package_name, package_version) + for element in elements + ] - for element in elements: - package_steps.append(StepInfo(*element, package_name, package_version)) except Exception as e: warnings.warn( f"{STEPS_GROUP} plugin from package {package_name}=={package_version} " diff --git a/src/stpipe/pipeline.py b/src/stpipe/pipeline.py index c69421d9..41a04c64 100644 --- a/src/stpipe/pipeline.py +++ b/src/stpipe/pipeline.py @@ -344,7 +344,7 @@ def get_pars(self, full_spec=True): """ pars = super().get_pars(full_spec=full_spec) pars["steps"] = {} - for step_name, _step_class in self.step_defs.items(): + for step_name in self.step_defs.keys(): pars["steps"][step_name] = getattr(self, step_name).get_pars( full_spec=full_spec ) diff --git a/src/stpipe/step.py b/src/stpipe/step.py index b2873222..c5e75868 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -456,7 +456,7 @@ def run(self, *args): model[ f"meta.cal_step.{self.class_alias}" ] = "SKIPPED" - except AttributeError as e: + except AttributeError as e: # noqa: PERF203 self.log.info( "Could not record skip into DataModel" f" header: {e}" @@ -1152,12 +1152,12 @@ def closeout(self, to_close=None, to_del=None): try: if hasattr(item, "close"): item.close() - except Exception as exception: + except Exception as exception: # noqa: PERF203 self.log.debug(f'Could not close "{item}"Reason:\n{exception}') for item in to_del: try: del item - except NameError as error: + except NameError as error: # noqa: PERF203 self.log.debug("An error has occurred: %s", error) gc.collect() From 7f239a15536007ae03d853cf37bd4cdd72d8c542 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 29 Aug 2023 15:42:52 -0400 Subject: [PATCH 18/24] Add ruff specific rules --- pyproject.toml | 2 +- src/stpipe/cmdline.py | 6 +++--- src/stpipe/exceptions.py | 2 +- src/stpipe/format_template.py | 4 ++-- src/stpipe/pipeline.py | 3 ++- src/stpipe/step.py | 3 ++- tests/test_step.py | 3 ++- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 966850d5..3c9d9836 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -112,7 +112,7 @@ select = [ 'FLY', # flynt (f-string conversion where possible) 'NPY', # NumPy-specific checks (recommendations from NumPy) 'PERF', # Perflint (performance linting) - # 'RUF', # ruff specific checks + 'RUF', # ruff specific checks ] line-length = 88 extend-exclude = [ diff --git a/src/stpipe/cmdline.py b/src/stpipe/cmdline.py index 8ece498e..ce90af4c 100644 --- a/src/stpipe/cmdline.py +++ b/src/stpipe/cmdline.py @@ -87,7 +87,7 @@ def build_from_spec(subspec, parts=None): parts = [] for key, val in subspec.items(): if isinstance(val, dict): - build_from_spec(val, parts + [key]) + build_from_spec(val, [*parts, key]) else: comment = subspec.inline_comments.get(key) or "" comment = comment.lstrip("#").strip() @@ -97,14 +97,14 @@ def build_from_spec(subspec, parts=None): help_string = comment else: help_string = f"{comment} [{default_value_string}]" - argument = "--" + ".".join(parts + [key]) + argument = "--" + ".".join([*parts, key]) if argument[2:] in built_in_configuration_parameters: raise ValueError( "The Step's spec is trying to override a built-in parameter" f" {argument!r}" ) parser.add_argument( - "--" + ".".join(parts + [key]), + "--" + ".".join([*parts, key]), type=str, help=help_string, metavar="", diff --git a/src/stpipe/exceptions.py b/src/stpipe/exceptions.py index 0ed4bda9..fe342c83 100644 --- a/src/stpipe/exceptions.py +++ b/src/stpipe/exceptions.py @@ -4,7 +4,7 @@ class StpipeException(Exception): # noqa: N818 """ -class StpipeExitException(StpipeException): # noqa: N818 +class StpipeExitException(StpipeException): """ An exception that carries an exit status that is returned by stpipe CLI tools. diff --git a/src/stpipe/format_template.py b/src/stpipe/format_template.py index fd486fa9..9dac089f 100644 --- a/src/stpipe/format_template.py +++ b/src/stpipe/format_template.py @@ -154,7 +154,7 @@ def format(self, format_string, **kwargs): # noqa: A003 # 0: The first replacement field. There should only be one. # 2: Get the format spec. # -1: Get the last character representing the type. - format_type = list(self.parse(key_format))[0][2][-1] + format_type = next(iter(self.parse(key_format)))[2][-1] try: value = key_format.format(CONVERSION[format_type](value)) @@ -177,7 +177,7 @@ def format(self, format_string, **kwargs): # noqa: A003 for unused in unused_keys if formatted_kwargs[unused] is not None ] - result_parts = [result] + unused_values + result_parts = [result, *unused_values] return self.separator.join(result_parts) # Make the instance callable diff --git a/src/stpipe/pipeline.py b/src/stpipe/pipeline.py index 41a04c64..bc53ce43 100644 --- a/src/stpipe/pipeline.py +++ b/src/stpipe/pipeline.py @@ -3,6 +3,7 @@ """ from collections.abc import Sequence from os.path import dirname, join +from typing import ClassVar from . import config_parser, crds_client, log from .extern.configobj.configobj import ConfigObj, Section @@ -24,7 +25,7 @@ class Pipeline(Step): """ # A set of steps used in the Pipeline. Should be overridden by # the subclass. - step_defs = {} + step_defs: ClassVar = {} def __init__(self, *args, **kwargs): """ diff --git a/src/stpipe/step.py b/src/stpipe/step.py index c5e75868..eb73cb80 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -18,6 +18,7 @@ split, splitext, ) +from typing import ClassVar try: from astropy.io import fits @@ -69,7 +70,7 @@ class Step: # Reference types for both command line override # definition and reference prefetch - reference_file_types = [] + reference_file_types: ClassVar = [] # Set to False in subclasses to skip prefetch, # but by default attempt to prefetch diff --git a/tests/test_step.py b/tests/test_step.py index baf3f4c3..8d660ca1 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -1,6 +1,7 @@ """Test step.Step""" import logging import re +from typing import ClassVar import asdf import pytest @@ -36,7 +37,7 @@ class SimplePipe(Pipeline): output_ext = string(default='simplestep') """ - step_defs = {"step1": SimpleStep} + step_defs: ClassVar = {"step1": SimpleStep} class LoggingPipeline(Pipeline): From b0c42569d793db910ce1868abc9b4b56a55b6ade Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 22 Aug 2023 13:09:47 -0400 Subject: [PATCH 19/24] Add logging linting Using fprint format and passing values in as extras to log is significantly faster and more secure than f strings --- pyproject.toml | 5 +---- src/stpipe/pipeline.py | 23 +++++++++++----------- src/stpipe/step.py | 43 ++++++++++++++++++++++++------------------ src/stpipe/subproc.py | 11 +++++------ tests/test_step.py | 2 +- 5 files changed, 44 insertions(+), 40 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3c9d9836..130a2668 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -94,7 +94,7 @@ select = [ 'ISC', # flake8-implicit-str-concat (prevent implicit string concat) 'ICN', # flake8-import-conventions (enforce import conventions) 'INP', # flake8-no-pep420 (prevent use of PEP420, i.e. implicit name spaces) - # 'G', # flake8-logging-format (best practices for logging) + 'G', # flake8-logging-format (best practices for logging) 'PIE', # flake8-pie (misc suggested improvement linting) 'T20', # flake8-print (prevent print statements in code) 'PT', # flake8-pytest-style (best practices for pytest) @@ -120,9 +120,6 @@ extend-exclude = [ 'src/stpipe/extern', 'scripts/strun', ] -extend-ignore = [ - 'W605', # invalid escape sequence -] [tool.ruff.extend-per-file-ignores] "tests/*.py" = ["S101", "S603", "S607", "INP001", "ARG001"] diff --git a/src/stpipe/pipeline.py b/src/stpipe/pipeline.py index bc53ce43..c84b3078 100644 --- a/src/stpipe/pipeline.py +++ b/src/stpipe/pipeline.py @@ -174,7 +174,7 @@ def get_config_from_reference(cls, dataset, disable=None, crds_observatory=None) disable = get_disable_crds_steppars() if disable: logger.debug( - f"{reftype.upper()}: CRDS parameter reference retrieval disabled." + "%s: CRDS parameter reference retrieval disabled.", reftype.upper() ) return refcfg @@ -197,7 +197,7 @@ def get_config_from_reference(cls, dataset, disable=None, crds_observatory=None) ) # # Now merge any config parameters from the step cfg file - logger.debug(f"Retrieving pipeline {reftype.upper()} parameters from CRDS") + logger.debug("Retrieving pipeline %s parameters from CRDS", reftype.upper()) try: ref_file = crds_client.get_reference_file( crds_parameters, @@ -205,13 +205,13 @@ def get_config_from_reference(cls, dataset, disable=None, crds_observatory=None) crds_observatory, ) except (AttributeError, crds_client.CrdsError): - logger.debug(f"{reftype.upper()}: No parameters found") + logger.debug("%s: No parameters found", reftype.upper()) else: if ref_file != "N/A": - logger.info(f"{reftype.upper()} parameters found: {ref_file}") + logger.info("%s parameters found: %s", reftype.upper(), ref_file) refcfg = cls.merge_pipeline_config(refcfg, ref_file) else: - logger.debug(f"No {reftype.upper()} reference files found.") + logger.debug("No %s reference files found.", reftype.upper()) return refcfg @@ -267,7 +267,7 @@ def _precache_references(self, input_file): ) as model: self._precache_references_opened(model) except (ValueError, TypeError, OSError): - self.log.info(f"First argument {input_file} does not appear to be a model") + self.log.info("First argument %s does not appear to be a model", input_file) def _precache_references_opened(self, model_or_container): """Pre-fetches references for `model_or_container`. @@ -309,10 +309,9 @@ def _precache_references_impl(self, model): fetch_types = sorted(set(self.reference_file_types) - set(ovr_refs.keys())) self.log.info( - "Prefetching reference files for dataset: " - + repr(model.meta.filename) - + " reftypes = " - + repr(fetch_types) + "Prefetching reference files for dataset: %r reftypes = %r", + model.meta.filename, + fetch_types, ) crds_refs = crds_client.get_multiple_reference_paths( model.get_crds_parameters(), fetch_types, model.crds_observatory @@ -322,7 +321,9 @@ def _precache_references_impl(self, model): for reftype, refpath in sorted(ref_path_map.items()): how = "Override" if reftype in ovr_refs else "Prefetch" - self.log.info(f"{how} for {reftype.upper()} reference file is '{refpath}'.") + self.log.info( + "%s for %s reference file is '%s'.", how, reftype.upper(), refpath + ) crds_client.check_reference_open(refpath) def get_pars(self, full_spec=True): diff --git a/src/stpipe/step.py b/src/stpipe/step.py index eb73cb80..0718f51e 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -361,7 +361,10 @@ def __init__( self.log.setLevel(log.logging.DEBUG) # Log the fact that we have been init-ed. - self.log.info(f"{self.__class__.__name__} instance created.") + self.log.info( + "%s instance created.", + self.__class__.__name__, + ) # Store the config file path so config filenames can be resolved # against it. @@ -387,7 +390,9 @@ def _check_args(self, args, discouraged_types, msg): for i, arg in enumerate(args): if isinstance(arg, discouraged_types): self.log.error( - f"{msg} {i} object. Use an instance of AbstractDataModel instead." + "%s %s object. Use an instance of AbstractDataModel instead.", + msg, + i, ) @property @@ -418,9 +423,9 @@ def run(self, *args): step_result = None - self.log.info(f"Step {self.name} running with args {args}.") + self.log.info("Step %s running with args %s.", self.name, args) - self.log.info(f"Step {self.name} parameters are: {self.get_pars()}") + self.log.info("Step %s parameters are: %s", self.name, self.get_pars()) if len(args): self.set_primary_input(args[0]) @@ -459,8 +464,9 @@ def run(self, *args): ] = "SKIPPED" except AttributeError as e: # noqa: PERF203 self.log.info( - "Could not record skip into DataModel" - f" header: {e}" + "Could not record skip into DataModel " + "header: %s", + e, ) elif isinstance(args[0], AbstractDataModel): try: @@ -470,7 +476,8 @@ def run(self, *args): except AttributeError as e: self.log.info( "Could not record skip into DataModel" - f" header: {e}" + " header: %s", + e, ) step_result = args[0] else: @@ -536,10 +543,10 @@ def run(self, *args): " `--save_results=false`" ) else: - self.log.info(f"Saving file {output_path}") + self.log.info("Saving file %s", output_path) result.save(output_path, overwrite=True) - self.log.info(f"Step {self.name} done") + self.log.info("Step %s done", self.name) finally: log.delegator.log = orig_log @@ -844,12 +851,12 @@ def get_config_from_reference(cls, dataset, disable=None, crds_observatory=None) disable = get_disable_crds_steppars() if disable: logger.info( - f"{reftype.upper()}: CRDS parameter reference retrieval disabled." + "%s: CRDS parameter reference retrieval disabled.", reftype.upper() ) return config_parser.ConfigObj() # Retrieve step parameters from CRDS - logger.debug(f"Retrieving step {reftype.upper()} parameters from CRDS") + logger.debug("Retrieving step %s parameters from CRDS", reftype.upper()) try: ref_file = crds_client.get_reference_file( crds_parameters, @@ -857,22 +864,22 @@ def get_config_from_reference(cls, dataset, disable=None, crds_observatory=None) crds_observatory, ) except (AttributeError, crds_client.CrdsError): - logger.debug(f"{reftype.upper()}: No parameters found") + logger.debug("%s: No parameters found", reftype.upper()) return config_parser.ConfigObj() if ref_file != "N/A": - logger.info(f"{reftype.upper()} parameters found: {ref_file}") + logger.info("%s parameters found: %s", reftype.upper(), ref_file) ref = config_parser.load_config_file(ref_file) ref_pars = { par: value for par, value in ref.items() if par not in ["class", "name"] } logger.debug( - f"{reftype.upper()} parameters retrieved from CRDS: {ref_pars}" + "%s parameters retrieved from CRDS: %s", reftype.upper(), ref_pars ) return ref - logger.debug(f"No {reftype.upper()} reference files found.") + logger.debug("No %s reference files found.", reftype.upper()) return config_parser.ConfigObj() @classmethod @@ -1001,7 +1008,7 @@ def save_model( **components, ) ) - self.log.info(f"Saved model in {output_path}") + self.log.info("Saved model in %s", output_path) return output_path @@ -1154,7 +1161,7 @@ def closeout(self, to_close=None, to_del=None): if hasattr(item, "close"): item.close() except Exception as exception: # noqa: PERF203 - self.log.debug(f'Could not close "{item}"Reason:\n{exception}') + self.log.debug('Could not close "%s"Reason:\n%s', item, exception) for item in to_del: try: del item @@ -1328,7 +1335,7 @@ def update_pars(self, parameters): getattr(self, step_name).update_pars(step_parameters) else: self.log.debug( - f"Parameter {parameter} is not valid for step {self}. Ignoring." + "Parameter %s is not valid for step %s. Ignoring.", parameter, self ) @classmethod diff --git a/src/stpipe/subproc.py b/src/stpipe/subproc.py index 487f2a98..e21e2eab 100644 --- a/src/stpipe/subproc.py +++ b/src/stpipe/subproc.py @@ -48,7 +48,7 @@ def process(self, *args): env[var] = val or None # Start the process and wait for it to finish. - self.log.info(f"Spawning {cmd_str!r}") + self.log.info("Spawning %r", cmd_str) try: p = subprocess.Popen( args=[cmd_str], @@ -60,19 +60,18 @@ def process(self, *args): ) err = p.wait() except Exception as e: - msg = f"Failed with an exception: \n{e}" - self.log.info(msg) + self.log.info("Failed with an exception: \n%s", e) if self.failure_as_exception: raise else: - self.log.info(f"Done with errorcode {err}") + self.log.info("Done with errorcode %s", err) # Log STDOUT/ERR if we are asked to do so. if self.log_stdout: - self.log.info(f"STDOUT: {p.stdout.read()}") + self.log.info("STDOUT: %s", p.stdout.read()) if self.log_stderr: - self.log.info(f"STDERR: {p.stderr.read()}") + self.log.info("STDERR: %s", p.stderr.read()) if self.exitcode_as_exception and err != 0: raise OSError(f"{cmd_str!r} returned error code {err}") diff --git a/tests/test_step.py b/tests/test_step.py index 8d660ca1..18e63974 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -53,7 +53,7 @@ class LoggingPipeline(Pipeline): def process(self): self.log.warning("This step has called out a warning.") - self.log.warning(f"{self.log} {self.log.handlers}") + self.log.warning("%s %s", self.log, self.log.handlers) def _datamodels_open(self, **kwargs): pass From 0a4017d5a8b3e8f45f0f66f83939d4fa8cae9d85 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 31 Oct 2023 13:02:48 -0400 Subject: [PATCH 20/24] Enable Repo-review --- .pre-commit-config.yaml | 18 ++++++++++++++++-- pyproject.toml | 28 +++++++++++++++++++++++++--- tests/cli/test_list.py | 1 + tests/cli/test_main.py | 1 + tests/test_abstract_datamodel.py | 1 + tests/test_config_parser.py | 1 + tests/test_format_template.py | 1 + tests/test_integration.py | 1 + tests/test_logger.py | 1 + tests/test_step.py | 1 + 10 files changed, 49 insertions(+), 5 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 01ae1096..1c822b6a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -36,10 +36,24 @@ repos: rev: 'v0.1.4' hooks: - id: ruff - args: ["--fix"] + args: ["--fix", "--show-fixes"] exclude: "scripts/strun" -- repo: https://github.com/psf/black +- repo: https://github.com/psf/black-pre-commit-mirror rev: 23.10.1 hooks: - id: black + +- repo: https://github.com/adamchainz/blacken-docs + rev: 1.16.0 + hooks: + - id: blacken-docs + additional_dependencies: + - black==22.12.0 + + +- repo: https://github.com/scientific-python/cookie + rev: 2023.08.23 + hooks: + - id: sp-repo-review + additional_dependencies: ["repo-review[cli]"] diff --git a/pyproject.toml b/pyproject.toml index 130a2668..82ab05cf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -48,7 +48,6 @@ strun = 'stpipe.cli.strun:main' requires = [ 'setuptools >=61', 'setuptools_scm[toml] >=3.4', - 'wheel', ] build-backend = 'setuptools.build_meta' @@ -62,11 +61,18 @@ zip-safe = true where = ['src'] [tool.pytest.ini_options] -minversion = 4.6 +minversion = 6 +log_cli_level = "INFO" +xfail_strict = true doctest_plus = true doctest_rst = true text_file_format = 'rst' -addopts = '' +addopts = [ + "--strict-config", + "--strict-markers", + "-ra", + "--color=yes" +] norecursedirs = [ 'src/stpipe/extern', ] @@ -78,6 +84,12 @@ filterwarnings = [ ] [tool.ruff] +src = [ + "src", + "tests", + "docs", + "setup.py", +] target-version = "py39" select = [ 'F', # Pyflakes @@ -149,3 +161,13 @@ exclude = ["src/stpipe/extern/*"] skip="*.pdf,*.fits,*.asdf,.tox,build,./tags,.git,docs/_build" # ignore-words-list=""" # """ + + +[tool.repo-review] +ignore = [ + "GH200", # Use dependabot + "PC140", # add MyPy to pre-commit + "PC180", # add prettier to pre-commit + "PC901", # custom pre-comit.ci message + "MY100", # Use MyPy +] diff --git a/tests/cli/test_list.py b/tests/cli/test_list.py index cb977e81..6bb3807d 100644 --- a/tests/cli/test_list.py +++ b/tests/cli/test_list.py @@ -1,4 +1,5 @@ import pytest + from stpipe import entry_points from stpipe.cli import handle_args from stpipe.entry_points import StepInfo diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py index f659e494..12c09ef4 100644 --- a/tests/cli/test_main.py +++ b/tests/cli/test_main.py @@ -1,6 +1,7 @@ import subprocess import pytest + import stpipe from stpipe import entry_points from stpipe.cli import handle_args diff --git a/tests/test_abstract_datamodel.py b/tests/test_abstract_datamodel.py index 95c1bd1c..803cf62c 100644 --- a/tests/test_abstract_datamodel.py +++ b/tests/test_abstract_datamodel.py @@ -3,6 +3,7 @@ """ import pytest + from stpipe.datamodel import AbstractDataModel diff --git a/tests/test_config_parser.py b/tests/test_config_parser.py index 626d81f2..bef530c0 100644 --- a/tests/test_config_parser.py +++ b/tests/test_config_parser.py @@ -2,6 +2,7 @@ from collections.abc import Mapping import pytest + from stpipe import config_parser from stpipe.extern.configobj.configobj import ConfigObj, Section from stpipe.utilities import _not_set diff --git a/tests/test_format_template.py b/tests/test_format_template.py index f737652f..106ffaa1 100644 --- a/tests/test_format_template.py +++ b/tests/test_format_template.py @@ -2,6 +2,7 @@ from functools import partial import pytest + from stpipe.format_template import FormatTemplate diff --git a/tests/test_integration.py b/tests/test_integration.py index c9255e82..757488fe 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -3,6 +3,7 @@ import asdf import yaml + from stpipe.integration import SCHEMAS_PATH diff --git a/tests/test_logger.py b/tests/test_logger.py index b28918ae..432224ff 100644 --- a/tests/test_logger.py +++ b/tests/test_logger.py @@ -2,6 +2,7 @@ import logging import pytest + from stpipe import log as stpipe_log diff --git a/tests/test_step.py b/tests/test_step.py index 18e63974..22c80ef7 100644 --- a/tests/test_step.py +++ b/tests/test_step.py @@ -5,6 +5,7 @@ import asdf import pytest + import stpipe.config_parser as cp from stpipe import cmdline from stpipe.pipeline import Pipeline From fb88b4a617e634615e319ee0fe9882fd529d641a Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 29 Aug 2023 10:43:11 -0400 Subject: [PATCH 21/24] Use prettier Prettier has the same idea as black, but for non python file types. --- .github/workflows/build.yml | 2 +- .github/workflows/changelog.yml | 10 +-- .github/workflows/ci.yml | 4 +- .pre-commit-config.yaml | 119 ++++++++++++++++---------------- CODE_OF_CONDUCT.md | 1 - pyproject.toml | 1 - 6 files changed, 69 insertions(+), 68 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 12215c1f..fafc7435 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -2,7 +2,7 @@ name: build on: release: - types: [ released ] + types: [released] pull_request: workflow_dispatch: diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml index cadc14fb..5827b36c 100644 --- a/.github/workflows/changelog.yml +++ b/.github/workflows/changelog.yml @@ -12,8 +12,8 @@ jobs: check: runs-on: ubuntu-latest steps: - - uses: scientific-python/action-check-changelogfile@0.2 - env: - CHANGELOG_FILENAME: CHANGES.rst - CHECK_MILESTONE: false - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - uses: scientific-python/action-check-changelogfile@0.2 + env: + CHANGELOG_FILENAME: CHANGES.rst + CHECK_MILESTONE: false + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e5613384..c7fb3ccd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -5,12 +5,12 @@ on: branches: - master tags: - - '*' + - "*" pull_request: schedule: # Weekly Monday 9AM build # * is a special character in YAML so you have to quote this string - - cron: '0 9 * * 1' + - cron: "0 9 * * 1" workflow_dispatch: concurrency: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1c822b6a..ce11b39d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,59 +1,62 @@ repos: - -- repo: https://github.com/pre-commit/pre-commit-hooks - rev: v4.5.0 - hooks: - - id: check-added-large-files - - id: check-ast - - id: check-case-conflict - - id: check-yaml - args: ["--unsafe"] - - id: check-toml - - id: check-merge-conflict - - id: check-symlinks - - id: debug-statements - exclude: "src/stpipe/cmdline.py" - - id: detect-private-key - - id: end-of-file-fixer - - id: trailing-whitespace - -- repo: https://github.com/pre-commit/pygrep-hooks - rev: v1.10.0 - hooks: - - id: rst-directive-colons - - id: rst-inline-touching-normal - - id: text-unicode-replacement-char - -- repo: https://github.com/codespell-project/codespell - rev: v2.2.6 - hooks: - - id: codespell - args: ["--write-changes"] - additional_dependencies: - - tomli - -- repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.1.4' - hooks: - - id: ruff - args: ["--fix", "--show-fixes"] - exclude: "scripts/strun" - -- repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.10.1 - hooks: - - id: black - -- repo: https://github.com/adamchainz/blacken-docs - rev: 1.16.0 - hooks: - - id: blacken-docs - additional_dependencies: - - black==22.12.0 - - -- repo: https://github.com/scientific-python/cookie - rev: 2023.08.23 - hooks: - - id: sp-repo-review - additional_dependencies: ["repo-review[cli]"] + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: check-added-large-files + - id: check-ast + - id: check-case-conflict + - id: check-yaml + args: ["--unsafe"] + - id: check-toml + - id: check-merge-conflict + - id: check-symlinks + - id: debug-statements + exclude: "src/stpipe/cmdline.py" + - id: detect-private-key + - id: end-of-file-fixer + - id: trailing-whitespace + + - repo: https://github.com/pre-commit/pygrep-hooks + rev: v1.10.0 + hooks: + - id: rst-directive-colons + - id: rst-inline-touching-normal + - id: text-unicode-replacement-char + + - repo: https://github.com/codespell-project/codespell + rev: v2.2.6 + hooks: + - id: codespell + args: ["--write-changes"] + additional_dependencies: + - tomli + + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: "v0.1.4" + hooks: + - id: ruff + args: ["--fix", "--show-fixes"] + exclude: "scripts/strun" + + - repo: https://github.com/psf/black-pre-commit-mirror + rev: 23.10.1 + hooks: + - id: black + + - repo: https://github.com/adamchainz/blacken-docs + rev: 1.16.0 + hooks: + - id: blacken-docs + additional_dependencies: + - black==22.12.0 + + - repo: https://github.com/pre-commit/mirrors-prettier + rev: "v3.0.2" + hooks: + - id: prettier + + - repo: https://github.com/scientific-python/cookie + rev: 2023.08.23 + hooks: + - id: sp-repo-review + additional_dependencies: ["repo-review[cli]"] diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md index ddba00df..8d726b0f 100644 --- a/CODE_OF_CONDUCT.md +++ b/CODE_OF_CONDUCT.md @@ -2,7 +2,6 @@ We expect all "spacetelescope" organization projects to adopt a code of conduct that ensures a productive, respectful environment for all open source contributors and participants. We are committed to providing a strong and enforced code of conduct and expect everyone in our community to follow these guidelines when interacting with others in all forums. Our goal is to keep ours a positive, inclusive, successful, and growing community. The community of participants in open source Astronomy projects is made up of members from around the globe with a diverse set of skills, personalities, and experiences. It is through these differences that our community experiences success and continued growth. - As members of the community, - We pledge to treat all people with respect and provide a harassment- and bullying-free environment, regardless of sex, sexual orientation and/or gender identity, disability, physical appearance, body size, race, nationality, ethnicity, and religion. In particular, sexual language and imagery, sexist, racist, or otherwise exclusionary jokes are not appropriate. diff --git a/pyproject.toml b/pyproject.toml index 82ab05cf..ad48e601 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -167,7 +167,6 @@ skip="*.pdf,*.fits,*.asdf,.tox,build,./tags,.git,docs/_build" ignore = [ "GH200", # Use dependabot "PC140", # add MyPy to pre-commit - "PC180", # add prettier to pre-commit "PC901", # custom pre-comit.ci message "MY100", # Use MyPy ] From 2567bb5f4e7de422c07907cc33debada22f61076 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 31 Oct 2023 13:14:51 -0400 Subject: [PATCH 22/24] Update repo-review --- .pre-commit-config.yaml | 2 +- pyproject.toml | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ce11b39d..2282c817 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -56,7 +56,7 @@ repos: - id: prettier - repo: https://github.com/scientific-python/cookie - rev: 2023.08.23 + rev: 2023.10.27 hooks: - id: sp-repo-review additional_dependencies: ["repo-review[cli]"] diff --git a/pyproject.toml b/pyproject.toml index ad48e601..f2c47bb3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -90,8 +90,15 @@ src = [ "docs", "setup.py", ] -target-version = "py39" -select = [ +line-length = 88 +extend-exclude = [ + 'docs', + 'src/stpipe/extern', + 'scripts/strun', +] + +[tool.ruff.lint] +extend-select = [ 'F', # Pyflakes 'W', 'E', # pycodestyle 'I', # isort @@ -126,14 +133,8 @@ select = [ 'PERF', # Perflint (performance linting) 'RUF', # ruff specific checks ] -line-length = 88 -extend-exclude = [ - 'docs', - 'src/stpipe/extern', - 'scripts/strun', -] -[tool.ruff.extend-per-file-ignores] +[tool.ruff.lint.extend-per-file-ignores] "tests/*.py" = ["S101", "S603", "S607", "INP001", "ARG001"] "src/stpipe/tests/*.py" = ["S101"] "src/stpipe/cli/*.py" = ["T201"] From 8dc4d95eefb60810fba7acab0b516c533beb5de2 Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 31 Oct 2023 13:18:25 -0400 Subject: [PATCH 23/24] Update changes --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index b4f934f4..709c2e7c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,7 +1,7 @@ 0.6.0 (unreleased) ================== -- +- Update style and linting checking [#103] 0.5.1 (2023-10-02) ================== From ba6ae328d595a6a99f07e199b11fab9ed0afb4ba Mon Sep 17 00:00:00 2001 From: William Jamieson Date: Tue, 31 Oct 2023 13:30:08 -0400 Subject: [PATCH 24/24] Switch to ruff formatter --- .pre-commit-config.yaml | 6 +----- pyproject.toml | 6 ++++++ src/stpipe/step.py | 6 +++++- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2282c817..114cd7c4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -37,11 +37,7 @@ repos: - id: ruff args: ["--fix", "--show-fixes"] exclude: "scripts/strun" - - - repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.10.1 - hooks: - - id: black + - id: ruff-format - repo: https://github.com/adamchainz/blacken-docs rev: 1.16.0 diff --git a/pyproject.toml b/pyproject.toml index f2c47bb3..795cedbd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,6 +82,9 @@ testpaths = [ filterwarnings = [ "error::ResourceWarning", ] +markers = [ + "soctests", +] [tool.ruff] src = [ @@ -133,6 +136,9 @@ extend-select = [ 'PERF', # Perflint (performance linting) 'RUF', # ruff specific checks ] +ignore = [ + "ISC001", # conflicts with ruff formatter +] [tool.ruff.lint.extend-per-file-ignores] "tests/*.py" = ["S101", "S603", "S607", "INP001", "ARG001"] diff --git a/src/stpipe/step.py b/src/stpipe/step.py index 0718f51e..7ce9f7b3 100644 --- a/src/stpipe/step.py +++ b/src/stpipe/step.py @@ -189,7 +189,11 @@ def from_cmdline(args): @classmethod def _parse_class_and_name( - cls, config, parent=None, name=None, config_file=None # noqa: ARG003 + cls, + config, + parent=None, # noqa: ARG003 + name=None, + config_file=None, ): if "class" in config: step_class = utilities.import_class(