Merge pull request #42 from aCLImatise/avoid-repeats

multimeric · web-flow · commit b5f395d030ba · 2020-09-02T15:16:42.000+10:00
Avoid repeats
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -14,7 +14,7 @@ repos:
           - "dataclasses"
 
   - repo: https://github.com/psf/black
-    rev: stable
+    rev: 20.8b1
     hooks:
       - id: black
 
diff --git a/acclimatise/__init__.py b/acclimatise/__init__.py
@@ -196,9 +196,11 @@ def explore_command(
     if command.depth < max_depth:
         # By default we use the best parent help-flag
         child_flags = flags if try_subcommand_flags else [command.generated_using]
-        for positional in command.positional:
+
+        # Try each *unique* positional
+        for positional in {positional.name for positional in command.positional}:
             subcommand = explore_command(
-                cmd=cmd + [positional.name],
+                cmd=cmd + [positional],
                 flags=child_flags,
                 parent=command,
                 max_depth=max_depth,
@@ -207,7 +209,6 @@ def explore_command(
             )
             if subcommand is not None:
                 command.subcommands.append(subcommand)
-
                 # If we had any subcommands then we probably don't have any positionals, or at least don't care about them
                 command.positional = []
 
diff --git a/acclimatise/cli.py b/acclimatise/cli.py
@@ -108,7 +108,9 @@ def explore(
     for format in formats:
         converter_cls = WrapperGenerator.choose_converter(format)
         converter = converter_cls(
-            generate_names=generate_names, ignore_positionals=not pos, case=case,
+            generate_names=generate_names,
+            ignore_positionals=not pos,
+            case=case,
         )
         list(converter.generate_tree(command, out_dir))
 
@@ -133,7 +135,9 @@ def pipe(cmd, pos, generate_names, case, format):
 
     converter_cls = WrapperGenerator.choose_converter(format)
     converter = converter_cls(
-        generate_names=generate_names, ignore_positionals=not pos, case=case,
+        generate_names=generate_names,
+        ignore_positionals=not pos,
+        case=case,
     )
     output = converter.save_to_string(command)
     print(output)
diff --git a/acclimatise/converter/wdl.py b/acclimatise/converter/wdl.py
@@ -38,7 +38,11 @@ def flag_to_command_input(
         if isinstance(named_flag.arg.args, model.EmptyFlagArg):
             args.update(dict(true=named_flag.arg.longest_synonym, false=""))
         else:
-            args.update(dict(prefix=named_flag.arg.longest_synonym,))
+            args.update(
+                dict(
+                    prefix=named_flag.arg.longest_synonym,
+                )
+            )
     elif isinstance(named_flag, model.Positional):
         args.update(dict(optional=False, position=named_flag.position))
 
diff --git a/acclimatise/model.py b/acclimatise/model.py
@@ -108,19 +108,19 @@ def command_tree(self) -> typing.Generator["Command", None, None]:
         for command in self.subcommands:
             yield from command.command_tree()
 
-    positional: typing.List["Positional"]
+    command: typing.List[str]
     """
-    All positional arguments supported by this command
+    The command line used to invoke this command, e.g. ["bwa", "mem"]
     """
 
-    named: typing.List["Flag"]
+    positional: typing.List["Positional"] = field(default_factory=list)
     """
-    All named arguments (flags) supported by this command
+    All positional arguments supported by this command
     """
 
-    command: typing.List[str]
+    named: typing.List["Flag"] = field(default_factory=list)
     """
-    The command line used to invoke this command, e.g. ["bwa", "mem"]
+    All named arguments (flags) supported by this command
     """
 
     parent: typing.Optional["Command"] = None
diff --git a/acclimatise/usage_parser/elements.py b/acclimatise/usage_parser/elements.py
@@ -36,7 +36,11 @@ def action(s, loc, toks):
 
 mandatory_element = (
     element_char.copy()
-    .setParseAction(lambda s, loc, toks: UsageElement(text=toks[0],))
+    .setParseAction(
+        lambda s, loc, toks: UsageElement(
+            text=toks[0],
+        )
+    )
     .setName("MandatoryElement")
 )
 """
diff --git a/docs/changes.rst b/docs/changes.rst
@@ -7,6 +7,7 @@ Changelog
 1.0.2 (2020-08-25)
 ------------------
 * Ensure we never return ``None`` from the ``DockerExecutor``
+* Add initial parsing of "output inputs". Thanks to `@bernt-matthias <https://github.com/bernt-matthias>`_ (`#15 <https://github.com/aCLImatise/CliHelpParser/pull/15>`_). However this information is not yet used in the converters.
 
 1.0.1 (2020-08-22)
 ------------------
diff --git a/test/executors/test_docker.py b/test/executors/test_docker.py
@@ -7,7 +7,9 @@
 def test_docker(bwamem_help):
     client = docker.from_env()
     container = client.containers.run(
-        "biocontainers/bwa:v0.7.17_cv1", entrypoint=["sleep", "999999999"], detach=True,
+        "biocontainers/bwa:v0.7.17_cv1",
+        entrypoint=["sleep", "999999999"],
+        detach=True,
     )
 
     exec = DockerExecutor(container)
@@ -23,7 +25,9 @@ def test_docker_kill():
     """
     client = docker.from_env(timeout=99999)
     container = client.containers.run(
-        "ubuntu:latest", entrypoint=["sleep", "999999999"], detach=True,
+        "ubuntu:latest",
+        entrypoint=["sleep", "999999999"],
+        detach=True,
     )
 
     exec = DockerExecutor(container)
@@ -54,7 +58,9 @@ def test_infinite_output():
     """
     client = docker.from_env(timeout=99999)
     container = client.containers.run(
-        "ubuntu:latest", entrypoint=["sleep", "999999999"], detach=True,
+        "ubuntu:latest",
+        entrypoint=["sleep", "999999999"],
+        detach=True,
     )
 
     exec = DockerExecutor(container)
diff --git a/test/name_generation/test_single_flag.py b/test/name_generation/test_single_flag.py
@@ -54,7 +54,11 @@ def test_name_to_words(gen):
     """
     Check that we can get an argument name even if the argument's flag is a symbol
     """
-    arg = Flag(synonyms=["--genomepaths"], description="", args=EmptyFlagArg(),)
+    arg = Flag(
+        synonyms=["--genomepaths"],
+        description="",
+        args=EmptyFlagArg(),
+    )
 
     name = gen.choose_variable_names([arg])[0].name
     assert "genome" in name
diff --git a/test/test_cli.py b/test/test_cli.py
@@ -69,7 +69,14 @@ def test_explore_samtools_no_subcommands(runner, caplog):
     caplog.set_level(100000)
     with tempfile.TemporaryDirectory() as tempdir:
         result = runner.invoke(
-            main, ["explore", "samtools", "--no-subcommands", "--out-dir", tempdir,],
+            main,
+            [
+                "explore",
+                "samtools",
+                "--no-subcommands",
+                "--out-dir",
+                tempdir,
+            ],
         )
         cli_worked(result)
         # Since we aren't looking at subcommands, there should be one file for each format
diff --git a/test/test_explore_e2e.py b/test/test_explore_e2e.py
@@ -1,10 +1,12 @@
 import os
 import shutil
 import tempfile
+from unittest.mock import Mock, patch
 
 import pytest
 
 from acclimatise import explore_command
+from acclimatise.model import Command, Positional
 
 from .util import (
     HelpText,
@@ -60,7 +62,27 @@ def test_explore_bwa():
     command = explore_command(["bwa"], max_depth=1)
 
     # Check that we parsed bwa mem correctly
-    mem = command.subcommands[1]
+    mem = [cmd for cmd in command.subcommands if cmd.command[1] == "mem"][0]
     assert len(mem.positional) == 3
     assert len(mem.subcommands) == 0
     assert len(mem.named) >= 30
+
+
+def test_repeat_positionals():
+    """
+    Test that, if we have multiple duplicate positionals, only the first is tested
+    """
+    parent = Command(
+        command=[],
+        positional=[
+            Positional(name="a", description="", position=i) for i in range(10)
+        ],
+    )
+    child = Command(command=[])
+
+    with patch("acclimatise.explore_command", new=lambda *args, **kwargs: child):
+        with patch("acclimatise.best_cmd", new=Mock(return_value=parent)) as mocked:
+            explore_command([])
+
+            # We should only call best_command once, since there's only one unique positional
+            assert mocked.call_count == 1
