From 7ee160df902e3e7233bd7e87b4734796cc430007 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 12 Jun 2017 10:26:58 -0400 Subject: [PATCH 1/2] Better PR messages for container_register. --- planemo/commands/cmd_container_register.py | 23 ++++++++++++++------ planemo/conda.py | 25 ++++++++++++++++++++-- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/planemo/commands/cmd_container_register.py b/planemo/commands/cmd_container_register.py index 90570b000..adfaf7856 100644 --- a/planemo/commands/cmd_container_register.py +++ b/planemo/commands/cmd_container_register.py @@ -1,13 +1,14 @@ """Module describing the planemo ``container_register`` command.""" import os +import string import click -from galaxy.tools.deps.mulled.util import quay_repository, v2_image_name +from galaxy.tools.deps.mulled.util import conda_build_target_str, quay_repository, v2_image_name from planemo import options from planemo.cli import command_function -from planemo.conda import best_practice_search, collect_conda_target_lists +from planemo.conda import best_practice_search, collect_conda_target_lists_and_tool_paths from planemo.git import add, branch, commit, push from planemo.github_util import clone_fork_branch, get_repository_object, pull_request from planemo.mulled import conda_to_mulled_targets @@ -15,7 +16,7 @@ REGISTERY_TARGET_NAME = "multi-package-containers" REGISTERY_TARGET_PATH = "combinations" REGISTERY_REPOSITORY = "jmchilton/multi-package-containers" -DEFAULT_MESSAGE = "Add %s (generated with Planemo)." +DEFAULT_MESSAGE = "Add container $hash.\n**Hash**: $hash\n\n**Packages**:\n$packages\n\n**For** :\n$tools\n\nGenerated with Planemo." @click.command('container_register') @@ -63,9 +64,11 @@ def cli(ctx, paths, **kwds): registry_target = RegistryTarget(ctx, **kwds) combinations_added = 0 - for conda_targets in collect_conda_target_lists(ctx, paths, recursive=kwds["recursive"]): + conda_targets_list, tool_paths_list = collect_conda_target_lists_and_tool_paths(ctx, paths, recursive=kwds["recursive"]) + for conda_targets, tool_paths in zip(conda_targets_list, tool_paths_list): ctx.vlog("Handling conda_targets [%s]" % conda_targets) mulled_targets = conda_to_mulled_targets(conda_targets) + mulled_targets_str = "- " + "\n- ".join(map(conda_build_target_str, mulled_targets)) if len(mulled_targets) < 2: ctx.vlog("Skipping registeration, fewer than 2 targets discovered.") # Skip these for now, we will want to revisit this for conda-forge dependencies and such. @@ -101,7 +104,8 @@ def cli(ctx, paths, **kwds): continue registry_target.write_targets(ctx, target_filename, mulled_targets) - registry_target.handle_pull_request(ctx, name, target_filename, **kwds) + tools_str = "\n".join(map(lambda p: "- " + os.path.basename(p), tool_paths)) + registry_target.handle_pull_request(ctx, name, target_filename, mulled_targets_str, tools_str, **kwds) combinations_added += 1 @@ -137,9 +141,14 @@ def has_pull_request_for(self, name): return has_pr - def handle_pull_request(self, ctx, name, target_filename, **kwds): + def handle_pull_request(self, ctx, name, target_filename, packages_str, tools_str, **kwds): if self.do_pull_request: - message = kwds["message"] % name + message = kwds["message"] + message = string.Template(message).safe_substitute({ + "hash": name, + "packages": packages_str, + "tools": tools_str, + }) branch_name = name.replace(":", "-") branch(ctx, self.target_repository, branch_name, from_branch="master") add(ctx, self.target_repository, target_filename) diff --git a/planemo/conda.py b/planemo/conda.py index ed589b378..948327368 100644 --- a/planemo/conda.py +++ b/planemo/conda.py @@ -101,15 +101,35 @@ def parse_target(target_str): def collect_conda_target_lists(ctx, paths, recursive=False, found_tool_callback=None): """Load CondaTarget lists from supplied artifact sources. + If a tool contains more than one requirement, the requirements will all + appear together as one list element of the output list. + """ + conda_target_lists, _ = collect_conda_target_lists_and_tool_paths(ctx, paths, recursive=recursive, found_tool_callback=found_tool_callback) + return conda_target_lists + + +def collect_conda_target_lists_and_tool_paths(ctx, paths, recursive=False, found_tool_callback=None): + """Load CondaTarget lists from supplied artifact sources. + If a tool contains more than one requirement, the requirements will all appear together as one list element of the output list. """ conda_target_lists = set([]) + tool_paths = {} for (tool_path, tool_source) in yield_tool_sources_on_paths(ctx, paths, recursive=recursive, yield_load_errors=False): if found_tool_callback: found_tool_callback(tool_path) - conda_target_lists.add(frozenset(tool_source_conda_targets(tool_source))) - return conda_target_lists + targets = frozenset(tool_source_conda_targets(tool_source)) + conda_target_lists.add(targets) + if targets not in tool_paths: + tool_paths[targets] = [] + tool_paths[targets].append(tool_path) + + # Turn them into lists so the order matches before returning... + conda_target_lists = list(conda_target_lists) + conda_target_tool_paths = [tool_paths[c] for c in conda_target_lists] + + return conda_target_lists, conda_target_tool_paths def tool_source_conda_targets(tool_source): @@ -140,5 +160,6 @@ def best_practice_search(conda_target): "build_conda_context", "collect_conda_targets", "collect_conda_target_lists", + "collect_conda_target_lists_and_tool_paths", "tool_source_conda_targets", ) From eb9df54a201d8f4b5b691c6305038dce50121c52 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Mon, 12 Jun 2017 13:14:00 -0400 Subject: [PATCH 2/2] Use deafultdict as recommended by @bgruening in PR review. --- planemo/conda.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/planemo/conda.py b/planemo/conda.py index 948327368..9be85ef84 100644 --- a/planemo/conda.py +++ b/planemo/conda.py @@ -5,6 +5,7 @@ from __future__ import absolute_import +import collections import os import threading @@ -115,14 +116,12 @@ def collect_conda_target_lists_and_tool_paths(ctx, paths, recursive=False, found appear together as one list element of the output list. """ conda_target_lists = set([]) - tool_paths = {} + tool_paths = collections.defaultdict(list) for (tool_path, tool_source) in yield_tool_sources_on_paths(ctx, paths, recursive=recursive, yield_load_errors=False): if found_tool_callback: found_tool_callback(tool_path) targets = frozenset(tool_source_conda_targets(tool_source)) conda_target_lists.add(targets) - if targets not in tool_paths: - tool_paths[targets] = [] tool_paths[targets].append(tool_path) # Turn them into lists so the order matches before returning...