Skip to content

Commit

Permalink
Add -r/--recursive options to planemo lint.
Browse files Browse the repository at this point in the history
Shed lint with --tools will always work this way since I think the Tool Shed is pretty aggressive about finding tools.

Mentioning #139 - though this doesn't implement the multiple paths on the command-line API change.
  • Loading branch information
jmchilton committed Apr 25, 2015
1 parent 2135f12 commit 01f2af9
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 24 deletions.
8 changes: 7 additions & 1 deletion planemo/commands/cmd_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@
@options.fail_level_option()
@options.skip_option()
@options.lint_xsd_option()
@options.recursive_option()
@pass_context
def cli(ctx, path, **kwds):
"""Check specified tool(s) for common errors and adherence to best
practices.
"""
lint_args = build_lint_args(ctx, **kwds)
exit = lint_tools_on_path(ctx, path, lint_args)
exit = lint_tools_on_path(
ctx,
path,
lint_args,
recursive=kwds["recursive"]
)
sys.exit(exit)
8 changes: 7 additions & 1 deletion planemo/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,15 @@ def fail_level_option():


def recursive_shed_option():
return recursive_option(
"Recursively perform command for nested repository directories.",
)


def recursive_option(help="Recursively perform command for subdirectories."):
return click.option(
'-r',
'--recursive',
is_flag=True,
help="Recursively perform command for nested repository directories.",
help=help,
)
3 changes: 2 additions & 1 deletion planemo/shed_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ def lint_repository(ctx, path, **kwds):
path,
)
if kwds["tools"]:
for (tool_path, tool_xml) in yield_tool_xmls(ctx, path):
for (tool_path, tool_xml) in yield_tool_xmls(ctx, path,
recursive=True):
info("+Linting tool %s" % tool_path)
lint_xml_with(
lint_ctx,
Expand Down
22 changes: 10 additions & 12 deletions planemo/tool_lint.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import os
import sys
import traceback

from planemo.io import info
from planemo.io import error

import planemo.linters.xsd

from galaxy.tools.loader_directory import load_tool_elements_from_path
from planemo.tools import load_tool_elements_from_path
from galaxy.tools.lint import lint_xml

SKIP_XML_MESSAGE = "Skipping XML file - does not appear to be a tool %s."
LINTING_TOOL_MESSAGE = "Linting tool %s"
SHED_FILES = ["tool_dependencies.xml", "repository_dependencies.xml"]


def lint_tools_on_path(ctx, path, lint_args, assert_tools=True):
def lint_tools_on_path(ctx, path, lint_args, **kwds):
assert_tools = kwds.get("assert_tools", True)
recursive = kwds.get("recursive", False)
exit = 0
valid_tools = 0
for (tool_path, tool_xml) in yield_tool_xmls(ctx, path):
for (tool_path, tool_xml) in yield_tool_xmls(ctx, path, recursive):
info("Linting tool %s" % tool_path)
if not lint_xml(tool_xml, **lint_args):
error("Failed linting")
Expand All @@ -30,8 +30,11 @@ def lint_tools_on_path(ctx, path, lint_args, assert_tools=True):
return exit


def yield_tool_xmls(ctx, path):
tools = load_tool_elements_from_path(path, load_exception_handler)
def yield_tool_xmls(ctx, path, recursive=False):
tools = load_tool_elements_from_path(
path,
recursive,
)
for (tool_path, tool_xml) in tools:
if not _is_tool_xml(ctx, tool_path, tool_xml):
continue
Expand All @@ -57,11 +60,6 @@ def build_lint_args(ctx, **kwds):
return lint_args


def load_exception_handler(path, exc_info):
error("Error loading tool with path %s" % path)
traceback.print_exception(*exc_info, limit=1, file=sys.stderr)


def _lint_extra_modules(**kwds):
xsd = kwds.get("xsd", False)
if xsd:
Expand Down
18 changes: 18 additions & 0 deletions planemo/tools.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import sys
import traceback
from planemo.io import error

from galaxy.tools import loader_directory


def load_tool_elements_from_path(path, recursive):
return loader_directory.load_tool_elements_from_path(
path,
load_exception_handler,
recursive,
)


def load_exception_handler(path, exc_info):
error("Error loading tool with path %s" % path)
traceback.print_exception(*exc_info, limit=1, file=sys.stderr)
36 changes: 31 additions & 5 deletions planemo_ext/galaxy/tools/loader_directory.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import fnmatch
import glob
import os
from ..tools import loader
Expand All @@ -8,16 +9,21 @@
log = logging.getLogger(__name__)

PATH_DOES_NOT_EXIST_ERROR = "Could not load tools from path [%s] - this path does not exist."
PATH_AND_RECURSIVE_ERROR = "Cannot specify a single file and recursive."
LOAD_FAILURE_ERROR = "Failed to load tool with path %s."


def load_exception_handler(path, exc_info):
log.warn(LOAD_FAILURE_ERROR % path, exc_info=exc_info)


def load_tool_elements_from_path(path, load_exception_handler=load_exception_handler):
def load_tool_elements_from_path(
path,
load_exception_handler=load_exception_handler,
recursive=False,
):
tool_elements = []
for file in __find_tool_files(path):
for file in __find_tool_files(path, recursive=recursive):
try:
looks_like_a_tool = __looks_like_a_tool(file)
except IOError:
Expand Down Expand Up @@ -45,10 +51,30 @@ def __looks_like_a_tool(path):
return False


def __find_tool_files(path):
def __find_tool_files(path, recursive):
is_file = not os.path.isdir(path)
if not os.path.exists(path):
raise Exception(PATH_DOES_NOT_EXIST_ERROR)
if not os.path.isdir(path):
elif is_file and recursive:
raise Exception(PATH_AND_RECURSIVE_ERROR)
elif is_file:
return [os.path.abspath(path)]
else:
return map(os.path.abspath, glob.glob(path + "/**.xml"))
if not recursive:
files = glob.glob(path + "/*.xml")
else:
files = _find_files(path, "*.xml")
return map(os.path.abspath, files)


def _find_files(directory, pattern='*'):
if not os.path.exists(directory):
raise ValueError("Directory not found {}".format(directory))

matches = []
for root, dirnames, filenames in os.walk(directory):
for filename in filenames:
full_path = os.path.join(root, filename)
if fnmatch.filter([full_path], pattern):
matches.append(os.path.join(root, filename))
return matches
3 changes: 2 additions & 1 deletion tests/data/repos/multi_repos_nested/cat1/cat1.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
</repeat>
</inputs>
<outputs>
<data name="out_file1" format="input" metadata_source="input1"/>
<data name="out_file1" format_source="input1" metadata_source="input1"/>
</outputs>
<tests>
<test>
Expand All @@ -22,5 +22,6 @@
</test>
</tests>
<help>
Copy a dataset.
</help>
</tool>
3 changes: 2 additions & 1 deletion tests/data/repos/multi_repos_nested/cat2/cat2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
</repeat>
</inputs>
<outputs>
<data name="out_file1" format="input" metadata_source="input1"/>
<data name="out_file1" format_source="input1" metadata_source="input1"/>
</outputs>
<tests>
<test>
Expand All @@ -22,5 +22,6 @@
</test>
</tests>
<help>
Copy a dataset.
</help>
</tool>
18 changes: 16 additions & 2 deletions tests/test_lint.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import os
import glob

from .test_utils import CliTestCase
from .test_utils import TEST_TOOLS_DIR
from .test_utils import (
CliTestCase,
TEST_TOOLS_DIR,
TEST_REPOS_DIR,
)


class LintTestCase(CliTestCase):
Expand Down Expand Up @@ -30,3 +33,14 @@ def test_skips(self):
# Check string splitting and stuff.
lint_cmd = ["lint", "--skip", "xml_order, citations", fail_citation]
self._check_exit_code(lint_cmd, exit_code=0)

def test_recursive(self):
nested_dir = os.path.join(TEST_REPOS_DIR, "multi_repos_nested")

# Fails to find any tools without -r.
lint_cmd = ["lint", "--skip", "citations", nested_dir]
self._check_exit_code(lint_cmd, exit_code=2)

# Works with -r.
lint_cmd = ["lint", "--skip", "citations", "-r", nested_dir]
self._check_exit_code(lint_cmd, exit_code=0)

0 comments on commit 01f2af9

Please sign in to comment.