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

Reduce use of "if defined(argument)" #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
author_email="[email protected]",
license="GNU",
packages=["wdlgen"],
install_requires=[],
install_requires=[
'miniwdl'
],
zip_safe=False,
long_description=long_description,
long_description_content_type="text/markdown",
Expand Down
45 changes: 21 additions & 24 deletions tests/test_task_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@
ParameterMeta,
)

import WDL
from WDL._parser import _ExprTransformer, parse, _grammar


def parse_miniwdl_token(token: str, text: str, version: bool = None):
"""
Uses miniWDL to parse some fragment of WDL

:param token: The name of the WDL token to parse. Refer to
https://github.com/chanzuckerberg/miniwdl/blob/master/WDL/_grammar.py for a complete list
:param text: The text to parse as WDL
:param version: "1.0" (default) or "draft-2"
"""
return _ExprTransformer().transform(parse(_grammar.get(version)[0], text, token))

class TestTaskGeneration(unittest.TestCase):
def test_simple_task(self):
Expand Down Expand Up @@ -60,27 +74,6 @@ def test_readme_task(self):
# command in next section
t.outputs.append(Output(WdlType.parse_type("File"), "standardOut", "stdout()"))

command = Task.Command("echo")
command.inputs.append(
Task.Command.CommandInput(
"taskGreeting",
optional=False,
position=None,
prefix="-a",
separate_value_from_prefix=True,
default=None,
)
)
command.inputs.append(
Task.Command.CommandInput(
"otherInput",
optional=True,
position=2,
prefix="optional-param=",
separate_value_from_prefix=False,
default=None,
)
)
command = Task.Command("echo")
command.inputs.append(
Task.Command.CommandInput(
Expand Down Expand Up @@ -164,12 +157,16 @@ def test_readme_example(self):
default=None,
)
)
expected = """\
output = command.get_string()
expected = '''\
echo \\
-a ~{taskGreeting} \\
~{if defined(otherInput) then ('"' + "optional-param=" + otherInput + '"') else ""}"""
~{'"optional-param=' + otherInput + '"'}'''
# t is the task
self.assertEqual(expected, command.get_string())
self.assertEqual(expected, output)

# Check that it's also valid WDL
parse_miniwdl_token('command2', 'command <<<\n' + output + '\n>>>')

def test_commandinput_space(self):
t = Task.Command.CommandInput(
Expand Down
18 changes: 7 additions & 11 deletions wdlgen/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ def get_string(self):
self.false,
)

pr = self.prefix if self.prefix else ""
bc = pr + (" " if self.separate and self.prefix else "")
prefix = self.prefix if self.prefix else ""
bc = prefix + (" " if self.separate and self.prefix else "")

if self.separate_arrays:
if array_sep or default or true or false:
Expand Down Expand Up @@ -154,16 +154,12 @@ def get_string(self):
options.append(f'false="{false if false else ""}"')

stroptions = "".join(o + " " for o in options)

prewithquotes = f'"{bc}" + ' if bc.strip() else ""
if self.optional and not default and not is_flag and prewithquotes:
if self.optional and not default and not is_flag and bc.strip():
# Option 1: We apply quotes are value, Option 2: We quote whole "prefix + name" combo
full_token = (
f"{prewithquotes} '\"' + {name} + '\"'"
if (self.separate and self.prefix and prewithquotes)
else f"'\"' + {prewithquotes}{name} + '\"'"
)
return f'~{{{stroptions}if defined({name}) then ({full_token}) else ""}}'
if self.separate and self.prefix:
return f'~{{\'"{prefix}" "\' + {name} + \'"\'}}'
else:
return f'~{{\'"{prefix}\' + {name} + \'"\'}}'
else:
return bc + f"~{{{stroptions}{name}}}"

Expand Down