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

Make argument type error messages use __str__ method of Predicates #317

Merged
merged 2 commits into from Apr 7, 2017
Merged

Make argument type error messages use __str__ method of Predicates #317

merged 2 commits into from Apr 7, 2017

Conversation

gmarceau
Copy link
Contributor

@gmarceau gmarceau commented Apr 7, 2017

Previous behavior looks like this

Error: Argument of output expected to be <plumbum.cli.switches.Predicate object at 0x10ce91c10>, not 'requirements.txt':
        ValueError('output is a file, should be a directory',)

now looks like this

    Error: Argument of output expected to be 'MakeDirectory', not 'requirements.txt':
        ValueError('output is a file, should be a directory',)

when checking a Predicate.

@henryiii
Copy link
Collaborator

henryiii commented Apr 7, 2017

Would you mind changing this to use "{0} {1}".format() style or "{val} {other}".format() style? I'd prefer that going forward.

@@ -350,7 +350,7 @@ def _handle_argument(val, argtype, name):
except (TypeError, ValueError):
ex = sys.exc_info()[1] # compat
raise WrongArgumentType("Argument of %s expected to be %r, not %r:\n %r" % (
name, argtype, val, ex))
name, str(argtype), val, ex))
Copy link
Collaborator

@henryiii henryiii Apr 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like:

raise WrongArgumentType("Argument of {name} expected to be {argtype}, not {val!r}:\n    {ex!r}".format(
                    name=name, argtype=argtype, val=val, ex=ex))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works

@henryiii
Copy link
Collaborator

henryiii commented Apr 7, 2017

As soon as Travis finishes, I'll accept.

@gmarceau
Copy link
Contributor Author

gmarceau commented Apr 7, 2017

We've been using a ton of plumbum at bodylabs. It is replacing both our usages of argparse and of Bash, no less. Thank you for creating it.

@coveralls
Copy link

coveralls commented Apr 7, 2017

Coverage Status

Coverage decreased (-0.4%) to 80.899% when pulling 619b354 on bodylabs:use_predicate___str__ into 67b25df on tomerfiliba:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 80.899% when pulling 619b354 on bodylabs:use_predicate___str__ into 67b25df on tomerfiliba:master.

@coveralls
Copy link

coveralls commented Apr 7, 2017

Coverage Status

Coverage remained the same at 81.258% when pulling 9d241e4 on bodylabs:use_predicate___str__ into 67b25df on tomerfiliba:master.

@henryiii henryiii merged commit 843b21e into tomerfiliba:master Apr 7, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 80.899% when pulling 9d241e4 on bodylabs:use_predicate___str__ into 67b25df on tomerfiliba:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 80.899% when pulling 9d241e4 on bodylabs:use_predicate___str__ into 67b25df on tomerfiliba:master.

@coveralls
Copy link

coveralls commented Apr 7, 2017

Coverage Status

Coverage decreased (-0.4%) to 80.899% when pulling 9d241e4 on bodylabs:use_predicate___str__ into 67b25df on tomerfiliba:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants