AttributeError in BoolParamType when using ctx.default_map for configuration

[ichard26]

Hello Click. I am a contributor of Black, which is a Python code formatter that is heavily opinionated. This might not be a bug and this might actually be a duplicate of #1553. I am just reraising the same issue with more details and information.

---

Black supports reading configuration from a pyproject.toml file. The CLI has a --config option to pass the path to the pyproject.toml file. This option has a callback to a read_pyproject_toml function which takes the filepath (or looks for it automatically if it wasn't explicitly given) and reads the Black configuration from the file. Once having read the configuration. Black injects them into ctx.default_map. This works really nicely to have a tiered configuration system where CLI arguments override configuration from a pyproject.toml file.

Below is a minimal example of the code causing the AttributeError.

import click


def read_pyproject_toml(ctx, param, value):
    """Inject Black configuration from "pyproject.toml" into defaults in `ctx`."""
    # Let's just pretend that a pyproject.toml file was found and read
    values = {"skip_string_normalization": 1}
    ctx.default_map = values


@click.command()
@click.option(
    "-S",
    "--skip-string-normalization",
    is_flag=True,
    help="Don't normalize string quotes or prefixes.",
)
@click.option(
    "--config",
    is_eager=True,
    callback=read_pyproject_toml,
    help="Read configuration from PATH.",
)
def main(skip_string_normalization: bool):
    pass


if __name__ == "__main__":
    main()

Actual Behavior

The problem is that when Click passes the argument to the main function it first converts the argument to a bool. That conversion fails with an exception in the Click core.

Traceback (most recent call last):
  File "attributeerror.py", line 29, in <module>
    main()
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 716, in main
    with self.make_context(prog_name, args, **extra) as ctx:
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 641, in make_context
    self.parse_args(ctx, args)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 940, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 1469, in handle_parse_result
    value = self.full_process_value(ctx, value)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 1790, in full_process_value
    return Parameter.full_process_value(self, ctx, value)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 1438, in full_process_value
    value = self.process_value(ctx, value)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 1428, in process_value
    return self.type_cast_value(ctx, value)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 1417, in type_cast_value
    return _convert(value, (self.nargs != 1) + bool(self.multiple))
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/core.py", line 1415, in _convert
    return self.type(value, self, ctx)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/types.py", line 39, in __call__
    return self.convert(value, param, ctx)
  File "/home/richard-26/.local/lib/python3.8/site-packages/click/types.py", line 350, in convert
    value = value.lower()
AttributeError: 'int' object has no attribute 'lower'

Expected Behavior

Ideally Click would display a helpful error message to the user instead of causing an AttributeError exception. Something like this:

richard-26@ubuntu-laptop:~/programming/black$ python attributeerror.py 
Usage: attributeerror.py [OPTIONS]
Try "attributeerror.py --help" for help.

Error: Invalid value for "-S" / "--skip-string-normalization": 0 is not a valid boolean

Except, then that raises the issue that a string version of a number being indistinguishable from its int or float version... Click could display a specialized error message for integers like The number 0 is not a valid boolean, but IMO that's a bit overkill and would become a slippery slope.

Alternative Solution

BoolParamType could accept 1 and 0 as a valid boolean since "1" and "0" are already accepted as boolean.

click/src/click/types.py

Lines 390 to 393 in da7bf34

	if value in ("true", "t", "1", "yes", "y"):
	return True
	elif value in ("false", "f", "0", "no", "n"):
	return False

Although, this would be a backward-incompatible change.

---

In the end. this is kinda a problem since the exception's traceback points to the Click core. Black has an issue reported with the same exception -> psf/black#1458. The exception isn't understandable to users as it looks like something is wrong with Click when their Black configuration is just wrong.

Regardless, thank you for taking the time to read this (possibly worthless) bug report. And for maintaining this great package. Black and many of my personal projects use Click as their CLI toolkit. If I am missing any information about Black's use case or something else, please feel free to ask!

Environment

• Python version: Python 3.8.1
• OS version: Ubuntu 18.04.4 LTS
• Click version: 7.1.1

[davidism]

Ok, so it looks like you do some parsing on input (config) data when reading from TOML, where Click is only expecting input data to be strings.

There's been a similar discussion in WTForms about accepting the Python type as well as strings. We should check that there's not a good reason for that distinction in Click, but I'm open to accepting it.

In the mean time, could you read the config as strings and let Click do the processing?

[ichard26]

I have done a quick test and it appears to be a non-breaking change. I'll have to do more extensive testing of all of the different configuration options though. Thanks for the quick reply!

EDIT: I've just removed my wording on my confusion since it didn't make sense in the end

[ichard26]

After further testing, it should be fine and your recommendation has been merged. Thanks!

I am closing this issue as it is no longer necessary.