From b22120c35db0a7edbe0a8bf0b10181256f32e442 Mon Sep 17 00:00:00 2001 From: Evan Bolyen Date: Tue, 28 Nov 2017 16:39:41 -0700 Subject: [PATCH 1/2] ENH: support collection types This adds support for List/Set collection types from the framework. --- q2cli/cache.py | 4 +- q2cli/commands.py | 18 ++-- q2cli/core.py | 37 +++++++ q2cli/handlers.py | 221 +++++++++++++++++++++++----------------- q2cli/tests/test_cli.py | 27 +++++ 5 files changed, 208 insertions(+), 99 deletions(-) diff --git a/q2cli/cache.py b/q2cli/cache.py index db5d74c1..3277c9a5 100644 --- a/q2cli/cache.py +++ b/q2cli/cache.py @@ -277,13 +277,13 @@ def _get_action_state(self, action): sig = action.signature for name, spec in itertools.chain(sig.signature_order.items(), sig.outputs.items()): - data = {'name': name, 'repr': repr(spec.qiime_type)} + data = {'name': name, 'repr': repr(spec.qiime_type), + 'ast': spec.qiime_type.to_ast()} if name in sig.inputs: type = 'input' elif name in sig.parameters: type = 'parameter' - data['ast'] = spec.qiime_type.to_ast() else: type = 'output' data['type'] = type diff --git a/q2cli/commands.py b/q2cli/commands.py index 4a0a5881..c15f4d8d 100644 --- a/q2cli/commands.py +++ b/q2cli/commands.py @@ -147,19 +147,25 @@ def __init__(self, name, plugin, action): def build_generated_handlers(self): import q2cli.handlers + handler_map = { + 'input': q2cli.handlers.ArtifactHandler, + 'parameter': q2cli.handlers.parameter_handler_factory, + 'output': q2cli.handlers.ResultHandler + } + handlers = collections.OrderedDict() for item in self.action['signature']: item = item.copy() type = item.pop('type') - if type == 'input': - handler = q2cli.handlers.ArtifactHandler - elif type == 'parameter': - handler = q2cli.handlers.parameter_handler_factory + if item['ast']['type'] == 'collection': + inner_handler = handler_map[type](**item) + handler = q2cli.handlers.CollectionHandler(inner_handler, + **item) else: - handler = q2cli.handlers.ResultHandler + handler = handler_map[type](**item) - handlers[item['name']] = handler(**item) + handlers[item['name']] = handler return handlers diff --git a/q2cli/core.py b/q2cli/core.py index f30e5a95..ea68ffda 100644 --- a/q2cli/core.py +++ b/q2cli/core.py @@ -106,3 +106,40 @@ def decorator(f): return click.option(*param_decls, **attrs)(f) return decorator + + +class MultipleType(click.ParamType): + """This is just a wrapper, it doesn't do anything on its own""" + def __init__(self, param_type): + self.param_type = param_type + + @property + def name(self): + return "MULTIPLE " + self.param_type.name + + def convert(self, value, param, ctx): + # Don't convert anything yet. + return value + + def fail(self, *args, **kwargs): + return self.param_type.fail(*args, **kwargs) + + def get_missing_message(self, *args, **kwargs): + return self.param_type.get_missing_message(*args, **kwargs) + + def get_metavar(self, *args, **kwargs): + metavar = self.param_type.get_metavar(*args, **kwargs) + if metavar is None: + return None + return "MULTIPLE " + metavar + + +class ResultPath(click.Path): + def __init__(self, repr, *args, **kwargs): + super().__init__(*args, **kwargs) + self.repr = repr + + def get_metavar(self, param): + if self.repr != 'Visualization': + return "PATH ARTIFACT " + self.repr + return "PATH VISUALIZATION" diff --git a/q2cli/handlers.py b/q2cli/handlers.py index 843e5223..33792083 100644 --- a/q2cli/handlers.py +++ b/q2cli/handlers.py @@ -109,9 +109,13 @@ def _parse_boolean(self, string): ctx = click.get_current_context() ctx.exit(1) - def _add_description(self, option): + def _add_description(self, option, requirement): if self.description: - option.help += '\n%s' % self.description + option.help += '%s %s' % (self.description, requirement) + elif option.help: + option.help += ' ' + requirement + else: + option.help = requirement return option @@ -268,31 +272,95 @@ def fail(*_): class GeneratedHandler(Handler): - def __init__(self, name, repr, default=NoDefault, description=None): + def __init__(self, name, repr, ast, default=NoDefault, description=None): super().__init__(name, prefix=self.prefix, default=default, description=description) self.repr = repr + self.ast = ast + + +class CollectionHandler(GeneratedHandler): + view_map = { + 'List': list, + 'Set': set + } + + def __init__(self, inner_handler, **kwargs): + self.inner_handler = inner_handler + # inner_handler needs to be set first so the prefix lookup works + super().__init__(**kwargs) + self.view_type = self.view_map[self.ast['name']] + + @property + def prefix(self): + return self.inner_handler.prefix + + def get_click_options(self): + import q2cli.core + for option in self.inner_handler.get_click_options(): + option.multiple = True + # validation happens on a callback for q2cli.core.Option, so unset + # it because we need standard click behavior for multi-options + # without this, the result of not-passing a value is `None` instead + # of `()` which confuses ._locate_value + option.callback = None + option.type = q2cli.core.MultipleType(option.type) + yield option + + def get_value(self, arguments, fallback=None): + args = self._locate_value(arguments, fallback, multiple=True) + if args is None: + return None + + decoded_values = [] + for arg in args: + # Use an empty dict because we don't need the inner handler to + # look for anything; that's our job. We just need it to decode + # whatever it was we found. + empty = collections.defaultdict(lambda: None) + decoded = self.inner_handler.get_value(empty, + fallback=lambda *_: arg) + decoded_values.append(decoded) + + value = self.view_type(decoded_values) + + if len(value) != len(decoded_values): + self._error_with_duplicate_in_set(decoded_values) + + return value + + def _error_with_duplicate_in_set(self, elements): + import click + import collections + + counter = collections.Counter(elements) + dups = {name for name, count in counter.items() if count > 1} + + ctx = click.get_current_context() + click.echo(ctx.get_usage() + '\n', err=True) + click.secho("Error: Option --%s was given these values: %r more than " + "one time, values passed should be unique." + % (self.cli_name, dups), err=True, fg='red', bold=True) + ctx.exit(1) class ArtifactHandler(GeneratedHandler): prefix = 'i_' def get_click_options(self): - import click import q2cli + import q2cli.core - help = "Artifact: %s" % self.repr - + type = q2cli.core.ResultPath(repr=self.repr, exists=True, + file_okay=True, dir_okay=False, + readable=True) if self.default is None: - help += ' [optional]' + requirement = '[optional]' else: - help += ' [required]' + requirement = '[required]' - option = q2cli.Option(['--' + self.cli_name], - type=click.Path(exists=True, file_okay=True, - dir_okay=False, readable=True), - help=help) - yield self._add_description(option) + option = q2cli.Option(['--' + self.cli_name], type=type, help="") + yield self._add_description(option, requirement) def get_value(self, arguments, fallback=None): import qiime2 @@ -308,18 +376,13 @@ class ResultHandler(GeneratedHandler): prefix = 'o_' def get_click_options(self): - import click import q2cli - help_txt = self.repr - if help_txt != 'Visualization': - help_txt = 'Artifact: %s' % help_txt - option = q2cli.Option(['--' + self.cli_name], - type=click.Path(exists=False, file_okay=True, - dir_okay=False, writable=True), - help="%s [required if not passing --output-dir]" - % help_txt) - yield self._add_description(option) + type = q2cli.core.ResultPath(self.repr, exists=False, file_okay=True, + dir_okay=False, writable=True) + option = q2cli.Option(['--' + self.cli_name], type=type, help="") + yield self._add_description( + option, '[required if not passing --output-dir]') def get_value(self, arguments, fallback=None): return self._locate_value(arguments, fallback) @@ -351,20 +414,22 @@ def __init__(self, name, default=NoDefault, description=None): def get_click_options(self): import click import q2cli + import q2cli.core name = '--' + self.cli_name type = click.Path(exists=True, file_okay=True, dir_okay=False, readable=True) + type = q2cli.core.MultipleType(type) help = ('Metadata file or artifact viewable as metadata. This ' - 'option may be supplied multiple times to merge metadata') + 'option may be supplied multiple times to merge metadata.') if self.default is None: - help += ' [optional]' + requirement = '[optional]' else: - help += ' [required]' + requirement = '[required]' option = q2cli.Option([name], type=type, help=help, multiple=True) - yield self._add_description(option) + yield self._add_description(option, requirement) def get_value(self, arguments, fallback=None): import os @@ -424,17 +489,17 @@ def get_click_options(self): name = '--' + self.cli_name type = str help = ('Category from metadata file or artifact viewable as ' - 'metadata') + 'metadata.') if self.default is None: - help += ' [optional]' + requirement = '[optional]' else: - help += ' [required]' + requirement = '[required]' option = q2cli.Option([name], type=type, help=help) yield from self.metadata_handler.get_click_options() - yield self._add_description(option) + yield self._add_description(option, requirement) def get_value(self, arguments, fallback=None): # Attempt to find all options before erroring so that all handlers' @@ -474,24 +539,20 @@ class RegularParameterHandler(GeneratedHandler): prefix = 'p_' def __init__(self, name, repr, ast, default=NoDefault, description=None): - super().__init__(name, repr, default=default, description=description) - self.ast = ast - - def get_type(self): import q2cli.util - # Specify 'Set'/'List' instead of 'type' == 'collection', because - # 'Dict' will require more modifications (i.e. keys, values) - if self.ast['name'] in ['Set', 'List']: - field, = self.ast['fields'] - return q2cli.util.convert_primitive(field) - return q2cli.util.convert_primitive(self.ast) + + super().__init__(name, repr, ast, default=default, + description=description) + # TODO: just create custom click.ParamType to avoid this silliness + if ast['type'] == 'collection': + ast, = ast['fields'] + self.type = q2cli.util.convert_primitive(ast) def get_click_options(self): import q2cli import q2cli.util - type = self.get_type() # Use the ugly lookup above - if type is bool: + if self.type is bool: no_name = self.prefix + 'no_' + self.name cli_no_name = q2cli.util.to_cli_name(no_name) name = '--' + self.cli_name + '/--' + cli_no_name @@ -501,67 +562,45 @@ def get_click_options(self): option_type = None else: name = '--' + self.cli_name - option_type = type + option_type = self.type + + if self.default is NoDefault: + requirement = '[required]' + elif self.default is None: + requirement = '[optional]' + else: + requirement = '[default: %s]' % self.default # Pass `default=None` and `show_default=False` to `click.Option` # because the handlers are responsible for resolving missing values and # supplying defaults. Telling Click about the default value here makes # it impossible to determine whether the user supplied or omitted a # value once the handlers are invoked. - multiple = self.ast['type'] == 'collection' - option = None - if self.default is NoDefault: - option = q2cli.Option([name], type=option_type, default=None, - show_default=False, help='[required]', - multiple=multiple) - elif self.default is None: - option = q2cli.Option([name], type=option_type, default=None, - show_default=False, help='[optional]', - multiple=multiple) - else: - option = q2cli.Option([name], type=option_type, default=None, - show_default=False, - help='[default: %s]' % self.default, - multiple=multiple) + option = q2cli.Option([name], type=option_type, default=None, + show_default=False, help='') - if multiple: - option.help = 'May be supplied multiple times. ' + option.help - - yield self._add_description(option) + yield self._add_description(option, requirement) def get_value(self, arguments, fallback=None): - value = self._locate_value(arguments, fallback, - multiple=self.ast['type'] == 'collection') + value = self._locate_value(arguments, fallback) if value is None: return None - elif self.get_type() is bool: - # Value may have been specified in --cmd-config (or another source - # in the future). If we don't have a bool type yet, attempt to - # interpret a string representing a boolean. + + elif self.type is bool: + # TODO: should we defer to the Bool primitive? It only allows + # 'true' and 'false'. if type(value) is not bool: value = self._parse_boolean(value) return value - elif self.ast['name'] == 'Set': - if len(value) > len(set(value)): - self._error_with_duplicate_in_set(value) - return set(value) - elif self.ast['name'] == 'List': - return list(value) else: import qiime2.sdk - return qiime2.sdk.parse_type( - self.repr, expect='primitive').decode(value) - - def _error_with_duplicate_in_set(self, elements): - import click - import collections - - counter = collections.Counter(elements) - dups = {name for name, count in counter.items() if count > 1} - - ctx = click.get_current_context() - click.echo(ctx.get_usage() + '\n', err=True) - click.secho("Error: Option --%s was given these values: %r more than " - "one time, values passed should be unique." - % (self.cli_name, dups), err=True, fg='red', bold=True) - ctx.exit(1) + primitive = qiime2.sdk.parse_type(self.repr, expect='primitive') + # TODO/HACK: the repr is the primitive used, but since there's a + # collection handler managing the set/list this get_value should + # handle only the pieces. This is super gross, but would be + # unecessary if click.ParamTypes were implemented for each + # kind of QIIME 2 input. + if self.ast['type'] == 'collection': + primitive, = primitive.fields + + return primitive.decode(value) diff --git a/q2cli/tests/test_cli.py b/q2cli/tests/test_cli.py index c25efa8c..e99e5a0d 100644 --- a/q2cli/tests/test_cli.py +++ b/q2cli/tests/test_cli.py @@ -169,6 +169,33 @@ def test_split_ints(self): self.assertEqual(left.view(list), [0]) self.assertEqual(right.view(list), [42, 43]) + def test_variadic_inputs(self): + qiime_cli = RootCommand() + command = qiime_cli.get_command(ctx=None, name='dummy-plugin') + output_path = os.path.join(self.tempdir, 'output.qza') + + ints1 = Artifact.import_data('IntSequence1', [1, 2, 3]).save( + os.path.join(self.tempdir, 'ints1.qza')) + ints2 = Artifact.import_data('IntSequence2', [4, 5, 6]).save( + os.path.join(self.tempdir, 'ints2.qza')) + set1 = Artifact.import_data('SingleInt', 7).save( + os.path.join(self.tempdir, 'set1.qza')) + set2 = Artifact.import_data('SingleInt', 8).save( + os.path.join(self.tempdir, 'set2.qza')) + + result = self.runner.invoke( + command, + ['variadic-input-method', '--i-ints', ints1, '--i-ints', ints2, + '--i-int-set', set1, '--i-int-set', set2, '--p-nums', '9', + '--p-nums', '10', '--p-opt-nums', '11', '--p-opt-nums', '12', + '--p-opt-nums', '13', '--o-output', output_path, '--verbose']) + + self.assertEqual(result.exit_code, 0) + self.assertTrue(os.path.exists(output_path)) + + output = Artifact.load(output_path) + self.assertEqual(output.view(list), list(range(1, 14))) + def test_with_parameters_only(self): qiime_cli = RootCommand() command = qiime_cli.get_command(ctx=None, name='dummy-plugin') From a997030612245f9af3c64df68f45d6ff5c6ba551 Mon Sep 17 00:00:00 2001 From: Evan Bolyen Date: Mon, 11 Dec 2017 11:24:24 -0700 Subject: [PATCH 2/2] SQUASH: add squash, then sift flour to remove bugs --- q2cli/core.py | 4 ++-- q2cli/handlers.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/q2cli/core.py b/q2cli/core.py index ea68ffda..dac9acf1 100644 --- a/q2cli/core.py +++ b/q2cli/core.py @@ -141,5 +141,5 @@ def __init__(self, repr, *args, **kwargs): def get_metavar(self, param): if self.repr != 'Visualization': - return "PATH ARTIFACT " + self.repr - return "PATH VISUALIZATION" + return "ARTIFACT PATH " + self.repr + return "VISUALIZATION PATH" diff --git a/q2cli/handlers.py b/q2cli/handlers.py index 33792083..a4d2ba59 100644 --- a/q2cli/handlers.py +++ b/q2cli/handlers.py @@ -110,12 +110,15 @@ def _parse_boolean(self, string): ctx.exit(1) def _add_description(self, option, requirement): + def pretty_cat(a, b, space=1): + if a: + return a + (' ' * space) + b + return b + if self.description: - option.help += '%s %s' % (self.description, requirement) - elif option.help: - option.help += ' ' + requirement - else: - option.help = requirement + option.help = pretty_cat(option.help, self.description) + option.help = pretty_cat(option.help, requirement, space=2) + return option