From 8c1ac55bf1a4150fad1181ec34c489708cd4072b Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Thu, 9 May 2024 19:38:22 -0400 Subject: [PATCH] Allow regex filtering from the CLI Adds :match and :search as valid filter methods, each of which should take =VALUE. Expands error handling to give different error messages for different filter misspecifications. --- snakebids/exceptions.py | 11 -- snakebids/plugins/component_edit.py | 91 ++++++++++++---- .../tests/test_plugins/test_component_edit.py | 100 ++++++++++++++++-- 3 files changed, 158 insertions(+), 44 deletions(-) diff --git a/snakebids/exceptions.py b/snakebids/exceptions.py index 56dbc6df..0d532ffc 100644 --- a/snakebids/exceptions.py +++ b/snakebids/exceptions.py @@ -35,16 +35,5 @@ def __init__(self, duplicated_names: Iterable[str]): ) -class MisspecifiedCliFilterError(Exception): - """Raised when a magic CLI filter cannot be parsed.""" - - def __init__(self, misspecified_filter: str): - super().__init__( - "The following filter provided by the CLI could not be parsed: " - f"{misspecified_filter}. Filters must be of the form " - "{entity}={filter} or {entity}:{REQUIRED|OPTIONAL|NONE} (case-insensitive)." - ) - - class SnakebidsPluginError(Exception): """Exception raised when a Snakebids plugin encounters a problem.""" diff --git a/snakebids/plugins/component_edit.py b/snakebids/plugins/component_edit.py index 1f9aa53b..51fc898d 100644 --- a/snakebids/plugins/component_edit.py +++ b/snakebids/plugins/component_edit.py @@ -1,19 +1,55 @@ from __future__ import annotations import argparse -import math from pathlib import Path -from typing import Any, Sequence +from typing import Any, Sequence, cast import attrs from typing_extensions import override from snakebids import bidsapp -from snakebids.exceptions import MisspecifiedCliFilterError from snakebids.plugins.base import PluginBase from snakebids.types import OptionalFilter +class FilterParseError(Exception): + """Raised when a magic CLI filter cannot be parsed.""" + + def __init__(self, filter: str, msg: str): + super().__init__( + "The following filter provided by the CLI could not be parsed: " + f"'{filter}'.\n\t{msg}".expandtabs(4) + ) + + @classmethod + def invalid_spec(cls, filter: str, spec: str): + """Raise if spec not recognized.""" + return cls( + filter, + f"':{spec}' is not a valid filter method. Must be one of 'required', " + "'optional', 'none', 'match', or 'search'.", + ) + + @classmethod + def missing_value(cls, filter: str, key: str, spec: str): + """Raise if no value provided.""" + return cls( + filter, f"':{spec}' requires a value, specified as '{key}:{spec}=VALUE'." + ) + + @classmethod + def only_key(cls, filter: str, key: str): + """Raise if only a key provided.""" + return cls(filter, "Filters must be specified as ENTITY[:METHOD]=VALUE.") + + @classmethod + def unneeded_value(cls, filter: str, key: str, spec: str, value: str): + """Raise if value provided to a boolean filter method.""" + return cls( + filter, f"'{key}:{spec}' should not be given a value (got '={value}')" + ) + + class FilterParse(argparse.Action): """Class for parsing CLI filters in argparse.""" @@ -26,32 +62,43 @@ def __call__( values: str | Sequence[Any] | None, option_string: str | None = None, ): - setattr(namespace, self.dest, {}) + result = {} if not values: return + boolean_filters = { + "optional": OptionalFilter, + "required": True, + "any": True, + "none": False, + } for pair in values: - eq = pair.find("=") - col = pair.find(":") - delim = min(eq if eq >= 0 else math.inf, col if col >= 0 else math.inf) - if delim is math.inf: - raise MisspecifiedCliFilterError(pair) - key = pair[:delim] - value = pair[delim + 1 :] - if delim == col: - spec = value.lower() - if spec == "optional": - value = OptionalFilter - elif spec in ["required", "any"]: - value = True - elif spec == "none": - value = False + if "=" in pair: + # split it into key and value + key, value = cast("tuple[str, str]", pair.split("=", 1)) + else: + key = pair + value = None + if ":" in key: + key, spec = cast("tuple[str, str]", key.split(":", 1)) + spec = spec.lower() + if spec in boolean_filters: + if value is not None: + raise FilterParseError.unneeded_value(pair, key, spec, value) + value = boolean_filters[spec] + elif spec in {"match", "search"}: + if value is None: + raise FilterParseError.missing_value(pair, key, spec) + value = {spec: value} else: - # The flag isn't recognized - raise MisspecifiedCliFilterError(pair) + raise FilterParseError.invalid_spec(pair, spec) + if value is None: + raise FilterParseError.only_key(pair, key) # assign into dictionary - getattr(namespace, self.dest)[key] = value + result[key] = value + + setattr(namespace, self.dest, result) @attrs.define diff --git a/snakebids/tests/test_plugins/test_component_edit.py b/snakebids/tests/test_plugins/test_component_edit.py index 67f43941..f41f7f89 100644 --- a/snakebids/tests/test_plugins/test_component_edit.py +++ b/snakebids/tests/test_plugins/test_component_edit.py @@ -13,8 +13,7 @@ from hypothesis import strategies as st import snakebids.tests.strategies as sb_st -from snakebids.exceptions import MisspecifiedCliFilterError -from snakebids.plugins.component_edit import ComponentEdit +from snakebids.plugins.component_edit import ComponentEdit, FilterParseError from snakebids.tests.helpers import allow_function_scoped from snakebids.types import InputsConfig, OptionalFilter @@ -115,6 +114,33 @@ def test_optional_filters(self, pybids_inputs: InputsConfig, flag: str): is OptionalFilter ) + @given( + pybids_inputs=sb_st.inputs_configs(), + flag=st.from_regex( + re.compile(r"(?:match)|(?:search)", re.IGNORECASE), fullmatch=True + ), + value=st.text(), + ) + @allow_function_scoped + def test_regex_filter(self, pybids_inputs: InputsConfig, flag: str, value: str): + p = argparse.ArgumentParser() + comp_edit = ComponentEdit() + comp_edit.add_cli_arguments(p, {"pybids_inputs": pybids_inputs}) + argv = list( + it.chain.from_iterable( + [[f"--filter-{key}", f"entity:{flag}={value}"] for key in pybids_inputs] + ) + ) + + args = p.parse_args(argv) + for key in pybids_inputs: + assert ( + args.__dict__[f"{comp_edit.PREFIX}.filter.{key}"]["entity"][ + flag.lower() + ] + == value + ) + @given( pybids_inputs=sb_st.inputs_configs(), flag=st.from_regex(re.compile(r"none", re.IGNORECASE), fullmatch=True), @@ -137,10 +163,39 @@ def test_none_filters(self, pybids_inputs: InputsConfig, flag: str): @given( pybids_inputs=sb_st.inputs_configs(min_size=1, max_size=1), flag=st.text().filter( - lambda s: s.lower() not in {"none", "any", "optional", "required"} + lambda s: s.lower() + not in {"none", "any", "optional", "required", "match", "search"} + and "=" not in s ), + value=st.just("") | st.text().map(lambda s: f"={s}"), + ) + def test_filter_with_invalid_spec( + self, pybids_inputs: InputsConfig, flag: str, value: str + ): + p = argparse.ArgumentParser() + comp_edit = ComponentEdit() + comp_edit.add_cli_arguments(p, {"pybids_inputs": pybids_inputs}) + argv = list( + it.chain.from_iterable( + [[f"--filter-{key}", f"entity:{flag}{value}"] for key in pybids_inputs] + ) + ) + + with pytest.raises( + FilterParseError, + match=re.compile( + rf"':{re.escape(flag.lower())}' is not a valid filter method" + ), + ): + p.parse_args(argv) + + @given( + pybids_inputs=sb_st.inputs_configs(min_size=1, max_size=1), + flag=st.sampled_from(["match", "search"]), ) - def test_filter_with_bad_flag_errors(self, pybids_inputs: InputsConfig, flag: str): + def test_filter_with_missing_value_errors( + self, pybids_inputs: InputsConfig, flag: str + ): p = argparse.ArgumentParser() comp_edit = ComponentEdit() comp_edit.add_cli_arguments(p, {"pybids_inputs": pybids_inputs}) @@ -151,8 +206,33 @@ def test_filter_with_bad_flag_errors(self, pybids_inputs: InputsConfig, flag: st ) with pytest.raises( - MisspecifiedCliFilterError, - match=re.compile(rf"following filter provided.*entity:{re.escape(flag)}"), + FilterParseError, + match=re.compile(rf"':{re.escape(flag.lower())}' requires a value"), + ): + p.parse_args(argv) + + @given( + pybids_inputs=sb_st.inputs_configs(min_size=1, max_size=1), + flag=st.sampled_from(["none", "any", "optional", "required"]), + value=st.text(), + ) + def test_boolean_filter_with_value_errors( + self, pybids_inputs: InputsConfig, flag: str, value: str + ): + p = argparse.ArgumentParser() + comp_edit = ComponentEdit() + comp_edit.add_cli_arguments(p, {"pybids_inputs": pybids_inputs}) + argv = list( + it.chain.from_iterable( + [[f"--filter-{key}", f"entity:{flag}={value}"] for key in pybids_inputs] + ) + ) + + with pytest.raises( + FilterParseError, + match=re.compile( + rf"'entity:{re.escape(flag.lower())}' should not be given a value" + ), ): p.parse_args(argv) @@ -162,9 +242,7 @@ def test_filter_with_bad_flag_errors(self, pybids_inputs: InputsConfig, flag: st lambda s: "=" not in s and ":" not in s and not s.startswith("-") ), ) - def test_filters_with_no_delimiter_errors( - self, pybids_inputs: InputsConfig, filt: str - ): + def test_filters_with_only_key_errors(self, pybids_inputs: InputsConfig, filt: str): p = argparse.ArgumentParser() comp_edit = ComponentEdit() comp_edit.add_cli_arguments(p, {"pybids_inputs": pybids_inputs}) @@ -173,8 +251,8 @@ def test_filters_with_no_delimiter_errors( ) with pytest.raises( - MisspecifiedCliFilterError, - match=re.compile(rf"following filter provided.*{re.escape(filt)}"), + FilterParseError, + match=re.compile(r"Filters must be specified as"), ): p.parse_args(argv)