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

"out" does not support creation of nested port namespaces #134

Open
ltalirz opened this issue Nov 6, 2019 · 2 comments
Open

"out" does not support creation of nested port namespaces #134

ltalirz opened this issue Nov 6, 2019 · 2 comments

Comments

@ltalirz
Copy link
Member

ltalirz commented Nov 6, 2019

This logic here:

plumpy/plumpy/processes.py

Lines 1133 to 1139 in 253e8d2

namespace = output_port.split(namespace_separator)
port_name = namespace.pop()
if namespace:
port_namespace = self.spec().outputs.get_port(namespace_separator.join(namespace))
else:
port_namespace = self.spec().outputs

does not allow the automatic creation of port namespaces.

Consider somebody calls self.out("output_files.structures.relaxed", node) where port "output_files" was already defined (dynamic), and port namespace "structures" is to be created on the fly.

Should this not be allowed?

Anyhow, I guess this would only really make sense if port namespaces were dynamic by default.

@sphuber
Copy link
Collaborator

sphuber commented Nov 6, 2019

This intentional, self.out is just to map an output onto the pre-defined output namespace, not to create namespaces. The definition/creation of namespaces is done in the spec. The syntax of self.out, however, allows for the "key" to be nested, i.e. to be able to do something like self.out('some.subspace.x', x) instead of something more verbose self.get_output_ports()['some']['subspace'].set('x', x). Additionally, the dynamic just applies to that namespace that it is applied to, so in your example you say that output_files can contain any number of SinglefileData. It doesn't mean that it can contain other PortNamespaces as well, who in turn also allow SinglefileData (and other PortNamespaces recursively...)

To come back to your example, even if the desired behavior was implemented, you would not even have to explicitly define the output_files space. You could just set self.spec().outputs.valid_type = SinglefileData and call self.out('output_files.structures.relaxed', node), right? I am not sure that having the valid types of a namespace automatically apply downwards to all the sub spaces it contains. What if you do this:

spec.input_namespace('namespace.pseudos', valid_type='UpfData')
spec.inputs['namespace'].valid_type = StructureData

should this now also add StructureData as a valid type for the nested namespace.pseudos?

@ltalirz
Copy link
Member Author

ltalirz commented Nov 6, 2019

I am not sure that having the valid types of a namespace automatically apply downwards to all the sub spaces it contains.

Yeah, I think I agree that port namespaces should not be created on the fly for the time being.

I wonder whether one could still improve the error message

+-> ERROR at 2019-11-06 14:54:23.824215+00:00
 | invalid value uuid: 195e7d96-cc9e-43f5-9313-6aba474a50a2 (unstored) specified with label output_files.Cu111.traj: port 'Cu111' does not exist in port namespace 'output_files'

so that it at least says "port namespace" Cu111 does not exist (instead of "port").

I think here one is really looking for a port namespace only, not for a port:

plumpy/plumpy/processes.py

Lines 1136 to 1137 in 253e8d2

if namespace:
port_namespace = self.spec().outputs.get_port(namespace_separator.join(namespace))

One could either pass an argument is_namespace to the get_port function or have another functionget_port_namespace that throws an appropriate exception if the port does not exist (and also validates that it is a namespace, if it does exist).

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

No branches or pull requests

2 participants