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

Param decorators assume attrs[key] is not None just because key in attrs #1959

Closed
janluke opened this issue Jun 16, 2021 · 2 comments · Fixed by #1965
Closed

Param decorators assume attrs[key] is not None just because key in attrs #1959

janluke opened this issue Jun 16, 2021 · 2 comments · Fixed by #1965
Milestone

Comments

@janluke
Copy link
Contributor

janluke commented Jun 16, 2021

The following lines cause a TypeError if one provide help=None:

if "help" in option_attrs:
option_attrs["help"] = inspect.cleandoc(option_attrs["help"])

By the way, why is cleandoc called by the decorator and not by Option itself? The docstring says "all keyword arguments are forwarded unchanged (except cls)".

The same problem appears when passing cls=None (also with @argument):

OptionClass = option_attrs.pop("cls", Option)
_param_memo(f, OptionClass(param_decls, **option_attrs))

I encountered this problem while writing a wrapper for the above decorators that explicitly list arguments for a better IDE support.

@janluke janluke changed the title @option calls cleandoc(help) even if help is None Passing help=None to @option causes a TypeError Jun 16, 2021
@janluke janluke changed the title Passing help=None to @option causes a TypeError Param decorators shouldn't assume attrs[key] is not None just because key in attrs Jun 16, 2021
@janluke janluke changed the title Param decorators shouldn't assume attrs[key] is not None just because key in attrs Param decorators assume attrs[key] is not None just because key in attrs Jun 16, 2021
@davidism
Copy link
Member

davidism commented Jul 4, 2021

By the way, why is cleandoc called by the decorator and not by Option itself? The docstring says "all keyword arguments are forwarded unchanged (except cls)".

Good question, I've noticed this before as well. I think it's because cleandoc is mostly useful when getting the doc from a decorated function's docstring, not for manually written strings. Let's explore changing that with an 8.1.0 PR, to see if there are any unexpected side effects.

Can you open a separate issue for that?

@janluke
Copy link
Contributor Author

janluke commented Jul 4, 2021

I think it's because cleandoc is mostly useful when getting the doc from a decorated function's docstring, not for manually written strings

But option.help is never taken from the docstring. Probably it was put there for similarity with @command.

Let's explore changing that with an 8.1.0 PR, to see if there are any unexpected side effects.

There shouldn't be any problem in moving this processing into Option. I'll check it out.

Can you open a separate issue for that?

Yep.

@davidism davidism modified the milestones: 8.1.0, 8.0.2 Jul 4, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 2022
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.

2 participants