From 8c1ac55bf1a4150fad1181ec34c489708cd4072b Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Thu, 9 May 2024 19:38:22 -0400 Subject: [PATCH 1/2] 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) From 2d4da359d4a6ee3789763e35d7035770dc0fb972 Mon Sep 17 00:00:00 2001 From: Peter Van Dyken Date: Sat, 13 Jul 2024 12:41:18 -0400 Subject: [PATCH 2/2] Update documentation with new filter syntax --- snakebids/plugins/__init__.py | 1 + snakebids/plugins/__init__.pyi | 2 ++ snakebids/plugins/component_edit.py | 29 +++++++++++++++---- .../tests/test_plugins/test_component_edit.py | 10 ++----- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/snakebids/plugins/__init__.py b/snakebids/plugins/__init__.py index 85600ae3..9b0713a4 100644 --- a/snakebids/plugins/__init__.py +++ b/snakebids/plugins/__init__.py @@ -20,6 +20,7 @@ "CliConfig", "ComponentEdit", "FilterParse", + "FilterParseError", "InvalidBidsError", "Pybidsdb", "SnakemakeBidsApp", diff --git a/snakebids/plugins/__init__.pyi b/snakebids/plugins/__init__.pyi index 50a17fa1..76368913 100644 --- a/snakebids/plugins/__init__.pyi +++ b/snakebids/plugins/__init__.pyi @@ -7,6 +7,7 @@ from .cli_config import ( from .component_edit import ( ComponentEdit, FilterParse, + FilterParseError, ) from .pybidsdb import ( Pybidsdb, @@ -29,6 +30,7 @@ __all__ = [ "CliConfig", "ComponentEdit", "FilterParse", + "FilterParseError", "InvalidBidsError", "Pybidsdb", "SnakemakeBidsApp", diff --git a/snakebids/plugins/component_edit.py b/snakebids/plugins/component_edit.py index 51fc898d..818b1a0f 100644 --- a/snakebids/plugins/component_edit.py +++ b/snakebids/plugins/component_edit.py @@ -10,6 +10,7 @@ from snakebids import bidsapp from snakebids.plugins.base import PluginBase from snakebids.types import OptionalFilter +from snakebids.utils.utils import text_fold class FilterParseError(Exception): @@ -111,6 +112,16 @@ class ComponentEdit(PluginBase): arguments are read and used to update the original component specification within config. + Filters are specified on the CLI using ``ENTITY[:METHOD][=VALUE]``, as follows: + + 1. ``ENTITY=VALUE`` selects paths based on an exact value match. + 2. ``ENTITY:match=REGEX`` and ``ENTITY:search=REGEX`` selects paths using regex + with :func:`re.match` and :func:`re.search` respectively. This syntax can be used + to select multiple values (e.g. ``'session:match=01|02'``). + 3. ``ENTITY:required`` selects all paths with the entity, regardless of value. + 4. ``ENTITY:none`` selects all paths without the entity. + 5. ``ENTITY:any`` removes filters for the entity. + CLI arguments created by this plugin cannot be overriden. Parameters @@ -139,9 +150,17 @@ def add_cli_arguments( # create filter parsers, one for each input_type filter_opts = parser.add_argument_group( "BIDS FILTERS", - "Filters to customize PyBIDS get() as key=value pairs, or as " - "key:{REQUIRED|OPTIONAL|NONE} (case-insensitive), to enforce the presence " - "or absence of values for that key.", + text_fold( + """ + Update filters for input components. Each filter can be specified as a + ENTITY=VALUE pair to select an value directly. To use regex filtering, + ENTITY:match=REGEX or ENTITY:search=REGEX can be used for re.match() or + re.search() respectively. Regex can also be used to select multiple + values, e.g. 'session:match=01|02'. ENTITY:required and ENTITY:none can + be used to require or prohibit presence of an entity in selected paths, + respectively. ENTITY:optional can be used to remove a filter. + """ + ), ) for input_type in pybids_inputs: @@ -155,7 +174,7 @@ def add_cli_arguments( nargs="+", action=FilterParse, dest=f"{self.PREFIX}.filter.{input_type}", - metavar="ENTITY=VALUE", + metavar="ENTITY[:METHOD][=VALUE]", help=f"(default: {' '.join(arglist_default)})", ) @@ -164,7 +183,7 @@ def add_cli_arguments( # create wildcards parsers, one for each input_type wildcards_opts = parser.add_argument_group( "INPUT WILDCARDS", - "File path entities to use as wildcards in snakemake", + "Provide entities to be used as wildcards.", ) for input_type in pybids_inputs: diff --git a/snakebids/tests/test_plugins/test_component_edit.py b/snakebids/tests/test_plugins/test_component_edit.py index f41f7f89..76ca725c 100644 --- a/snakebids/tests/test_plugins/test_component_edit.py +++ b/snakebids/tests/test_plugins/test_component_edit.py @@ -183,9 +183,7 @@ def test_filter_with_invalid_spec( with pytest.raises( FilterParseError, - match=re.compile( - rf"':{re.escape(flag.lower())}' is not a valid filter method" - ), + match=re.compile(r"is not a valid filter method"), ): p.parse_args(argv) @@ -207,7 +205,7 @@ def test_filter_with_missing_value_errors( with pytest.raises( FilterParseError, - match=re.compile(rf"':{re.escape(flag.lower())}' requires a value"), + match=re.compile(rf"':{flag}' requires a value"), ): p.parse_args(argv) @@ -230,9 +228,7 @@ def test_boolean_filter_with_value_errors( with pytest.raises( FilterParseError, - match=re.compile( - rf"'entity:{re.escape(flag.lower())}' should not be given a value" - ), + match=re.compile(rf"'entity:{flag}' should not be given a value"), ): p.parse_args(argv)