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

Add the concept of a "lazy" PortNamespace #121

Merged
merged 2 commits into from
Jun 28, 2019

Conversation

sphuber
Copy link
Collaborator

@sphuber sphuber commented Jun 26, 2019

Fixes #117

The current design will skip the validation of a PortNamespace if it
is not required and no values are passed for it upon process creation.
However, before validation, the inputs are pre-processed. This entails
that the input dictionary is mapped onto the input port namespace and
any input that is not explicitly specified, but corresponds to a port
with a default, has the field populated with said default value.

This behavior, however, now makes it impossible to skip the validation
of a non-required port namespace by not specifying any inputs, as soon
as it contains any port with a default. Even if no inputs are passed,
the pre-processing will populate the defaults, which will trigger the
validation because the inputs for the namespace are no longer empty.

To make it possible to have non-required namespaces, that contain nested
ports with defaul values, optionally validated, we introduce a new port
namespace property: "lazy". If a PortNamespace is "lazy" the pre
processing will only be triggered if there are explicit inputs defined
for it. If this is not the case, the population of defaults is skipped.

@sphuber sphuber force-pushed the fix_117_lazy_port_namespaces branch from 3710be6 to 1a6e8db Compare June 27, 2019 13:03
The current design will skip the validation of a `PortNamespace` if it
is not required *and* no values are passed for it upon process creation.
However, before validation, the inputs are pre-processed. This entails
that the input dictionary is mapped onto the input port namespace and
any input that is not explicitly specified, but corresponds to a port
with a default, has the field populated with said default value.

This behavior, however, now makes it impossible to skip the validation
of a non-required port namespace by not specifying any inputs, as soon
as it contains any port with a default. Even if no inputs are passed,
the pre-processing will populate the defaults, which will trigger the
validation because the inputs for the namespace are no longer empty.

To make it possible to have non-required namespaces, that contain nested
ports with default values, optionally validated, we introduce a new port
namespace property: `populate_defaults`. If a `PortNamespace` has the
`populate_defaults` set to `False` the pre processing will only be
triggered if there are explicit inputs defined for it. If this is not the
case, the population of defaults is skipped.
@sphuber sphuber force-pushed the fix_117_lazy_port_namespaces branch from 1a6e8db to 1fb6627 Compare June 27, 2019 15:38
@sphuber
Copy link
Collaborator Author

sphuber commented Jun 27, 2019

OK @giovannipizzi , the port namespace property has been renamed to populate_defaults and has been documented.

:param dynamic: boolean, if True, the namespace will accept values even when no explicit port is defined
:param populate_defaults: boolean, if False, the pre-processing step does not populate defaults, also not of any
nested ports, if no value was explicitly specified for this namespace.
"""
Copy link
Member

Choose a reason for hiding this comment

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

I would just add an explicit note: ".. note: if the port is explicitly specified, this parameter is ignored and the defaults are populated"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that is what it says, but maybe I can reword it, so that it more clear

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have changed the docstring. I think it should be clear now

Copy link
Member

@giovannipizzi giovannipizzi 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 sphuber merged commit 99c100a into develop Jun 28, 2019
@sphuber sphuber deleted the fix_117_lazy_port_namespaces branch June 28, 2019 06:40
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.

Allow for 'lazy' pre processing of port namespaces
2 participants