diff --git a/snakebids/app.py b/snakebids/app.py index d47ac430..b130f6a0 100644 --- a/snakebids/app.py +++ b/snakebids/app.py @@ -84,26 +84,26 @@ def _check_deprecations(self): "`SnakeBidsApp.parser` is deprecated and no longer has any effect. To " "modify the parser, use the new `bidsapp` module." ) - warnings.warn(msg, stacklevel=1) + warnings.warn(msg, stacklevel=3) if self.config is not None: msg = ( "`SnakeBidsApp.config` is deprecated and no longer has any effect. To " "modify the config, use the new `bidsapp` module." ) - warnings.warn(msg, stacklevel=1) + warnings.warn(msg, stacklevel=3) if self.args is not None: msg = ( "`SnakeBidsApp.args` is deprecated and no longer has any effect. To " "modify CLI arguments, use the new `bidsapp` module." ) - warnings.warn(msg, stacklevel=1) + warnings.warn(msg, stacklevel=3) if self.version is not None: msg = ( "`SnakeBidsApp.version` is deprecated and no longer has any effect. To " "explcitly set the app version, use the `snakebids.plugins.Version` " "plugin" ) - warnings.warn(msg, stacklevel=1) + warnings.warn(msg, stacklevel=3) def _get_args(self): args: dict[str, Any] = {} diff --git a/snakebids/plugins/cli_config.py b/snakebids/plugins/cli_config.py index 1949f44b..582a889a 100644 --- a/snakebids/plugins/cli_config.py +++ b/snakebids/plugins/cli_config.py @@ -10,7 +10,6 @@ from snakebids import bidsapp from snakebids.exceptions import ConfigError from snakebids.io.yaml import get_yaml_io -from snakebids.plugins.base import PluginBase def _make_underscore_dash_aliases(name: str) -> set[str]: @@ -60,7 +59,7 @@ def _find_type(name: str, *, yamlsafe: bool = True) -> type[Any]: @attrs.define -class CliConfig(PluginBase): +class CliConfig: """Configure CLI arguments directly in config. Arguments are provided in config in a dictionary stored under ``cli_key``. Each @@ -124,15 +123,10 @@ def add_cli_arguments( # a str to allow the edge case where it's already # been converted if "type" in arg: - try: - arg_dict = {**arg, "type": _find_type(str(arg["type"]))} - except KeyError as err: - msg = f"{arg['type']} is not available as a type for {name}" - raise TypeError(msg) from err + arg_dict = {**arg, "type": _find_type(str(arg["type"]))} else: arg_dict = arg - self.add_argument( - parser, + parser.add_argument( *_make_underscore_dash_aliases(name), **arg_dict, # type: ignore ) diff --git a/snakebids/plugins/component_edit.py b/snakebids/plugins/component_edit.py index b6f033dd..1f9aa53b 100644 --- a/snakebids/plugins/component_edit.py +++ b/snakebids/plugins/component_edit.py @@ -1,6 +1,7 @@ from __future__ import annotations import argparse +import math from pathlib import Path from typing import Any, Sequence @@ -30,12 +31,15 @@ def __call__( return for pair in values: - if "=" in pair: - # split it into key and value - key, value = pair.split("=", 1) - elif ":" in pair: - key, spec = pair.split(":", 1) - spec = spec.lower() + 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"]: @@ -45,8 +49,6 @@ def __call__( else: # The flag isn't recognized raise MisspecifiedCliFilterError(pair) - else: - raise MisspecifiedCliFilterError(pair) # assign into dictionary getattr(namespace, self.dest)[key] = value diff --git a/snakebids/plugins/snakemake.py b/snakebids/plugins/snakemake.py index f5702c09..bd85fa9a 100644 --- a/snakebids/plugins/snakemake.py +++ b/snakebids/plugins/snakemake.py @@ -288,7 +288,7 @@ def finalize_config(self, config: dict[str, Any]): try: prepare_bidsapp_output(config["output_dir"], self.force_output) except RunError as err: - print(err.msg) + print(err.msg, file=sys.stderr) sys.exit(1) self.cwd = config["output_dir"] root = Path() diff --git a/snakebids/tests/strategies.py b/snakebids/tests/strategies.py index 2f7cebe7..17d34c61 100644 --- a/snakebids/tests/strategies.py +++ b/snakebids/tests/strategies.py @@ -41,32 +41,33 @@ def nothing() -> Any: return st.nothing() # type: ignore -class PathStrategy(st.SearchStrategy[Path]): - def resolve(self) -> st.SearchStrategy[Path]: - ... - - -def paths(*, absolute: bool | None = None) -> PathStrategy: - valid_chars = st.characters( - min_codepoint=48, max_codepoint=122, whitelist_categories=["Ll", "Lu"] - ) - paths = st.lists(st.text(valid_chars)).map(lambda x: Path(*x)) +def paths( + *, + min_segments: int = 0, + max_segments: int | None = None, + absolute: bool | None = None, + resolve: bool = False, +) -> st.SearchStrategy[Path]: + valid_chars = st.characters(blacklist_characters=["/", "\x00"], codec="UTF-8") + paths = st.lists( + st.text(valid_chars, min_size=1), min_size=min_segments, max_size=max_segments + ).map(lambda x: Path(*x)) - def relative_paths(): - return paths.filter(lambda p: not p.is_absolute()) + relative_paths = paths.filter(lambda p: not p.is_absolute()) - def absolute_paths(): - return paths.map(lambda p: Path("/", p)) + absolute_paths = paths.map(lambda p: Path("/", p)) if absolute: - result = absolute_paths() + result = absolute_paths elif absolute is False: - result = relative_paths() + result = relative_paths else: - result = absolute_paths() | relative_paths() + result = absolute_paths | relative_paths - result.resolve = lambda: result.map(lambda p: p.resolve()) # type: ignore - return result # type: ignore + if resolve: + return result.map(lambda p: p.resolve()) + + return result def bids_entity( @@ -241,6 +242,8 @@ def input_configs( def inputs_configs( *, keys: st.SearchStrategy[str] | None = None, + min_size: int = 0, + max_size: int | None = 5, filters: bool = True, wildcards: bool = True, paths: bool = True, @@ -248,7 +251,8 @@ def inputs_configs( return st.dictionaries( keys if keys is not None else st.text(min_size=1), input_configs(filters=filters, wildcards=wildcards, paths=paths), - max_size=5, + min_size=min_size, + max_size=max_size, ) diff --git a/snakebids/tests/test_plugins/test_bidsargs.py b/snakebids/tests/test_plugins/test_bidsargs.py new file mode 100644 index 00000000..3c79f5c4 --- /dev/null +++ b/snakebids/tests/test_plugins/test_bidsargs.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +import itertools as it +from argparse import ArgumentParser +from typing import Iterable + +import pytest +from hypothesis import given +from hypothesis import strategies as st + +from snakebids.exceptions import ConfigError +from snakebids.plugins.bidsargs import BidsArgs + + +class _St: + positional_args = st.text().filter(lambda s: not s.startswith("-")) + analysis_level_choices = positional_args | st.iterables(positional_args) + + +class TestAddCliArguments: + @given( + analysis_level_choices=_St.positional_args, + analysis_level_choices_config=st.text(), + ) + def test_analysis_level_attributes_are_mutually_exclusive( + self, + analysis_level_choices: str | Iterable[str], + analysis_level_choices_config: str, + ): + with pytest.raises(ConfigError, match="cannot be simultaneously defined"): + BidsArgs( + analysis_level=True, + analysis_level_choices=analysis_level_choices, + analysis_level_choices_config=analysis_level_choices_config, + ).add_cli_arguments(ArgumentParser(), {}, {}) + + @given(analysis_level_choices=_St.positional_args) + def test_analysis_levels_can_be_directly_defined( + self, analysis_level_choices: str | Iterable[str] + ): + parser = ArgumentParser() + a1, a2 = it.tee(analysis_level_choices) + bidsargs = BidsArgs(analysis_level=True, analysis_level_choices=a1) + bidsargs.add_cli_arguments(parser, {}, {}) + for arg in a2: + nspc = parser.parse_args(["...", "...", arg]) + assert nspc.analysis_level == arg + + @given( + analysis_level_choices=_St.positional_args, + analysis_level_choices_config=st.text(), + ) + def test_analysis_levels_can_be_defined_in_config( + self, + analysis_level_choices: str | Iterable[str], + analysis_level_choices_config: str, + ): + parser = ArgumentParser() + choices = list(analysis_level_choices) + bidsargs = BidsArgs( + analysis_level=True, + analysis_level_choices_config=analysis_level_choices_config, + ) + bidsargs.add_cli_arguments(parser, {}, {analysis_level_choices_config: choices}) + for arg in choices: + nspc = parser.parse_args(["...", "...", arg]) + assert nspc.analysis_level == arg diff --git a/snakebids/tests/test_plugins/test_cli_config.py b/snakebids/tests/test_plugins/test_cli_config.py index 527ab633..ad50cacf 100644 --- a/snakebids/tests/test_plugins/test_cli_config.py +++ b/snakebids/tests/test_plugins/test_cli_config.py @@ -1,8 +1,10 @@ from __future__ import annotations import argparse +from pathlib import Path from typing import Literal +import more_itertools as itx import pytest from hypothesis import given from hypothesis import strategies as st @@ -35,6 +37,15 @@ def test_underscores_to_dashes( assert all(nm.__dict__.values()) assert len(nm.__dict__) == len(args) + @given(arg=st.text().filter(lambda s: not s.startswith("-") and len(s))) + def test_positional_args_added_without_conversion(self, arg: str): + cli = CliConfig() + parser = argparse.ArgumentParser() + config = {"parse_args": {arg: {"help": "..."}}} + cli.add_cli_arguments(parser, config) + nm = parser.parse_args(["..."]) + assert itx.one(nm.__dict__.values()) == "..." + def test_fails_if_undefined_type_given(self): cli = CliConfig() config = {"parse_args": {"--new-param": {"type": "UnheardClass"}}} @@ -64,6 +75,20 @@ def test_convert_arg_to_builtin(): assert isinstance(args.new_param, int) +def test_path_works_as_builtin(): + cli = CliConfig() + parser = argparse.ArgumentParser() + new_args = { + "--new-param": { + "help": "Generic Help Message", + "type": "Path", + } + } + cli.add_cli_arguments(parser, {"parse_args": new_args}) + args = parser.parse_args(["--new-param", "foo"]) + assert args.new_param == Path("foo") + + def test_non_serialiable_type_raises_error(): cli = CliConfig() parser = argparse.ArgumentParser() diff --git a/snakebids/tests/test_plugins/test_component_edit.py b/snakebids/tests/test_plugins/test_component_edit.py index ae2d1760..67f43941 100644 --- a/snakebids/tests/test_plugins/test_component_edit.py +++ b/snakebids/tests/test_plugins/test_component_edit.py @@ -8,10 +8,12 @@ from pathlib import Path from typing import Any, ClassVar +import pytest from hypothesis import given 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.tests.helpers import allow_function_scoped from snakebids.types import InputsConfig, OptionalFilter @@ -132,6 +134,50 @@ def test_none_filters(self, pybids_inputs: InputsConfig, flag: str): for key in pybids_inputs: assert args.__dict__[f"{comp_edit.PREFIX}.filter.{key}"]["entity"] is False + @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"} + ), + ) + def test_filter_with_bad_flag_errors(self, pybids_inputs: InputsConfig, flag: 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}"] for key in pybids_inputs] + ) + ) + + with pytest.raises( + MisspecifiedCliFilterError, + match=re.compile(rf"following filter provided.*entity:{re.escape(flag)}"), + ): + p.parse_args(argv) + + @given( + pybids_inputs=sb_st.inputs_configs(min_size=1, max_size=1), + filt=st.text().filter( + 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 + ): + 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}", filt] for key in pybids_inputs]) + ) + + with pytest.raises( + MisspecifiedCliFilterError, + match=re.compile(rf"following filter provided.*{re.escape(filt)}"), + ): + p.parse_args(argv) + @st.composite def two_configs( diff --git a/snakebids/tests/test_plugins/test_snakemake.py b/snakebids/tests/test_plugins/test_snakemake.py index d2942010..07b6c38d 100644 --- a/snakebids/tests/test_plugins/test_snakemake.py +++ b/snakebids/tests/test_plugins/test_snakemake.py @@ -1,5 +1,6 @@ from __future__ import annotations +import argparse import copy import tempfile from pathlib import Path @@ -8,12 +9,11 @@ import pytest from hypothesis import given from hypothesis import strategies as st -from pathvalidate import Platform, is_valid_filepath from pytest_mock import MockerFixture from pytest_mock.plugin import MockType from snakebids import bidsapp -from snakebids.exceptions import ConfigError +from snakebids.exceptions import ConfigError, RunError from snakebids.plugins import Version from snakebids.plugins.snakemake import ( CONFIGFILE_CHOICES, @@ -58,6 +58,18 @@ def test_errors_if_no_configfile_found(self, fakefs_tmpdir: Path): SnakemakeBidsApp(snakemake_dir=tmp, snakefile_path=Path()) +class TestAddCliArguments: + def test_snakemake_help_arg_added(self, mocker: MockerFixture): + from snakebids.plugins.snakemake import snakemake + + mock = mocker.patch.object(snakemake, "main") + smk = SnakemakeBidsApp.create_empty() + parser = argparse.ArgumentParser() + smk.add_cli_arguments(parser) + parser.parse_args(["--help-snakemake"]) + mock.assert_called_once_with(["-h"]) + + class TestResolvePath: @pytest.fixture def arg_dict(self) -> dict[str, str | list[str]]: @@ -181,9 +193,7 @@ def check_write_config( ) @given( - path=sb_st.paths() - .resolve() - .filter( + path=sb_st.paths(resolve=True).filter( lambda p: p != Path().resolve() and not str(p).startswith(str(Path("results").resolve())) ) @@ -239,7 +249,7 @@ def test_output_in_app_triggers_workflow_mode( root=Path("results"), ) - @given(tail=st.text().filter(lambda s: is_valid_filepath(s, Platform.LINUX))) + @given(tail=sb_st.paths(absolute=False)) @allow_function_scoped def test_output_under_results_triggers_workflow_mode( self, @@ -270,7 +280,7 @@ def test_output_under_results_triggers_workflow_mode( ) @given( - output=st.sampled_from(["", "results"]).map(Path) | sb_st.paths().resolve(), + output=st.sampled_from(["", "results"]).map(Path) | sb_st.paths(resolve=True), configfile=sb_st.paths(absolute=False), ) @allow_function_scoped @@ -304,7 +314,7 @@ def test_non_relative_configfile_is_preserved( assert smk.configfile_outpath == configfile @given( - output=st.sampled_from(["", "results"]).map(Path) | sb_st.paths().resolve(), + output=st.sampled_from(["", "results"]).map(Path) | sb_st.paths(resolve=True), configfile=sb_st.paths(), config_output=sb_st.paths(), ) @@ -322,6 +332,27 @@ def test_specified_configfile_output_always_preserved( assert smk.configfile_outpath == config_output + @given( + err=st.text(), output=sb_st.paths(absolute=True, min_segments=1, max_segments=1) + ) + @allow_function_scoped + def test_exits_on_run_error( + self, + mocker: MockerFixture, + capsys: pytest.CaptureFixture[str], + err: str, + output: Path, + ): + smk = SnakemakeBidsApp.create_empty() + mocks = get_io_mocks(mocker) + mocks["prepare_output"].side_effect = RunError(err) + config = {"output_dir": output.resolve()} + + with pytest.raises(SystemExit): + smk.finalize_config(config) + cap = capsys.readouterr() + assert err == cap.err[:-1] + class TestVersion: def test_get_app_version_no_package(