The Wayback Machine - https://web.archive.org/web/20210204190309/https://github.com/pallets/click/issues/1324
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

Passing an instance of Pathlib.Path to CliRunner.invoke() gets a TypeError #1324

Closed
alexwlchan opened this issue May 27, 2019 · 3 comments
Closed

Comments

@alexwlchan
Copy link

@alexwlchan alexwlchan commented May 27, 2019

Consider the following example:

from pathlib import Path

import click
from click.testing import CliRunner


@click.command()
@click.argument("DIRNAME")
def print_directory(dirname):
    print(dirname)


runner = CliRunner()
print(runner.invoke(print_directory, ["example"]))
print(runner.invoke(print_directory, [Path("example")]))

With Python 3.6.4 and click 7.0, this is the output you get:

<Result okay>
<Result TypeError("object of type 'PosixPath' has no len()",)>

This can be easily fixed by wrapping in str(), i.e.,

print(runner.invoke(print_directory, [str(Path("example"))]))

But in general I've got used to the implicit contract (at least in the standard library) that I can use a Path object anywhere I can use a string representation of a path. Since paths are often the arguments to command-line tools, it'd be nice to be able to use a Path object here as well.

Alternatively, raise a more helpful TypeError if you pass a non-string object into invoke() – I spent a while poking around in my code to see where I was calling len() on the wrong object before realising it was actually coming from click.

@alexwlchan alexwlchan changed the title Passing an instance of Pathlib.Path to the test runner gets a TypeError Passing an instance of Pathlib.Path to CliRunner.invoke() gets a TypeError May 27, 2019
@joshuastorck
Copy link
Contributor

@joshuastorck joshuastorck commented May 31, 2019

This code change would solve your problem in core.BaseCommand.main:

        if args is None:
            args = get_os_args()
        else:
            args = [arg.__fspath__() if hasattr(arg, "__fspath__") else arg
                    for arg in args]

However, that would still not make support for other types work, such as integers. In general, the parser assumes all inputs are strings. To properly handle this, a pre-processing step would have to be done to determine which arguments on the command line are associated with which types. However, this adds a lot of complexity, since command line arguments like dashes need to be handled. This would be a huge overhaul to the parser and is probably not a good idea.

I suggest closing this issue and just using the noted workaround.

@davidism davidism closed this May 31, 2019
@alexwlchan
Copy link
Author

@alexwlchan alexwlchan commented May 31, 2019

I think it’s reasonable to say “we can’t coerce all arguments to strings”, but the confusing error message remains.

Would it be possible to add a check something like:

if not all(isinstance(arg, string_types) for arg in args):
    raise TypeError(“args must all be strings; got %r”)

to make it more obvious when somebody has made this mistake?

@davidism
Copy link
Member

@davidism davidism commented May 31, 2019

If the documentation is unclear that the invoke-related APIs expect strings, that's something I'm ok with changing, but otherwise I think this is just expected behavior. In general, and I know this is somewhat contradictory given that Click does a bunch of validation, Python shouldn't be checking the types of arguments.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.