From 537b5f807f311965b587c43306c2bfdee3be0ee6 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 29 Nov 2024 20:27:43 +0000 Subject: [PATCH 1/6] make_ruff_stricter --- ruff.toml | 8 +++++++- tests/callbacks/fitting/test_fitting_methods.py | 8 ++++++-- tests/devices/test_block.py | 2 +- tests/devices/test_init.py | 2 +- tests/test_log.py | 4 ++-- 5 files changed, 17 insertions(+), 7 deletions(-) diff --git a/ruff.toml b/ruff.toml index 6323b8e..9ecefae 100644 --- a/ruff.toml +++ b/ruff.toml @@ -15,18 +15,24 @@ extend-select = [ "B", "NPY", # Numpy-specific rules "RUF", # Ruff-specific checks, include some useful asyncio rules + "FURB", # Rules from refurb + "ERA", # Commented-out code + "PT", # Pytest-specific rules + "LOG", # Logging-specific rules + "G", # Logging-specific rules ] ignore = [ "D406", # Section name should end with a newline ("{name}") "D407", # Missing dashed underline after section ("{name}") "D213", # Incompatible with D212 "D203", # Incompatible with D211 + "B901", # This is a standard, expected, pattern in bluesky ] [lint.per-file-ignores] "tests/*" = [ "N802", "D", # Don't require method documentation for test methods - "ANN" # Don't require tests to use type annotations + "ANN", # Don't require tests to use type annotations ] "doc/conf.py" = [ "D100" diff --git a/tests/callbacks/fitting/test_fitting_methods.py b/tests/callbacks/fitting/test_fitting_methods.py index c5e9e3b..8fed994 100644 --- a/tests/callbacks/fitting/test_fitting_methods.py +++ b/tests/callbacks/fitting/test_fitting_methods.py @@ -266,7 +266,9 @@ def test_polynomial_model_order(self, deg: int): # -1 and 8 are both invalid polynomial degrees x = np.zeros(3) - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match="The polynomial degree should be at least 0 and smaller than 8." + ): Polynomial.model(deg).func(x) def test_polynomial_model(self): @@ -301,7 +303,9 @@ def test_invalid_polynomial_guess(self, deg: int): y = np.array([1.0, 0.0, 1.0]) # -1 and 8 are both invalid polynomial degrees - with pytest.raises(ValueError): + with pytest.raises( + ValueError, match="The polynomial degree should be at least 0 and smaller than 8." + ): Polynomial.guess(deg)(x, y) diff --git a/tests/devices/test_block.py b/tests/devices/test_block.py index 8ecebbc..711bbe5 100644 --- a/tests/devices/test_block.py +++ b/tests/devices/test_block.py @@ -206,7 +206,7 @@ async def test_block_set_with_settle_time_longer_than_timeout(): @pytest.mark.parametrize( - "func,args", + ("func", "args"), [ (block_r, (float, "some_block")), (block_rw, (float, "some_block")), diff --git a/tests/devices/test_init.py b/tests/devices/test_init.py index affcfda..e946a40 100644 --- a/tests/devices/test_init.py +++ b/tests/devices/test_init.py @@ -33,7 +33,7 @@ def test_get_pv_prefix(): def test_cannot_get_pv_prefix(): with patch("os.getenv") as mock_getenv: mock_getenv.return_value = None - with pytest.raises(EnvironmentError): + with pytest.raises(EnvironmentError, match="MYPVPREFIX environment variable not available"): get_pv_prefix() diff --git a/tests/test_log.py b/tests/test_log.py index 29c3556..b9689c4 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -49,13 +49,13 @@ def test_set_bluesky_log_levels_default_previously_unset(): def test_set_bluesky_log_levels_default_previously_set(): # Setup, set some explicit log levels on various loggers. - logging.getLogger("ibex_bluesky_core").setLevel(logging.WARN) + logging.getLogger("ibex_bluesky_core").setLevel(logging.WARNING) logging.getLogger("bluesky").setLevel(logging.INFO) logging.getLogger("ophyd_async").setLevel(logging.DEBUG) set_bluesky_log_levels() # Assert we didn't override the previously explicitly-set levels - assert logging.getLogger("ibex_bluesky_core").level == logging.WARN + assert logging.getLogger("ibex_bluesky_core").level == logging.WARNING assert logging.getLogger("bluesky").level == logging.INFO assert logging.getLogger("ophyd_async").level == logging.DEBUG From b6806681db8ec4392782bcfafe26e089467efd63 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 29 Nov 2024 20:43:22 +0000 Subject: [PATCH 2/6] Fix ruff preview-mode checks --- src/ibex_bluesky_core/callbacks/plotting.py | 2 +- src/ibex_bluesky_core/devices/dae/__init__.py | 2 +- .../devices/dae/dae_period_settings.py | 8 ++++---- src/ibex_bluesky_core/devices/simpledae/__init__.py | 3 +-- src/ibex_bluesky_core/devices/simpledae/reducers.py | 4 ++-- src/ibex_bluesky_core/devices/simpledae/waiters.py | 4 ++-- tests/devices/simpledae/test_controllers.py | 6 +++--- tests/devices/simpledae/test_reducers.py | 10 +++++----- tests/devices/test_block.py | 10 +++++----- tests/test_log.py | 2 +- tests/test_preprocessors.py | 4 ++-- 11 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/ibex_bluesky_core/callbacks/plotting.py b/src/ibex_bluesky_core/callbacks/plotting.py index 5547346..2feb436 100644 --- a/src/ibex_bluesky_core/callbacks/plotting.py +++ b/src/ibex_bluesky_core/callbacks/plotting.py @@ -37,7 +37,7 @@ def __init__( """ super().__init__(y=y, x=x, *args, **kwargs) # noqa: B026 if yerr is not None: - self.yerr, *others = get_obj_fields([yerr]) + self.yerr, *_others = get_obj_fields([yerr]) else: self.yerr = None self.yerr_data = [] diff --git a/src/ibex_bluesky_core/devices/dae/__init__.py b/src/ibex_bluesky_core/devices/dae/__init__.py index 81a506c..8485ce6 100644 --- a/src/ibex_bluesky_core/devices/dae/__init__.py +++ b/src/ibex_bluesky_core/devices/dae/__init__.py @@ -32,7 +32,7 @@ def _get_names_and_values(element: Element) -> tuple[Any, Any] | tuple[None, Non def set_value_in_dae_xml( - elements: List[Element], name: str, value: str | Enum | int | None | float + elements: List[Element], name: str, value: str | Enum | int | float | None ) -> None: """Find and set a value in the DAE XML, given a name and value. diff --git a/src/ibex_bluesky_core/devices/dae/dae_period_settings.py b/src/ibex_bluesky_core/devices/dae/dae_period_settings.py index 66ec95e..f57a09f 100644 --- a/src/ibex_bluesky_core/devices/dae/dae_period_settings.py +++ b/src/ibex_bluesky_core/devices/dae/dae_period_settings.py @@ -59,12 +59,12 @@ class DaePeriodSettingsData: """Dataclass for the hardware period settings.""" periods_settings: List[SinglePeriodSettings] | None = None - periods_soft_num: None | int = None + periods_soft_num: int | None = None periods_type: PeriodType | None = None periods_src: PeriodSource | None = None - periods_file: None | str = None - periods_seq: None | int = None - periods_delay: None | int = None + periods_file: str | None = None + periods_seq: int | None = None + periods_delay: int | None = None def _convert_xml_to_period_settings(value: str) -> DaePeriodSettingsData: diff --git a/src/ibex_bluesky_core/devices/simpledae/__init__.py b/src/ibex_bluesky_core/devices/simpledae/__init__.py index e99e118..25be9e0 100644 --- a/src/ibex_bluesky_core/devices/simpledae/__init__.py +++ b/src/ibex_bluesky_core/devices/simpledae/__init__.py @@ -72,8 +72,7 @@ def __init__( # published when the top-level SimpleDae object is read. extra_readables = set() for strategy in [self.controller, self.waiter, self.reducer]: - for sig in strategy.additional_readable_signals(self): - extra_readables.add(sig) + extra_readables.update(strategy.additional_readable_signals(self)) logger.info("extra readables: %s", list(extra_readables)) self.add_readables(devices=list(extra_readables)) diff --git a/src/ibex_bluesky_core/devices/simpledae/reducers.py b/src/ibex_bluesky_core/devices/simpledae/reducers.py index cc2e883..4e313d4 100644 --- a/src/ibex_bluesky_core/devices/simpledae/reducers.py +++ b/src/ibex_bluesky_core/devices/simpledae/reducers.py @@ -3,7 +3,7 @@ import asyncio import logging import math -from abc import ABCMeta, abstractmethod +from abc import ABC, abstractmethod from typing import TYPE_CHECKING, Collection, Sequence import scipp as sc @@ -41,7 +41,7 @@ async def sum_spectra(spectra: Collection[DaeSpectra]) -> sc.Variable | sc.DataA return summed_counts -class ScalarNormalizer(Reducer, StandardReadable, metaclass=ABCMeta): +class ScalarNormalizer(Reducer, StandardReadable, ABC): """Sum a set of user-specified spectra, then normalize by a scalar signal.""" def __init__(self, prefix: str, detector_spectra: Sequence[int]) -> None: diff --git a/src/ibex_bluesky_core/devices/simpledae/waiters.py b/src/ibex_bluesky_core/devices/simpledae/waiters.py index 258e870..af32177 100644 --- a/src/ibex_bluesky_core/devices/simpledae/waiters.py +++ b/src/ibex_bluesky_core/devices/simpledae/waiters.py @@ -2,7 +2,7 @@ import asyncio import logging -from abc import ABCMeta, abstractmethod +from abc import ABC, abstractmethod from typing import TYPE_CHECKING, Generic, TypeVar from ophyd_async.core import ( @@ -22,7 +22,7 @@ T = TypeVar("T", int, float) -class SimpleWaiter(Waiter, Generic[T], metaclass=ABCMeta): +class SimpleWaiter(Waiter, Generic[T], ABC): """Wait for a single DAE variable to be greater or equal to a specified numeric value.""" def __init__(self, value: T) -> None: diff --git a/tests/devices/simpledae/test_controllers.py b/tests/devices/simpledae/test_controllers.py index bd7ad95..a850793 100644 --- a/tests/devices/simpledae/test_controllers.py +++ b/tests/devices/simpledae/test_controllers.py @@ -30,7 +30,7 @@ def aborting_run_per_point_controller() -> RunPerPointController: return RunPerPointController(save_run=False) -async def test_period_per_point_controller_publishes_current_period( +def test_period_per_point_controller_publishes_current_period( simpledae: SimpleDae, period_per_point_controller: PeriodPerPointController ): assert period_per_point_controller.additional_readable_signals(simpledae) == [ @@ -83,7 +83,7 @@ async def test_run_per_point_controller_starts_and_ends_runs( get_mock_put(simpledae.controls.end_run).assert_called_once_with(None, wait=True) -async def test_run_per_point_controller_publishes_run( +def test_run_per_point_controller_publishes_run( simpledae: SimpleDae, run_per_point_controller: RunPerPointController ): assert run_per_point_controller.additional_readable_signals(simpledae) == [ @@ -91,7 +91,7 @@ async def test_run_per_point_controller_publishes_run( ] -async def test_aborting_run_per_point_controller_doesnt_publish_run( +def test_aborting_run_per_point_controller_doesnt_publish_run( simpledae: SimpleDae, aborting_run_per_point_controller: RunPerPointController ): assert aborting_run_per_point_controller.additional_readable_signals(simpledae) == [] diff --git a/tests/devices/simpledae/test_reducers.py b/tests/devices/simpledae/test_reducers.py index de78479..2950442 100644 --- a/tests/devices/simpledae/test_reducers.py +++ b/tests/devices/simpledae/test_reducers.py @@ -49,7 +49,7 @@ def __init__(self): # Scalar Normalizer -async def test_period_good_frames_normalizer_publishes_period_good_frames( +def test_period_good_frames_normalizer_publishes_period_good_frames( period_good_frames_reducer: PeriodGoodFramesNormalizer, ): fake_dae: SimpleDae = FakeDae() # type: ignore @@ -60,7 +60,7 @@ async def test_period_good_frames_normalizer_publishes_period_good_frames( assert period_good_frames_reducer.denominator(fake_dae) == fake_dae.period.good_frames -async def test_good_frames_normalizer_publishes_good_frames( +def test_good_frames_normalizer_publishes_good_frames( good_frames_reducer: GoodFramesNormalizer, ): fake_dae: SimpleDae = FakeDae() # type: ignore @@ -71,7 +71,7 @@ async def test_good_frames_normalizer_publishes_good_frames( assert good_frames_reducer.denominator(fake_dae) == fake_dae.good_frames -async def test_scalar_normalizer_publishes_uncertainties( +def test_scalar_normalizer_publishes_uncertainties( simpledae: SimpleDae, good_frames_reducer: GoodFramesNormalizer, ): @@ -284,7 +284,7 @@ async def test_monitor_normalizer_uncertainties( assert intensity_stddev == pytest.approx(math.sqrt((6000 + (6000**2 / 15000)) / 15000**2), 1e-4) -async def test_monitor_normalizer_publishes_raw_and_normalized_counts( +def test_monitor_normalizer_publishes_raw_and_normalized_counts( simpledae: SimpleDae, monitor_normalizer: MonitorNormalizer, ): @@ -294,7 +294,7 @@ async def test_monitor_normalizer_publishes_raw_and_normalized_counts( assert monitor_normalizer.mon_counts in readables -async def test_monitor_normalizer_publishes_raw_and_normalized_count_uncertainties( +def test_monitor_normalizer_publishes_raw_and_normalized_count_uncertainties( simpledae: SimpleDae, monitor_normalizer: MonitorNormalizer, ): diff --git a/tests/devices/test_block.py b/tests/devices/test_block.py index 711bbe5..06c0e57 100644 --- a/tests/devices/test_block.py +++ b/tests/devices/test_block.py @@ -100,12 +100,12 @@ async def test_locate(rw_rbv_block): } -async def test_hints(readable_block): +def test_hints(readable_block): # The primary readback should be the only "hinted" signal on a block assert readable_block.hints == {"fields": ["float_block"]} -async def test_mot_hints(mot_block): +def test_mot_hints(mot_block): assert mot_block.hints == {"fields": ["mot_block"]} @@ -239,18 +239,18 @@ async def test_runcontrol_read_and_describe(readable_block): assert descriptor["float_block-run_control-in_range"]["dtype"] == "boolean" -async def test_runcontrol_hints(readable_block): +def test_runcontrol_hints(readable_block): # Hinted field for explicitly reading run-control: is the reading in range? hints = readable_block.run_control.hints assert hints == {"fields": ["float_block-run_control-in_range"]} -async def test_runcontrol_monitors_correct_pv(readable_block): +def test_runcontrol_monitors_correct_pv(readable_block): source = readable_block.run_control.in_range.source assert source.endswith("UNITTEST:MOCK:CS:SB:float_block:RC:INRANGE") -async def test_mot_block_runcontrol_monitors_correct_pv(mot_block): +def test_mot_block_runcontrol_monitors_correct_pv(mot_block): source = mot_block.run_control.in_range.source # The main "motor" uses mot_block:SP:RBV, but run control should not. assert source.endswith("UNITTEST:MOCK:CS:SB:mot_block:RC:INRANGE") diff --git a/tests/test_log.py b/tests/test_log.py index b9689c4..180fadd 100644 --- a/tests/test_log.py +++ b/tests/test_log.py @@ -25,7 +25,7 @@ def test_setup_logging_does_not_crash_if_directory_cannot_be_created( mock_makedirs.side_effect = OSError setup_logging() - stdout, stderr = capfd.readouterr() + _, stderr = capfd.readouterr() assert stderr == "unable to create ibex_bluesky_core log directory\n" diff --git a/tests/test_preprocessors.py b/tests/test_preprocessors.py index 2a3ec9e..3201d2d 100644 --- a/tests/test_preprocessors.py +++ b/tests/test_preprocessors.py @@ -24,7 +24,7 @@ async def mock_rb_num() -> SignalWithExpectedRbv: return SignalWithExpectedRbv(mock_rbnum_signal, rb_num) -async def test_rb_number_preprocessor_adds_rb_number(RE, mock_rb_num): +def test_rb_number_preprocessor_adds_rb_number(RE, mock_rb_num): with ( patch( "ibex_bluesky_core.preprocessors._get_rb_number_signal", return_value=mock_rb_num.signal @@ -45,7 +45,7 @@ def plan(): assert start_doc["rb_number"] == mock_rb_num.rb_num -async def test_rb_number_preprocessor_adds_unknown_if_signal_not_connected(RE, mock_rb_num): +def test_rb_number_preprocessor_adds_unknown_if_signal_not_connected(RE, mock_rb_num): with ( patch( "ibex_bluesky_core.preprocessors._get_rb_number_signal", return_value=mock_rb_num.signal From 56d81a175e99aeadc921396d7266ca9814064d95 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 29 Nov 2024 20:49:06 +0000 Subject: [PATCH 3/6] Enable ruff UP --- manual_system_tests/dae_scan.py | 2 +- manual_system_tests/interruption.py | 2 +- ruff.toml | 1 + src/ibex_bluesky_core/devices/__init__.py | 6 +++--- src/ibex_bluesky_core/devices/block.py | 14 +++++++------- src/ibex_bluesky_core/devices/dae/__init__.py | 8 ++++---- .../devices/dae/dae_period_settings.py | 3 +-- .../devices/dae/dae_tcb_settings.py | 5 ++--- .../devices/simpledae/__init__.py | 6 +++--- .../devices/simpledae/reducers.py | 3 ++- src/ibex_bluesky_core/plan_stubs/__init__.py | 3 ++- src/ibex_bluesky_core/preprocessors.py | 2 +- tests/callbacks/test_document_logging_callback.py | 2 +- tests/test_run_engine.py | 3 ++- 14 files changed, 31 insertions(+), 29 deletions(-) diff --git a/manual_system_tests/dae_scan.py b/manual_system_tests/dae_scan.py index eb9e32a..123d19c 100644 --- a/manual_system_tests/dae_scan.py +++ b/manual_system_tests/dae_scan.py @@ -1,8 +1,8 @@ """Demonstration plan showing basic bluesky functionality.""" import os +from collections.abc import Generator from pathlib import Path -from typing import Generator import bluesky.plan_stubs as bps import bluesky.plans as bp diff --git a/manual_system_tests/interruption.py b/manual_system_tests/interruption.py index 152a42a..dacdb3e 100644 --- a/manual_system_tests/interruption.py +++ b/manual_system_tests/interruption.py @@ -1,7 +1,7 @@ """Demonstration plan showing basic bluesky functionality.""" import os -from typing import Generator +from collections.abc import Generator import bluesky.plan_stubs as bps from bluesky.utils import Msg diff --git a/ruff.toml b/ruff.toml index 9ecefae..5d17a91 100644 --- a/ruff.toml +++ b/ruff.toml @@ -20,6 +20,7 @@ extend-select = [ "PT", # Pytest-specific rules "LOG", # Logging-specific rules "G", # Logging-specific rules + "UP", # Pyupgrade ] ignore = [ "D406", # Section name should end with a newline ("{name}") diff --git a/src/ibex_bluesky_core/devices/__init__.py b/src/ibex_bluesky_core/devices/__init__.py index a96f78e..306e63f 100644 --- a/src/ibex_bluesky_core/devices/__init__.py +++ b/src/ibex_bluesky_core/devices/__init__.py @@ -5,7 +5,7 @@ import binascii import os import zlib -from typing import Type, TypeVar +from typing import TypeVar from ophyd_async.core import SignalDatatype, SignalRW from ophyd_async.epics.core import epics_signal_rw @@ -18,7 +18,7 @@ def get_pv_prefix() -> str: prefix = os.getenv("MYPVPREFIX") if prefix is None: - raise EnvironmentError("MYPVPREFIX environment variable not available - please define") + raise OSError("MYPVPREFIX environment variable not available - please define") return prefix @@ -48,7 +48,7 @@ def compress_and_hex(value: str) -> bytes: return binascii.hexlify(compr) -def isis_epics_signal_rw(datatype: Type[T], read_pv: str, name: str = "") -> SignalRW[T]: +def isis_epics_signal_rw(datatype: type[T], read_pv: str, name: str = "") -> SignalRW[T]: """Make a RW signal with ISIS' PV naming standard ie. read_pv as TITLE, write_pv as TITLE:SP.""" write_pv = f"{read_pv}:SP" return epics_signal_rw(datatype, read_pv, write_pv, name) diff --git a/src/ibex_bluesky_core/devices/block.py b/src/ibex_bluesky_core/devices/block.py index bab6476..7b64d44 100644 --- a/src/ibex_bluesky_core/devices/block.py +++ b/src/ibex_bluesky_core/devices/block.py @@ -3,7 +3,7 @@ import asyncio import logging from dataclasses import dataclass -from typing import Callable, Generic, Type, TypeVar +from typing import Callable, Generic, TypeVar from bluesky.protocols import Locatable, Location, Movable, Triggerable from ophyd_async.core import ( @@ -119,7 +119,7 @@ def __init__(self, prefix: str, name: str = "") -> None: class BlockR(StandardReadable, Triggerable, Generic[T]): """Device representing an IBEX readable block of arbitrary data type.""" - def __init__(self, datatype: Type[T], prefix: str, block_name: str) -> None: + def __init__(self, datatype: type[T], prefix: str, block_name: str) -> None: """Create a new read-only block. Args: @@ -154,7 +154,7 @@ class BlockRw(BlockR[T], Movable): def __init__( self, - datatype: Type[T], + datatype: type[T], prefix: str, block_name: str, *, @@ -231,7 +231,7 @@ class BlockRwRbv(BlockRw[T], Locatable): def __init__( self, - datatype: Type[T], + datatype: type[T], prefix: str, block_name: str, *, @@ -326,7 +326,7 @@ def __repr__(self) -> str: return f"{self.__class__.__name__}(name={self.name})" -def block_r(datatype: Type[T], block_name: str) -> BlockR[T]: +def block_r(datatype: type[T], block_name: str) -> BlockR[T]: """Get a local read-only block for the current instrument. See documentation of BlockR for more information. @@ -335,7 +335,7 @@ def block_r(datatype: Type[T], block_name: str) -> BlockR[T]: def block_rw( - datatype: Type[T], block_name: str, *, write_config: BlockWriteConfig[T] | None = None + datatype: type[T], block_name: str, *, write_config: BlockWriteConfig[T] | None = None ) -> BlockRw[T]: """Get a local read-write block for the current instrument. @@ -347,7 +347,7 @@ def block_rw( def block_rw_rbv( - datatype: Type[T], block_name: str, *, write_config: BlockWriteConfig[T] | None = None + datatype: type[T], block_name: str, *, write_config: BlockWriteConfig[T] | None = None ) -> BlockRwRbv[T]: """Get a local read/write/setpoint readback block for the current instrument. diff --git a/src/ibex_bluesky_core/devices/dae/__init__.py b/src/ibex_bluesky_core/devices/dae/__init__.py index 8485ce6..f0bfd31 100644 --- a/src/ibex_bluesky_core/devices/dae/__init__.py +++ b/src/ibex_bluesky_core/devices/dae/__init__.py @@ -1,11 +1,11 @@ """Utilities for the DAE device - mostly XML helpers.""" from enum import Enum -from typing import Any, Dict, List +from typing import Any from xml.etree.ElementTree import Element -def convert_xml_to_names_and_values(xml: Element) -> Dict[str, str]: +def convert_xml_to_names_and_values(xml: Element) -> dict[str, str]: """Convert an XML element's children to a dict containing .text:.text.""" names_and_values = dict() elements = get_all_elements_in_xml_with_child_called_name(xml) @@ -16,7 +16,7 @@ def convert_xml_to_names_and_values(xml: Element) -> Dict[str, str]: return names_and_values -def get_all_elements_in_xml_with_child_called_name(xml: Element) -> List[Element]: +def get_all_elements_in_xml_with_child_called_name(xml: Element) -> list[Element]: """Find all elements with a "name" element, but ignore the first one as it's the root.""" elements = xml.findall("*/Name/..") return elements @@ -32,7 +32,7 @@ def _get_names_and_values(element: Element) -> tuple[Any, Any] | tuple[None, Non def set_value_in_dae_xml( - elements: List[Element], name: str, value: str | Enum | int | float | None + elements: list[Element], name: str, value: str | Enum | int | float | None ) -> None: """Find and set a value in the DAE XML, given a name and value. diff --git a/src/ibex_bluesky_core/devices/dae/dae_period_settings.py b/src/ibex_bluesky_core/devices/dae/dae_period_settings.py index f57a09f..341c942 100644 --- a/src/ibex_bluesky_core/devices/dae/dae_period_settings.py +++ b/src/ibex_bluesky_core/devices/dae/dae_period_settings.py @@ -4,7 +4,6 @@ import xml.etree.ElementTree as ET from dataclasses import dataclass from enum import Enum -from typing import List from xml.etree.ElementTree import tostring from bluesky.protocols import Locatable, Location, Movable @@ -58,7 +57,7 @@ class SinglePeriodSettings: class DaePeriodSettingsData: """Dataclass for the hardware period settings.""" - periods_settings: List[SinglePeriodSettings] | None = None + periods_settings: list[SinglePeriodSettings] | None = None periods_soft_num: int | None = None periods_type: PeriodType | None = None periods_src: PeriodSource | None = None diff --git a/src/ibex_bluesky_core/devices/dae/dae_tcb_settings.py b/src/ibex_bluesky_core/devices/dae/dae_tcb_settings.py index b272fa4..9d7c998 100644 --- a/src/ibex_bluesky_core/devices/dae/dae_tcb_settings.py +++ b/src/ibex_bluesky_core/devices/dae/dae_tcb_settings.py @@ -4,7 +4,6 @@ import xml.etree.ElementTree as ET from dataclasses import dataclass from enum import Enum -from typing import Dict from xml.etree.ElementTree import tostring from bluesky.protocols import Locatable, Location, Movable @@ -66,14 +65,14 @@ class TimeRegimeRow: class TimeRegime: """Time regime - contains a dict(rows) which is row_number:TimeRegimeRow.""" - rows: Dict[int, TimeRegimeRow] + rows: dict[int, TimeRegimeRow] @dataclass(kw_only=True) class DaeTCBSettingsData: """Dataclass for the DAE TCB settings.""" - tcb_tables: Dict[int, TimeRegime] | None = None + tcb_tables: dict[int, TimeRegime] | None = None tcb_file: str | None = None time_unit: TimeUnit | None = None tcb_calculation_method: CalculationMethod | None = None diff --git a/src/ibex_bluesky_core/devices/simpledae/__init__.py b/src/ibex_bluesky_core/devices/simpledae/__init__.py index 25be9e0..27a67ef 100644 --- a/src/ibex_bluesky_core/devices/simpledae/__init__.py +++ b/src/ibex_bluesky_core/devices/simpledae/__init__.py @@ -51,9 +51,9 @@ def __init__( """ self.prefix = prefix - self.controller: "Controller" = controller - self.waiter: "Waiter" = waiter - self.reducer: "Reducer" = reducer + self.controller: Controller = controller + self.waiter: Waiter = waiter + self.reducer: Reducer = reducer logger.info( "created simpledae with prefix=%s, controller=%s, waiter=%s, reducer=%s", diff --git a/src/ibex_bluesky_core/devices/simpledae/reducers.py b/src/ibex_bluesky_core/devices/simpledae/reducers.py index 4e313d4..0c9b431 100644 --- a/src/ibex_bluesky_core/devices/simpledae/reducers.py +++ b/src/ibex_bluesky_core/devices/simpledae/reducers.py @@ -4,7 +4,8 @@ import logging import math from abc import ABC, abstractmethod -from typing import TYPE_CHECKING, Collection, Sequence +from collections.abc import Collection, Sequence +from typing import TYPE_CHECKING import scipp as sc from ophyd_async.core import ( diff --git a/src/ibex_bluesky_core/plan_stubs/__init__.py b/src/ibex_bluesky_core/plan_stubs/__init__.py index 803e43b..12c1ddb 100644 --- a/src/ibex_bluesky_core/plan_stubs/__init__.py +++ b/src/ibex_bluesky_core/plan_stubs/__init__.py @@ -1,6 +1,7 @@ """Core plan stubs.""" -from typing import Callable, Generator, ParamSpec, TypeVar, cast +from collections.abc import Generator +from typing import Callable, ParamSpec, TypeVar, cast import bluesky.plan_stubs as bps from bluesky.utils import Msg diff --git a/src/ibex_bluesky_core/preprocessors.py b/src/ibex_bluesky_core/preprocessors.py index 8ccaa30..bf2bc96 100644 --- a/src/ibex_bluesky_core/preprocessors.py +++ b/src/ibex_bluesky_core/preprocessors.py @@ -1,7 +1,7 @@ """Bluesky plan preprocessors specific to ISIS.""" import logging -from typing import Generator +from collections.abc import Generator from bluesky import plan_stubs as bps from bluesky import preprocessors as bpp diff --git a/tests/callbacks/test_document_logging_callback.py b/tests/callbacks/test_document_logging_callback.py index a38cd38..8004826 100644 --- a/tests/callbacks/test_document_logging_callback.py +++ b/tests/callbacks/test_document_logging_callback.py @@ -1,8 +1,8 @@ # pyright: reportMissingParameterType=false import json +from collections.abc import Generator from pathlib import Path -from typing import Generator from unittest.mock import mock_open, patch import bluesky.plan_stubs as bps diff --git a/tests/test_run_engine.py b/tests/test_run_engine.py index b7f4f4c..0256dd5 100644 --- a/tests/test_run_engine.py +++ b/tests/test_run_engine.py @@ -1,7 +1,8 @@ # pyright: reportMissingParameterType=false import threading -from typing import Any, Generator +from collections.abc import Generator +from typing import Any from unittest.mock import MagicMock import bluesky.plan_stubs as bps From e30b3a7744d27caedfc611e007647ae71eb5543e Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Fri, 29 Nov 2024 21:20:21 +0000 Subject: [PATCH 4/6] How tight can we go before it gets too annoying? --- ruff.toml | 14 ++++++++++++-- src/ibex_bluesky_core/callbacks/document_logger.py | 2 +- src/ibex_bluesky_core/callbacks/file_logger.py | 4 ++-- .../callbacks/fitting/fitting_utils.py | 5 +++-- .../devices/simpledae/controllers.py | 4 ++-- tests/callbacks/test_write_log_callback.py | 8 ++++---- 6 files changed, 24 insertions(+), 13 deletions(-) diff --git a/ruff.toml b/ruff.toml index 5d17a91..3a11cde 100644 --- a/ruff.toml +++ b/ruff.toml @@ -2,6 +2,7 @@ line-length = 100 indent-width = 4 [lint] +preview = true extend-select = [ "N", # pep8-naming "D", # pydocstyle (can use this later but for now causes too many errors) @@ -21,6 +22,7 @@ extend-select = [ "LOG", # Logging-specific rules "G", # Logging-specific rules "UP", # Pyupgrade + "PL", # Rules from pylint ] ignore = [ "D406", # Section name should end with a newline ("{name}") @@ -28,12 +30,17 @@ ignore = [ "D213", # Incompatible with D212 "D203", # Incompatible with D211 "B901", # This is a standard, expected, pattern in bluesky + "PLR6301" # Too noisy ] [lint.per-file-ignores] "tests/*" = [ "N802", - "D", # Don't require method documentation for test methods - "ANN", # Don't require tests to use type annotations + "D", # Don't require method documentation for test methods + "ANN", # Don't require tests to use type annotations + "PLR2004", # Allow magic numbers in tests + "PLR0915", # Allow complex tests + "PLR0914", # Allow complex tests + "PLC2701", # Allow tests to import "private" things ] "doc/conf.py" = [ "D100" @@ -41,3 +48,6 @@ ignore = [ [lint.pep8-naming] extend-ignore-names = ["RE"] # Conventional name used for RunEngine + +[lint.pylint] +max-args = 6 diff --git a/src/ibex_bluesky_core/callbacks/document_logger.py b/src/ibex_bluesky_core/callbacks/document_logger.py index a11b0d5..77290cf 100644 --- a/src/ibex_bluesky_core/callbacks/document_logger.py +++ b/src/ibex_bluesky_core/callbacks/document_logger.py @@ -34,5 +34,5 @@ def __call__(self, name: str, document: dict[str, Any]) -> None: to_write: dict[str, Any] = {"type": name, "document": document} - with open(self.filename, "a") as outfile: + with open(self.filename, "a", encoding="utf8") as outfile: outfile.write(f"{json.dumps(to_write)}\n") diff --git a/src/ibex_bluesky_core/callbacks/file_logger.py b/src/ibex_bluesky_core/callbacks/file_logger.py index e059c91..6e2ec6f 100644 --- a/src/ibex_bluesky_core/callbacks/file_logger.py +++ b/src/ibex_bluesky_core/callbacks/file_logger.py @@ -65,7 +65,7 @@ def start(self, doc: RunStart) -> None: ) header_data[START_TIME] = formatted_time - with open(self.filename, "a", newline="") as outfile: + with open(self.filename, "a", newline="", encoding="utf-8") as outfile: for key, value in header_data.items(): outfile.write(f"{key}: {value}\n") @@ -102,7 +102,7 @@ def event(self, doc: Event) -> Event: else value ) - with open(self.filename, "a", newline="") as outfile: + with open(self.filename, "a", newline="", encoding="utf-8") as outfile: file_delimiter = "," if doc[SEQ_NUM] == 1: # If this is the first event, write out the units before writing event data. diff --git a/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py b/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py index 43eec3e..1193ba6 100644 --- a/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py +++ b/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py @@ -198,8 +198,9 @@ class Polynomial(Fit): @classmethod def _check_degree(cls, args: tuple[int, ...]) -> int: """Check that polynomial degree is valid.""" - degree = args[0] if args else 7 - if not (0 <= degree <= 7): + max_degree = 7 + degree = args[0] if args else max_degree + if not (0 <= degree <= max_degree): raise ValueError("The polynomial degree should be at least 0 and smaller than 8.") return degree diff --git a/src/ibex_bluesky_core/devices/simpledae/controllers.py b/src/ibex_bluesky_core/devices/simpledae/controllers.py index 771fe80..12fe774 100644 --- a/src/ibex_bluesky_core/devices/simpledae/controllers.py +++ b/src/ibex_bluesky_core/devices/simpledae/controllers.py @@ -83,7 +83,7 @@ async def start_counting(self, dae: "SimpleDae") -> None: await dae.controls.resume_run.trigger(wait=True, timeout=None) await wait_for_value( dae.run_state, - lambda v: v in [RunstateEnum.RUNNING, RunstateEnum.WAITING, RunstateEnum.VETOING], + lambda v: v in {RunstateEnum.RUNNING, RunstateEnum.WAITING, RunstateEnum.VETOING}, timeout=10, ) @@ -130,7 +130,7 @@ async def start_counting(self, dae: "SimpleDae") -> None: await dae.controls.begin_run.trigger(wait=True, timeout=None) await wait_for_value( dae.run_state, - lambda v: v in [RunstateEnum.RUNNING, RunstateEnum.WAITING, RunstateEnum.VETOING], + lambda v: v in {RunstateEnum.RUNNING, RunstateEnum.WAITING, RunstateEnum.VETOING}, timeout=10, ) diff --git a/tests/callbacks/test_write_log_callback.py b/tests/callbacks/test_write_log_callback.py index ad10861..b4b131b 100644 --- a/tests/callbacks/test_write_log_callback.py +++ b/tests/callbacks/test_write_log_callback.py @@ -26,7 +26,7 @@ def test_header_data_all_available_on_start(cb): cb.start(run_start) result = save_path / f"{run_start['uid']}.txt" - mock_file.assert_called_with(result, "a", newline="") + mock_file.assert_called_with(result, "a", newline="", encoding="utf-8") # time should have been renamed to start_time and converted to human readable mock_file().write.assert_any_call("start_time: 2024-10-04 14:43:43\n") mock_file().write.assert_any_call(f"uid: {uid}\n") @@ -67,7 +67,7 @@ def test_event_prints_header_with_units_and_respects_precision_of_value_on_first with patch("ibex_bluesky_core.callbacks.file_logger.open", mock_open()) as mock_file: cb.event(event) - mock_file.assert_called_with(cb.filename, "a", newline="") + mock_file.assert_called_with(cb.filename, "a", newline="", encoding="utf-8") first_call = call(f"\n{field_name}({units})\n") second_call = call(f"{expected_value:.{prec}f}\n") mock_file().write.assert_has_calls([first_call, second_call]) @@ -92,7 +92,7 @@ def test_event_prints_header_without_units_and_does_not_truncate_precision_if_no with patch("ibex_bluesky_core.callbacks.file_logger.open", mock_open()) as mock_file: cb.event(event) - mock_file.assert_called_with(cb.filename, "a", newline="") + mock_file.assert_called_with(cb.filename, "a", newline="", encoding="utf-8") mock_file().write.assert_has_calls([call("\ntest\n"), call("1.2345\n")]) assert mock_file().write.call_count == 2 @@ -118,7 +118,7 @@ def test_event_prints_header_only_on_first_event_and_does_not_truncate_if_not_fl with patch("ibex_bluesky_core.callbacks.file_logger.open", mock_open()) as mock_file: cb.event(second_event) - mock_file.assert_called_with(cb.filename, "a", newline="") + mock_file.assert_called_with(cb.filename, "a", newline="", encoding="utf-8") mock_file().write.assert_called_once_with(f"{expected_value}\n") assert mock_file().write.call_count == 1 From 5abe9dcf862045082407af6f59de08874c7f15a1 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Sat, 30 Nov 2024 08:54:02 +0000 Subject: [PATCH 5/6] Even more rules --- ruff.toml | 20 +++++++++++-------- .../callbacks/fitting/fitting_utils.py | 2 -- .../test_document_logging_callback.py | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ruff.toml b/ruff.toml index 3a11cde..6ae2f8e 100644 --- a/ruff.toml +++ b/ruff.toml @@ -5,15 +5,17 @@ indent-width = 4 preview = true extend-select = [ "N", # pep8-naming - "D", # pydocstyle (can use this later but for now causes too many errors) + "D", # pydocstyle "I", # isort (for imports) "E501", # Line too long ({width} > {limit}) - "E", - "F", - "W", - "ANN", + "E", # Pycodestyle errors + "W", # Pycodestyle warnings + "F", # Pyflakes + "PL", # Pylint + "B", # Flake8-bugbear + "PIE", # Flake8-pie + "ANN", # Annotations "ASYNC", # Asyncio-specific checks - "B", "NPY", # Numpy-specific rules "RUF", # Ruff-specific checks, include some useful asyncio rules "FURB", # Rules from refurb @@ -22,7 +24,8 @@ extend-select = [ "LOG", # Logging-specific rules "G", # Logging-specific rules "UP", # Pyupgrade - "PL", # Rules from pylint + "SLF", # Private-member usage + "PERF", # Performance-related rules ] ignore = [ "D406", # Section name should end with a newline ("{name}") @@ -34,13 +37,14 @@ ignore = [ ] [lint.per-file-ignores] "tests/*" = [ - "N802", + "N802", # Allow test names to be long / not pep8 "D", # Don't require method documentation for test methods "ANN", # Don't require tests to use type annotations "PLR2004", # Allow magic numbers in tests "PLR0915", # Allow complex tests "PLR0914", # Allow complex tests "PLC2701", # Allow tests to import "private" things + "SLF001", # Allow tests to use "private" things ] "doc/conf.py" = [ "D100" diff --git a/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py b/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py index 1193ba6..dee8187 100644 --- a/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py +++ b/src/ibex_bluesky_core/callbacks/fitting/fitting_utils.py @@ -30,7 +30,6 @@ def model(cls, *args: int) -> lmfit.Model: (x-values: NDArray, parameters: np.float64 -> y-values: NDArray) """ - pass @classmethod @abstractmethod @@ -47,7 +46,6 @@ def guess( (x-values: NDArray, y-values: NDArray -> parameters: Dict[str, lmfit.Parameter]) """ - pass @classmethod def fit(cls, *args: int) -> FitMethod: diff --git a/tests/callbacks/test_document_logging_callback.py b/tests/callbacks/test_document_logging_callback.py index 8004826..26c55e8 100644 --- a/tests/callbacks/test_document_logging_callback.py +++ b/tests/callbacks/test_document_logging_callback.py @@ -22,7 +22,7 @@ def basic_plan() -> Generator[Msg, None, None]: result: RunEngineResult = RE(basic_plan()) filepath = log_location / f"{result.run_start_uids[0]}.log" - for i in range(0, 2): + for i in range(2): assert m.call_args_list[i].args == (filepath, "a") # Checks that the file is opened 2 times, for open and then stop From e888b423e6b9156128bcd376b5aab5a9edd16de4 Mon Sep 17 00:00:00 2001 From: Tom Willemsen Date: Mon, 2 Dec 2024 09:45:33 +0000 Subject: [PATCH 6/6] Tabs -> spaces --- ruff.toml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/ruff.toml b/ruff.toml index 6ae2f8e..e8b272d 100644 --- a/ruff.toml +++ b/ruff.toml @@ -9,15 +9,15 @@ extend-select = [ "I", # isort (for imports) "E501", # Line too long ({width} > {limit}) "E", # Pycodestyle errors - "W", # Pycodestyle warnings + "W", # Pycodestyle warnings "F", # Pyflakes "PL", # Pylint - "B", # Flake8-bugbear - "PIE", # Flake8-pie + "B", # Flake8-bugbear + "PIE", # Flake8-pie "ANN", # Annotations - "ASYNC", # Asyncio-specific checks - "NPY", # Numpy-specific rules - "RUF", # Ruff-specific checks, include some useful asyncio rules + "ASYNC", # Asyncio-specific checks + "NPY", # Numpy-specific rules + "RUF", # Ruff-specific checks, include some useful asyncio rules "FURB", # Rules from refurb "ERA", # Commented-out code "PT", # Pytest-specific rules @@ -30,8 +30,8 @@ extend-select = [ ignore = [ "D406", # Section name should end with a newline ("{name}") "D407", # Missing dashed underline after section ("{name}") - "D213", # Incompatible with D212 - "D203", # Incompatible with D211 + "D213", # Incompatible with D212 + "D203", # Incompatible with D211 "B901", # This is a standard, expected, pattern in bluesky "PLR6301" # Too noisy ]