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: Allow nested output namespaces #5954

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Apr 3, 2023

Up till now, the following was not possible:

@calcfunction
def add(x, y):
    return {'nested.output': x + y}

An exception would be raised by plumpy because the nested output namespace, defined by the ProcessSpec that is automatically generated from the function signature, does not contain the port output. The automatically generated process spec does mark the outputs namespace as dynamic, but this was not being applied recursively.

Not only is this functionality for users, the Parser.parse_from_node functonality is currently broken if the Parser returns output nodes in nested namespaces. The reason is that the parse_from_node creates a calcfunction on-the-fly which raises when it gets the outputs with the nested labels. Even though the original Process class may have specified these nested output namespaces, the on-the-fly calcfunction to capture the manual re-parsing does not support this.

The addition of this functionality requires a change in plumpy.

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.

@sphuber thanks! I reviewed it yesterday, and since the test is passed, should be all fine.

@sphuber
Copy link
Contributor Author

sphuber commented Apr 4, 2023

Thanks @unkcpz, I will just quickly add something to the documentation before merging this

@sphuber sphuber force-pushed the feature/process-functions-nested-outputs branch 2 times, most recently from 02d62af to c877ac3 Compare April 4, 2023 08:28
Up till now, the following was not possible:

    @calcfunction
    def add(x, y):
        return {'nested.output': x + y}

An exception would be raised by `plumpy` because the `nested` output
namespace, defined by the `ProcessSpec` that is automatically generated
from the function signature, does not contain the port `output`. The
automatically generated process spec _does_ mark the outputs namespace
as dynamic, but this was not being applied recursively.

Not only is this functionality for users, the `Parser.parse_from_node`
functonality is currently broken if the `Parser` returns output nodes in
nested namespaces. The reason is that the `parse_from_node` creates a
`calcfunction` on-the-fly which raises when it gets the outputs with the
nested labels. Even though the original `Process` class may have
specified these nested output namespaces, the on-the-fly calcfunction to
capture the manual re-parsing does not support this.

The addition of this functionality requires a change in `plumpy` which
was released with `plumpy==0.21.6` which is therefore made the minimum
required version.

This version also includes another fix where the recently introduced
check on the type of the return value of a workchain conditional
predicate, was changed from raising an exception, to logging a
deprecation warning. The correspondig test is updated to check that the
user warning is emitted instead of catching the exception.
@sphuber sphuber force-pushed the feature/process-functions-nested-outputs branch from c877ac3 to 5169803 Compare April 4, 2023 08:31
@sphuber sphuber merged commit 1ed957e into aiidateam:main Apr 4, 2023
@sphuber sphuber deleted the feature/process-functions-nested-outputs branch April 4, 2023 09:14
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.

2 participants