Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
Use packaging.version.parse() instead of `float()` for version comparisons

Co-Authored-By: mvdbeek <m.vandenbeek@gmail.com>
  • Loading branch information
nsoranzo and mvdbeek authored May 2, 2019
1 parent 02c4c46 commit 205b4a6
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 12 deletions.
4 changes: 2 additions & 2 deletions lib/galaxy/tools/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,9 +648,9 @@ def parse(self, tool_source, guid=None, dynamic=False):
if self.python_template_version is None:
# If python_template_version not specified we assume tools with profile versions >= 19.05 are python 3 ready
if self.profile >= 19.05:
self.python_template_version = 3.5
self.python_template_version = '3.5'
else:
self.python_template_version = 2.7
self.python_template_version = '2.7'

# Get the (user visible) name of the tool
self.name = tool_source.parse_name()
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/tools/actions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ def get_ext_or_implicit_ext(hda):
return hda.ext


def determine_output_format(output, parameter_context, input_datasets, input_dataset_collections, random_input_ext, python_template_version=3):
def determine_output_format(output, parameter_context, input_datasets, input_dataset_collections, random_input_ext, python_template_version='3'):
""" Determines the output format for a dataset based on an abstract
description of the output (galaxy.tools.parser.ToolOutput), the parameter
wrappers, a map of the input datasets (name => HDA), and the last input
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/tools/parser/cwl.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def parse_profile(self):
return "16.04"

def parse_python_template_version(self):
return 3.5
return '3.5'


class CwlPageSource(PageSource):
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/tools/parser/xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ def parse_profile(self):
def parse_python_template_version(self):
python_template_version = self.root.get("python_template_version", None)
if python_template_version is not None:
python_template_version = float(python_template_version)
python_template_version = packaging.version.parse(python_template_version)
return python_template_version


Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/tools/parser/yaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def parse_profile(self):
def parse_python_template_version(self):
python_template_version = self.root_dict.get("python_template_version", None)
if python_template_version is not None:
python_template_version = float(python_template_version)
python_template_version = packaging.version.parse(python_template_version)
return python_template_version


Expand Down
6 changes: 3 additions & 3 deletions lib/galaxy/util/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def fill_template(template_text,
compiler_class=Compiler,
first_exception=None,
futurized=False,
python_template_version=3,
python_template_version='3',
**kwargs):
"""Fill a cheetah template out for specified context.
Expand All @@ -57,7 +57,7 @@ def fill_template(template_text,
except NotFound as e:
if first_exception is None:
first_exception = e
if python_template_version < 3 and retry > 0 and sys.version_info.major > 2:
if sys.version_info.major > 2 and python_template_version.release[0] < 3 and retry > 0:
tb = e.__traceback__
last_stack = traceback.extract_tb(tb)[-1]
if last_stack.name == '<listcomp>':
Expand All @@ -83,7 +83,7 @@ def fill_template(template_text,
except Exception as e:
if first_exception is None:
first_exception = e
if sys.version_info.major > 2 and python_template_version < 3 and not futurized:
if sys.version_info.major > 2 and python_template_version.release[0] < 3 and not futurized:
# Possibly an error caused by attempting to run python 2
# template code on python 3. Run the generated module code
# through futurize and hope for the best.
Expand Down
2 changes: 1 addition & 1 deletion test/unit/tools/test_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ class MockTool(object):

def __init__(self, app):
self.profile = 16.01
self.python_template_version = 2.7
self.python_template_version = '2.7'
self.app = app
self.hooks_called = []
self.environment_variables = []
Expand Down
4 changes: 2 additions & 2 deletions test/unit/tools/test_fill_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def test_fill_list_comprehension_template():


def test_fill_list_comprehension_template_2():
template_str = fill_template(LIST_COMPREHENSION_TEMPLATE, python_template_version=2, retry=1)
template_str = fill_template(LIST_COMPREHENSION_TEMPLATE, python_template_version='2', retry=1)
assert template_str == 'echo 1\n'


Expand All @@ -73,5 +73,5 @@ def test_gen_expr():


def test_fix_template_two_to_three():
template_str = fill_template(TWO_TO_THREE_TEMPLATE, python_template_version=2, retry=1)
template_str = fill_template(TWO_TO_THREE_TEMPLATE, python_template_version='2', retry=1)
assert template_str == 'a a 1'

0 comments on commit 205b4a6

Please sign in to comment.