Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow builtins and resolved paths as cli types #352

Merged
merged 2 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to ruamel because pyyaml didn't support allowing only specified types to be serialized, it either tried to serialize everything (using weird custom python tags that snakemake would then refuse to read), or only serialize basic types (but we need it to specifically support Path). ruamel did support this.

If we switch to reading yaml with ruamel in the future, we could further take advantage of its round-trip comment preservation


[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]:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yamlsafe is unused, but leaving this more as a hint to the future that we may want to support more flexible parsing of types for different applications

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