CLI overrides in projects cause conflict with unset optional positional arguments
- Dominant language
- Python
- Stars
- 33.9k
- Forks
- 4.7k
- Avg merge
- 3m
- Merged PRs (30d)
- 1
Description
## How to reproduce the behaviour
The project templates allow [overriding variables](https://spacy.io/usage/projects#custom-scripts) via the CLI as `--vars.foo`, which uses the same mechanism we use for CLI config overrides. Under the hood, it works by setting the following on the `typer` CLI decorator, which will capture any additional arguments. We then parse those extra values using a helper function that interprets them and maps them to `key`/`value` pairs.
```python
context_settings={"allow_extra_args": True, "ignore_unknown_options": True}
```
In the project CLI, this causes a problem with optional positional arguments – in this case, the `project_dir`, which defaults to the current working directory. If no project dir is provided and CLI overrides are set, the first override is interpreted as the positional argument. To work around this, the project directory has to be specified explicitly as `.`.
### Example
```yaml
vars:
foo: bar
commands:
- name: test
script:
- echo ${vars.foo}
```
This raises an error:
```bash
python -m spacy project run test --vars.foo baz
```
```
Error: Invalid value for '[PROJECT_DIR]': Directory '--vars.foo' does not exist.
```
This works as expected:
```bash
python -m spacy project run test . --vars.foo baz
```
Since this all happens at the level of `typer` when the arguments are provided (and before anything even gets to our helper functions), we can't easily solve this within our helpers. It's also pretty reasonable behaviour on `typer`/`click`'s part IMO, since the logic for allowing extra args is really only supposed to capture _extras_ (and it's us who use this feature in a more abstract way for additional arguments).
Making the `project_dir` required isn't really an option, because this would break the otherwise very convenient workflows. However, we could just accept this edge case and add a note to the docs that you need the `.` if you use CLI overrides.
I can't immediately think of an elegant solution, though, if we actually wanted to fix this in the code. We could remove the `exists` check from the `project_dir` arg, make it accept a string and then check whether whatever the CLI received as that positional argument `startswith("--")`. If so, we know it must be a CLI variable override and can't be the actual project directory, so we set `project_dir` to `Path.cwd()` and add the value starting with `--` back to the overrides. This is a bit hacky, though, and we'll lose the automated CLI-level check that verifies that the project directory exists.
Contributor guide
Assessment
This issue has not been assessed yet.