Skip to content

Commit

Permalink
Addressed comments by Sebastiaan.
Browse files Browse the repository at this point in the history
In particular, the most important changes include:
- now the auto-group flag is called `--auto-group`, is
  a flag and is False by default
- only kept `--include` and `--exclude` options, checking
  typestrings and allowing to end with `%`. One benefit
  is that while reimplementing I replaced some `isinstance`
  with string comparisons, with potential further benefits.
- `--include` and `--exclude` are now mutually exclusive
- improved documentation of the main point of the issue
  • Loading branch information
giovannipizzi committed Apr 3, 2020
1 parent 43d852e commit ba4b217
Show file tree
Hide file tree
Showing 9 changed files with 478 additions and 198 deletions.
71 changes: 28 additions & 43 deletions aiida/cmdline/commands/cmd_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ def update_environment(argv):
sys.path = _path


def validate_entrypoint_string_or_all(ctx, param, value, allow_all=True): # pylint: disable=unused-argument,invalid-name
"""Validate that `value` is a valid entrypoint string or the string 'all'."""
def validate_entrypoint_string(ctx, param, value): # pylint: disable=unused-argument,invalid-name
"""Validate that `value` is a valid entrypoint string."""
from aiida.orm import autogroup

try:
autogroup.Autogroup.validate(value, allow_all=allow_all)
autogroup.Autogroup.validate(value)
except Exception as exc:
raise click.BadParameter(str(exc) + ' ({})'.format(value))

Expand All @@ -55,58 +55,41 @@ def validate_entrypoint_string_or_all(ctx, param, value, allow_all=True): # pyl
@verdi.command('run', context_settings=dict(ignore_unknown_options=True,))
@click.argument('scriptname', type=click.STRING)
@click.argument('varargs', nargs=-1, type=click.UNPROCESSED)
@click.option('--group/--no-group', default=True, show_default=True, help='Enables the autogrouping')
@click.option('--auto-group', is_flag=True, help='Enables the autogrouping')
@click.option(
'-l',
'--group-label-prefix',
'--auto-group-label-prefix',
type=click.STRING,
required=False,
help='Specify the prefix of the label of the auto group (numbers might be automatically '
'appended to generate unique names per run)'
'appended to generate unique names per run).'
)
@click.option(
'-n',
'--group-name',
type=click.STRING,
required=False,
help='Specify the name of the auto group [DEPRECATED, USE --group-label-prefix instead]'
help='Specify the name of the auto group [DEPRECATED, USE --auto-group-label-prefix instead]. '
'This also enables auto-grouping.'
)
@click.option(
'-e',
'--exclude',
cls=MultipleValueOption,
default=lambda: [],
help='Exclude these classes from auto grouping (use full entrypoint strings)',
callback=functools.partial(validate_entrypoint_string_or_all, allow_all=False)
default=None,
help='Exclude these classes from auto grouping (use full entrypoint strings).',
callback=functools.partial(validate_entrypoint_string)
)
@click.option(
'-i',
'--include',
cls=MultipleValueOption,
default=lambda: ['all'],
help='Include these classes from auto grouping (use full entrypoint strings or "all")',
callback=validate_entrypoint_string_or_all
)
@click.option(
'-E',
'--excludesubclasses',
cls=MultipleValueOption,
default=lambda: [],
help='Exclude these classes and their sub classes from auto grouping (use full entrypoint strings)',
callback=functools.partial(validate_entrypoint_string_or_all, allow_all=False)
)
@click.option(
'-I',
'--includesubclasses',
cls=MultipleValueOption,
default=lambda: [],
help='Include these classes and their sub classes from auto grouping (use full entrypoint strings)',
callback=functools.partial(validate_entrypoint_string_or_all, allow_all=False)
default=None,
help='Include these classes from auto grouping (use full entrypoint strings or "all").',
callback=validate_entrypoint_string
)
@decorators.with_dbenv()
def run(
scriptname, varargs, group, group_label_prefix, group_name, exclude, excludesubclasses, include, includesubclasses
):
def run(scriptname, varargs, auto_group, auto_group_label_prefix, group_name, exclude, include):
# pylint: disable=too-many-arguments,exec-used
"""Execute scripts with preloaded AiiDA environment."""
from aiida.cmdline.utils.shell import DEFAULT_MODULES_LIST
Expand All @@ -126,20 +109,22 @@ def run(
globals_dict['{}'.format(alias)] = getattr(__import__(app_mod, {}, {}, model_name), model_name)

if group_name:
warnings.warn('--group-name is deprecated, use `--group-label` instead', AiidaDeprecationWarning) # pylint: disable=no-member
if group_label:
raise click.BadParameter('You cannot specify both --group-name and --group-label; use --group-label only')
group_label = group_name

if group:
warnings.warn('--group-name is deprecated, use `--auto-group-label-prefix` instead', AiidaDeprecationWarning) # pylint: disable=no-member
if auto_group_label_prefix:
raise click.BadParameter(
'You cannot specify both --group-name and --auto-group-label-prefix; use --group-label only'
)
auto_group_label_prefix = group_name
# To have the old behavior, with auto-group enabled.
auto_group = True

if auto_group:
aiida_verdilib_autogroup = autogroup.Autogroup()
# if group_label_prefix is None, use autogenerated name
if group_label_prefix is not None:
aiida_verdilib_autogroup.set_group_label_prefix(group_label_prefix)
# Set the ``group_label_prefix`` if defined, otherwise a default prefix will be used
if auto_group_label_prefix is not None:
aiida_verdilib_autogroup.set_group_label_prefix(auto_group_label_prefix)
aiida_verdilib_autogroup.set_exclude(exclude)
aiida_verdilib_autogroup.set_include(include)
aiida_verdilib_autogroup.set_exclude_with_subclasses(excludesubclasses)
aiida_verdilib_autogroup.set_include_with_subclasses(includesubclasses)

# Note: this is also set in the exec environment! This is the intended behavior
autogroup.CURRENT_AUTOGROUP = aiida_verdilib_autogroup
Expand Down
Loading

0 comments on commit ba4b217

Please sign in to comment.