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

List arguments are not recognized in the context default_map #124

Closed
maxb2 opened this issue Nov 6, 2023 · 3 comments · Fixed by #129
Closed

List arguments are not recognized in the context default_map #124

maxb2 opened this issue Nov 6, 2023 · 3 comments · Fixed by #129
Labels
bug Something isn't working upstream

Comments

@maxb2
Copy link
Owner

maxb2 commented Nov 6, 2023

First discovered in #117

The example in #117 doesn't work (you get missing argument LABEL_NAMES) with typer-config.

However, if you make a similar click app, it works fine:

# app.py
import click

@click.command(context_settings = {"default_map": {"my_list": [1,2,3]}})
@click.option('--my-list', type=list)
@click.pass_context
def hello(ctx, my_list):
    print(my_list)
    print(ctx.get_parameter_source("my_list"))
    print(ctx.default_map)

if __name__ == '__main__':
    hello()

Then invoked, you get:

$ python app.py
[1, 2, 3]
ParameterSource.DEFAULT_MAP
{'my_list': [1, 2, 3]}

as expected.

@maxb2 maxb2 added the bug Something isn't working label Nov 6, 2023
@jlwhelan28
Copy link

Hi there @maxb2 I've been running into the same bug after finding your original solution here tiangolo/typer#86 (comment)

I also found this thread tiangolo/typer#518 which suggests adding a callback to the list-like Argument to set a default value. Modifying that non-eager callback to pull directly from ctx.default_map seems to work for this bug. The underlying issue seems to be the Argument list default loaded into ctx.default_map isn't making it to ctx.params, unlike the single value arguments and options which work fine. Using your original example

from typing import Optional
import typer
import yaml

app = typer.Typer( )

def argument_list_callback(ctx: typer.Context, param: typer.CallbackParam, value: Optional[list[str]]) -> list[str]:
    ctx.default_map = ctx.default_map or {}
    if param.name in ctx.default_map:
        default = ctx.default_map[param.name]
    else:
        default = []
    return value if value else default

def conf_callback(ctx: typer.Context, param: typer.CallbackParam, value: str):
    if value:
        typer.echo(f"Loading config file: {value}")
        try: 
            with open(value, 'r') as f:    # Load config file
                conf = yaml.safe_load(f)
            ctx.default_map = ctx.default_map or {}   # Initialize the default map
            ctx.default_map.update(conf)   # Merge the config dict into default_map
        except Exception as ex:
            raise typer.BadParameter(str(ex))
    return value

@app.command()
def main(
    arg1: str,
    arg2: list[str] = typer.Argument(default=None, callback=argument_list_callback),
    config: str = typer.Option("", callback=conf_callback, is_eager=True),
    opt1: str = typer.Option(...),
    opt2: str = typer.Option("hello"),
):
    typer.echo(f"{opt1} {opt2} {arg1}")
    typer.echo(f"{arg2}")


if __name__ == "__main__":
    app()
# temp.yaml
opt1: "apple"
opt2: "pear"
arg1: "lemon"
arg2: ["oak", "aspen", "maple"]
python app.py --config temp.yaml
> Loading config file: temp.yaml
> apple pear lemon
> ['oak', 'aspen', 'maple']
python app.py strawberry bear wolf snake tiger --config temp.yaml
> Loading config file: temp.yaml
> apple pear strawberry
> ['bear', 'wolf', 'snake', 'tiger']

The example doesn't work if you provide arg2 on CLI and arg1 in the config only, would have to have some sort of distinguishing types like in the multiple arguments example in the typer docs https://typer.tiangolo.com/tutorial/multiple-values/arguments-with-multiple-values/

@maxb2 maxb2 added the upstream label Nov 10, 2023
@maxb2
Copy link
Owner Author

maxb2 commented Nov 10, 2023

@jlwhelan28 thanks for the research! I'll add your argument_list_callback() function to this library if you don't mind (or you could do it if you want a free PR 😉). I don't think I can automatically fix this, but it would be nice to at least provide the callback so users don't have to copy/paste it.

@maxb2
Copy link
Owner Author

maxb2 commented Nov 17, 2023

See 1.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants