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

Process functions: Parse docstring to set input port help attribute #5919

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Mar 7, 2023

Fixes #5896
Fixes #5918

The docstring-parser package is used to parse the docstring of a
process function if one is present. If the parsing is successful the
description of the function arguments is used to set the help
attribute of the corresponding InputPort that is added to the
dynamically generated process specification.

This is especially useful when the process function is exposed in a
wrapping workchain as now the description of the inputs is inherited
from the docstring and becomes immediately available on the process spec
of the workchain. This allows a user to inspect the inputs description
without having to go look at the source code of the function.

@sphuber sphuber force-pushed the feature/5918/process-functions-infer-help-from-docstring branch 4 times, most recently from 5cc4539 to 8b46bed Compare March 16, 2023 16:40
@sphuber
Copy link
Contributor Author

sphuber commented Mar 16, 2023

This is ready for review. @unkcpz @chrisjsewell @superstar54

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

Thanks @sphuber. I am curious if there are many params and by accident the user carelessly put a typo for the docstring of one parameter, if the whole parsing will fail and show empty or the parser still works for valid params docstring? For instance what will happened to the docstring below?

    @calcfunction
    def function(param_a, param_b, param_c):  # pylint: disable=unused-argument
        """Some documentation.
        :param param_a: Some description.
        :param param_b Fantastic docstring.
        """

def raise_exception():
raise RuntimeError()

monkeypatch.setattr(docstring_parser, 'parse', lambda x: raise_exception())
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
monkeypatch.setattr(docstring_parser, 'parse', lambda x: raise_exception())
monkeypatch.setattr(docstring_parser, 'parse', lambda _: raise_exception())

@sphuber
Copy link
Contributor Author

sphuber commented Mar 17, 2023

Thanks @sphuber. I am curious if there are many params and by accident the user carelessly put a typo for the docstring of one parameter, if the whole parsing will fail and show empty or the parser still works for valid params docstring? For instance what will happened to the docstring below?

In your example, the parser thinks the parameter definitions are actually "just" the long description:

In [14]: parsed = parse(function.__doc__)

In [15]: parsed.params
Out[15]: []

In [16]: parsed.long_description
Out[16]: ':param param_a: Some description.\n:param param_b Fantastic docstring.'

In [17]: parsed.short_description
Out[17]: 'Some documentation.'

So the parser doesn't raise an exception nor emit a warning. This is kind of tricky, since I am not sure how to reliably detect whether the docstring was "correctly" parsed. Just checking whether parsed.params is empty won't help, because maybe the docstring didn't provide any params, and so it was not a parser failure. If you have ideas, please let me know.

Currently a warning is only printed if the parsing really excepts, but I have tried creating docstrings that cause the parser to except and I haven't managed. I think it is designed to never except. So not sure how to detect whether parsing was successful.

@unkcpz
Copy link
Member

unkcpz commented Mar 17, 2023

Currently a warning is only printed if the parsing really excepts, but I have tried creating docstrings that cause the parser to except and I haven't managed. I think it is designed to never except. So not sure how to detect whether parsing was successful.

Thanks for checking. I think warnings for docstring would frighten users, at least I personally didn't write proper docstring especially aways, so maybe just show nothing.

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

The lambda is not a mandatory change. All looks to me.

sphuber added 2 commits March 17, 2023 14:13
The `docstring-parser` package is used to parse the docstring of a
process function if one is present. If the parsing is successful the
description of the function arguments is used to set the `help`
attribute of the corresponding `InputPort` that is added to the
dynamically generated process specification.

This is especially useful when the process function is exposed in a
wrapping workchain as now the description of the inputs is inherited
from the docstring and becomes immediately available on the process spec
of the workchain. This allows a user to inspect the inputs description
without having to go look at the source code of the function.
This functionality was already supported, given that process functions
automatically have a `Process` class generated from the function
signature, but it was never documented explicitly. It now also mentions
that inferred `valid_type` and `help` attributes on the input ports as
inferred from the type hints and docstrings, respectively, are
preserverd when exposing. An explicit test has been added for this.
@sphuber sphuber force-pushed the feature/5918/process-functions-infer-help-from-docstring branch from 8b46bed to a38886d Compare March 17, 2023 13:14
@sphuber sphuber merged commit 0bc8ef0 into aiidateam:main Mar 17, 2023
@sphuber sphuber deleted the feature/5918/process-functions-infer-help-from-docstring branch March 17, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants