Skip to content

Commit

Permalink
Allow builtins and resolved paths as cli types
Browse files Browse the repository at this point in the history
The architecture for type conversion in the argparse integration was
intended for using pathlib.Path, but had no flexibility even for
builtin types like int

Here, remove the old `globals()` lookup, which exposed too much
internal api. Instead, attempt to lookup simple strings as builtins, and
strings with dots as resolved module imports (e.g.
`module.submodule.type`). `Path` is kept as an exception, so using
`type: Path` will still convert the argument into a path

ruamel is now used for serialization, as pyyaml didn't support
serialization of specific python classes (e.g.  Path) without
restricting other classes: it was all or nothing. ruamel is safe by
default, and allows specifically opting in to representing different
classes

Clarify in docs how Path should be used for all paths to avoid relative
path issues

Resolves #294
  • Loading branch information
pvandyken committed Jan 16, 2024
1 parent c3bba2f commit 6ce4e2c
Show file tree
Hide file tree
Showing 15 changed files with 350 additions and 109 deletions.
23 changes: 22 additions & 1 deletion docs/bids_app/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,28 @@ A mapping from the name of each ``analysis_level`` to the list of rules or files

### `parse_args`

A dictionary of command-line parameters to make available as part of the BIDS app. Each item of the mapping is passed to [argparse's add_argument function](#argparse.ArgumentParser.add_argument). A number of default entries are present in a new snakebids project's config file that structure the BIDS app's CLI, but additional command-line arguments can be added as necessary.
A dictionary of command-line parameters to make available as part of the BIDS app. Each item of the mapping is passed to [argparse's `add_argument` function](#argparse.ArgumentParser.add_argument). A number of default entries are present in a new snakebids project's config file that structure the BIDS app's CLI, but additional command-line arguments can be added as necessary.

As in [`ArgumentParser.add_argument()`](#argparse.ArgumentParser.add_argument), `type` may be used to convert the argument to the specified type. It may be set to any type that can be serialized into yaml, for instance, `str`, `int`, `float`, and `boolean`.

```yaml
parse_args:
--a-string:
help: args are string by default
--a-path:
help: |
A path pointing to data needed for the pipeline. These are still converted
into strings, but are first resolved into absolute paths (see below)
type: Path
--another-path:
help: This type annotation does the same thing as above
type: pathlib.Path
--a-number:
help: A number important for the analysis
type: float
```

When CLI parameters are used to collect paths, `type` should be set to [`Path`](#pathlib.Path) (or [`pathlib.Path`](#pathlib.Path)). These arguments will still be serialized as strings (since yaml doesn't have a path type), but snakebids will automatically resolve all arguments into absolute paths. This is important to prevent issues with snakebids and relative paths.


### `debug`
Expand Down
115 changes: 96 additions & 19 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ snakemake = [
{ version = ">=5.28.0,<8", python = ">=3.8" },
{ version = ">=7.18.2,<8", python = ">=3.11" },
]
PyYAML = ">=6"
typing-extensions = ">=3.10.0"
attrs = ">=21.2.0"
boutiques = "^0.5.25"
Expand Down Expand Up @@ -70,6 +69,7 @@ copier = ">=8.2.0"
jinja2-time = ">=0.2.0"
# minimum 2.31.0 because of security vulnerability
requests = ">=2.31.0"
ruamel-yaml = ">=0.17.2"

[tool.poetry.group.dev.dependencies]
pytest = "^7.0.0"
Expand Down
4 changes: 2 additions & 2 deletions snakebids/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@
parse_snakebids_args,
)
from snakebids.exceptions import ConfigError, RunError
from snakebids.io.config import write_config
from snakebids.types import OptionalFilter
from snakebids.utils.output import (
prepare_bidsapp_output,
write_config_file,
write_output_mode,
)
from snakebids.utils.utils import DEPRECATION_FLAG, to_resolved_path
Expand Down Expand Up @@ -234,7 +234,7 @@ def run_snakemake(self) -> None:
app = plugin(app) or app

# Write the config file
write_config_file(
write_config(
config_file=new_config_file,
data=dict(
app.config,
Expand Down
31 changes: 24 additions & 7 deletions snakebids/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
import snakemake
from typing_extensions import override

from snakebids.exceptions import MisspecifiedCliFilterError
from snakebids.exceptions import ConfigError, MisspecifiedCliFilterError
from snakebids.io.yaml import get_yaml_io
from snakebids.types import InputsConfig, OptionalFilter
from snakebids.utils.utils import to_resolved_path

Expand Down Expand Up @@ -204,6 +205,26 @@ def create_parser(include_snakemake: bool = False) -> argparse.ArgumentParser:
return parser


def _find_type(name: str, *, yamlsafe: bool = True) -> type[Any]:
import importlib

if name == "Path":
return Path
*module_name, obj_name = name.split(".") if "." in name else ("builtins", name)
try:
type_ = getattr(importlib.import_module(".".join(module_name)), obj_name)
except (ImportError, AttributeError) as err:
msg = f"{name} could not be resolved"
raise ConfigError(msg) from err
if not callable(type_):
msg = f"{name} cannot be used as a type"
raise ConfigError(msg)
if yamlsafe and type_ not in get_yaml_io().representer.yaml_representers:
msg = f"{name} cannot be serialized into yaml"
raise ConfigError(msg)
return type_


def add_dynamic_args(
parser: argparse.ArgumentParser,
parse_args: Mapping[str, Any],
Expand All @@ -220,16 +241,12 @@ def add_dynamic_args(
# a str to allow the edge case where it's already
# been converted
if "type" in arg:
try:
arg_dict = {**arg, "type": globals()[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
app_group.add_argument(
*_make_underscore_dash_aliases(name),
**arg_dict,
**arg_dict, # type: ignore
)

# general parser for
Expand Down
Loading

0 comments on commit 6ce4e2c

Please sign in to comment.