Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint extra #350

Merged
merged 9 commits into from Oct 27, 2015
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions planemo_ext/galaxy/tools/lint_util.py
@@ -0,0 +1,5 @@


def is_datasource(tool_xml):
"""Returns true if the tool is a datasource tool"""
return tool_xml.getroot().attrib.get('tool_type', '') == 'data_source'
34 changes: 33 additions & 1 deletion planemo_ext/galaxy/tools/linters/inputs.py
@@ -1,6 +1,8 @@
from ..lint_util import is_datasource


def lint_inputs(tool_xml, lint_ctx):
datasource = is_datasource(tool_xml)
inputs = tool_xml.findall("./inputs//param")
num_inputs = 0
for param in inputs:
Expand All @@ -23,10 +25,40 @@ def lint_inputs(tool_xml, lint_ctx):
if "format" not in param_attrib:
lint_ctx.warn("Param input [%s] with no format specified - 'data' format will be assumed.", param_name)
# TODO: Validate type, much more...

conditional_selects = tool_xml.findall("./inputs//conditional")
for conditional in conditional_selects:
select = conditional.find('./param[@type="select"]')
# Should conditionals ever not have a select?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can have a boolean. According to https://github.com/galaxy-iuc/standards/blob/master/docs/best_practices/tool_xml.rst#booleans we do not recommend to use booleans for a conditional but it is possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting. Thanks for that, I'll update it accordingly

if not len(select):
lint_ctx.info("Conditional without <param type=\"select\" />")
continue

select_options = select.findall('./option[@value]')
select_option_ids = [option.attrib['value'] for option in select_options]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe here it would be good to call lint_ctx.error() if the <option> has no value attribute.

whens = conditional.findall('./when')
when_ids = [when.attrib['value'] for when in whens]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe here it would be good to call lint_ctx.error() if the <when> has no value attribute.


for select_id in select_option_ids:
if select_id not in when_ids:
lint_ctx.warn("No <when /> block found for select option '%s'" % select_id)

for when_id in when_ids:
if when_id not in select_option_ids:
lint_ctx.warn("No <option /> block found for when block '%s'" % when_id)

if datasource:
for datasource_tag in ('display', 'uihints'):
if not any([param.tag == datasource_tag for param in inputs]):
lint_ctx.info("%s tag usually present in data sources" % datasource_tag)

if num_inputs:
lint_ctx.info("Found %d input parameters.", num_inputs)
else:
lint_ctx.warn("Found not input parameters.")
if datasource:
lint_ctx.info("No input parameters, OK for data sources")
else:
lint_ctx.warn("Found no input parameters.")


def lint_repeats(tool_xml, lint_ctx):
Expand Down
4 changes: 4 additions & 0 deletions planemo_ext/galaxy/tools/linters/outputs.py
Expand Up @@ -8,6 +8,10 @@ def lint_output(tool_xml, lint_ctx):
lint_ctx.warn("Tool contains multiple output sections, behavior undefined.")

num_outputs = 0
if len(outputs) == 0:
lint_ctx.warn("No outputs found")
return

for output in list(outputs[0]):
if output.tag not in ["data", "collection"]:
lint_ctx.warn("Unknown element found in outputs [%s]" % output.tag)
Expand Down
10 changes: 7 additions & 3 deletions planemo_ext/galaxy/tools/linters/tests.py
@@ -1,10 +1,14 @@

from ..lint_util import is_datasource

# Misspelled so as not be picked up by nosetests.
def lint_tsts(tool_xml, lint_ctx):
tests = tool_xml.findall("./tests/test")
if not tests:
datasource = is_datasource(tool_xml)

if not tests and not datasource:
lint_ctx.warn("No tests found, most tools should define test cases.")
elif datasource:
lint_ctx.info("No tests found, that should be OK for data_sources.")

num_valid_tests = 0
for test in tests:
Expand All @@ -24,7 +28,7 @@ def lint_tsts(tool_xml, lint_ctx):
else:
num_valid_tests += 1

if num_valid_tests:
if num_valid_tests or datasource:
lint_ctx.valid("%d test(s) found.", num_valid_tests)
else:
lint_ctx.warn("No valid test(s) found.")
27 changes: 24 additions & 3 deletions planemo_ext/galaxy/tools/linters/xml_order.py
Expand Up @@ -17,16 +17,37 @@
'citations',
]

DATASOURCE_TAG_ORDER = [
'description',
'command',
'inputs',
'request_param_translation',
'uihints',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are uihints even respected for data source tools? Hmm... we can revisit that later though.

Can you add macros and configfiles back in the same order as normal tools?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an awful good question regarding uihints.

Yes, sorry, I removed everything I didn't see during very brief greps through the tools directory.

'outputs',
'options',
'help',
'citations',
]


# Ensure the XML blocks appear in the correct order prescribed
# by the tool author best practices.
def lint_xml_order(tool_xml, lint_ctx):
tool_root = tool_xml.getroot()

if tool_root.attrib.get('tool_type', '') == 'data_source':
_validate_for_tags(tool_root, lint_ctx, DATASOURCE_TAG_ORDER)
else:
_validate_for_tags(tool_root, lint_ctx, TAG_ORDER)


def _validate_for_tags(root, lint_ctx, tag_ordering):
last_tag = None
last_key = None
for elem in list(tool_xml.getroot()):
for elem in root:
tag = elem.tag
if tag in TAG_ORDER:
key = TAG_ORDER.index(tag)
if tag in tag_ordering:
key = tag_ordering.index(tag)
if last_key:
if last_key > key:
lint_ctx.warn("Best practice violation [%s] elements should come before [%s]" % (tag, last_tag))
Expand Down