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

Traceback when CLI option has bad default could be more helpful #1749

Closed
effigies opened this issue Jan 6, 2021 · 2 comments · Fixed by #1805
Closed

Traceback when CLI option has bad default could be more helpful #1749

effigies opened this issue Jan 6, 2021 · 2 comments · Fixed by #1805
Assignees
Milestone

Comments

@effigies
Copy link

effigies commented Jan 6, 2021

Expected Behavior

In the following command, I add an option --multiple with the metadata multiple=True, but the default False instead of [False].

Ideally, the traceback should indicate that the --multiple option has a bad default.

#!/usr/bin/env python
import click

@click.command()
@click.option("--multiple", multiple=True, default=False, show_default=True)
def cli(debug):
    pass

if __name__ == '__main__':
    cli()

Actual Behavior

Tell us what happens instead.

Traceback (most recent call last):
  File "./test_click.py", line 12, in <module>
    cli()
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 1025, in __call__
    return self.main(*args, **kwargs)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 954, in main
    with self.make_context(prog_name, args, **extra) as ctx:
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 852, in make_context
    self.parse_args(ctx, args)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 1261, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 2078, in handle_parse_result
    value, source = self.consume_value(ctx, opts)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 2509, in consume_value
    value, source = super().consume_value(ctx, opts)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 1975, in consume_value
    value = self.get_default(ctx)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 2453, in get_default
    return super().get_default(ctx, call=call)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 1957, in get_default
    return self.type_cast_value(ctx, value)
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 2007, in type_cast_value
    return _convert(value, (self.nargs != 1) + bool(self.multiple))
  File "/home/user/anaconda3/envs/pybids-pre/lib/python3.8/site-packages/click/core.py", line 2005, in _convert
    return tuple(_convert(x, level - 1) for x in value)
TypeError: 'bool' object is not iterable

I discovered this while running tests on pre-released dependencies, picking up click 8.0.0a1, which no longer accepts the above. The offending case was in a subcommand with many options, so it was not as obvious as it would be with a good error message.

Environment

  • Python version: 3.8
  • Click version: 8.0.0a1
@Saif807380
Copy link
Contributor

I'm from MLH and I'm thinking of working on this issue, however I'm unsure of what type does default take when multiple is True or when nargs is set. Does it expect list/tuple or basically any iterable? Also will just putting the return statement causing the TypeError inside a try block suffice or do I add checks before returning?

@davidism
Copy link
Member

davidism commented Mar 4, 2021

multiple or nargs expect a list/tuple. multiple and nargs together expect a list/tuple of lists/tuples, although that's not particularly important here. For now any iterable should be allowed, that code is checking all value sources, not only the default. A try block sounds fine, but I would isolate checking that it's iterable from doing the conversion, since the ParamType might raise TypeError for its own reasons:

try:
    iter_value = iter(value)
except TypeError:
    raise TypeError(...)

return tuple(... for x in iter_value)

@davidism davidism added this to the 8.0.0 milestone Mar 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants