Traceback when CLI option has bad default could be more helpful

effigies commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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)

pallets / click