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

Restore behavior of JobCalculation.get_option returning default value #2013

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Oct 1, 2018

Fixes #2012

Recently, the various methods to get options from a JobCalculation,
the historically called get_* methods, would return a default value
if the value was not explicitly set as an attribute. These methods were
recently replaced by the generic get_option method and the
JobCalculation.options dictionary, however, by default the get_option
would only return a value if explicitly set. One would have to set the
argument only_actually_set to False to get the default value specified
in JobCalculation.options, if it is defined and not None.

The business logic however, was always calling get_option with default
argument while expecting the default to be returned when not explicitly
set. In addition, certain options whose get methods used to return a
default no longer had a default defined, e.g. append_text.

Here we properly define defaults for the options that always had one as
defined by the old explicit get methods. In addition, the default for
the only_actually_set for get_option is changed to False. The
internal code in general always expects to get defaults even when not
set. Note that this required the code for get_builder_restart to be
adapted as that expects only those options to be set that were actually
defined by the user when constructing the JobCalculation.

Recently, the various methods to get `options` from a `JobCalculation`,
the historically called `get_*` methods, would return a default value
if the value was not explicitly set as an attribute. These methods were
recently replaced by the generic `get_option` method and the
`JobCalculation.options` dictionary, however, by default the `get_option`
would only return a value if explicitly set. One would have to set the
argument `only_actually_set` to `False` to get the default value specified
in `JobCalculation.options`, if it is defined and not `None`.

The business logic however, was always calling `get_option` with default
argument while expecting the default to be returned when not explicitly
set. In addition, certain options whose get methods used to return a
default no longer had a default defined, e.g. `append_text`.

Here we properly define defaults for the options that always had one as
defined by the old explicit get methods. In addition, the default for
the `only_actually_set` for `get_option` is changed to `False`. The
internal code in general always expects to get defaults even when not
set. Note that this required the code for `get_builder_restart` to be
adapted as that expects only those options to be set that were actually
defined by the user when constructing the `JobCalculation`.
@coveralls
Copy link

Coverage Status

Coverage increased (+1.2%) to 67.81% when pulling 24434ea on sphuber:fix_2012_job_calculation_get_option_default into 36d6aef on aiidateam:develop.

@sphuber sphuber merged commit 5d273c1 into aiidateam:develop Oct 1, 2018
@sphuber sphuber deleted the fix_2012_job_calculation_get_option_default branch October 12, 2018 14:37
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.

None yet

3 participants