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

Do not use pre-release PyInquirer #684

Merged
merged 12 commits into from
Jul 23, 2020
19 changes: 16 additions & 3 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,29 @@ is as follows:

If you're not used to this workflow with git, you can start with some [basic docs from GitHub](https://help.github.com/articles/fork-a-repo/).

## Installing dev requirements

If you want to work with developing the nf-core/tools code, you'll need a couple of extra Python packages.
These are listed in `requirements-dev.txt` and can be installed as follows:

```bash
pip install --upgrade -r requirements-dev.txt
```

Then install your local fork of nf-core/tools:

```bash
pip install -e .
```

## Code formatting with Black

All Python code in nf-core/tools must be passed through the [Black Python code formatter](https://black.readthedocs.io/en/stable/).
This ensures a harmonised code formatting style throughout the package, from all contributors.

You can run Black on the command line - first install using `pip` and then run recursively on the whole repository:
You can run Black on the command line (it's included in `requirements-dev.txt`) - eg. to run recursively on the whole repository:

```bash
pip install black
black .
```

Expand Down Expand Up @@ -61,7 +75,6 @@ New features should also come with new tests, to keep the test-coverage high (we
You can try running the tests locally before pushing code using the following command:

```bash
pip install --upgrade pip pytest pytest-datafiles pytest-cov mock
pytest --color=yes tests/
```

Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pytest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ jobs:

- name: Install python dependencies
run: |
python -m pip install --upgrade pip pytest pytest-datafiles pytest-cov mock jsonschema
pip install .
python -m pip install --upgrade pip -r requirements-dev.txt
pip install -e .

- name: Install Nextflow
run: |
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,10 @@ pip install --upgrade --force-reinstall git+https://github.com/nf-core/tools.git
```

If you intend to make edits to the code, first make a fork of the repository and then clone it locally.
Go to the cloned directory and either install with pip:
Go to the cloned directory and install with pip (also installs development requirements):

```bash
pip install -e .
pip install --upgrade -r requirements-dev.txt -e .
```

### Using a specific Python interpreter
Expand Down
80 changes: 48 additions & 32 deletions nf_core/launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
from __future__ import print_function
from rich.console import Console
from rich.markdown import Markdown
from rich.prompt import Confirm

import click
import copy
import json
import logging
import os
from PyInquirer import prompt, Separator
import PyInquirer
import re
import subprocess
import textwrap
Expand All @@ -21,16 +21,14 @@
log = logging.getLogger(__name__)

#
# NOTE: WE ARE USING A PRE-RELEASE VERSION OF PYINQUIRER
#
# This is so that we can capture keyboard interruptions in a nicer way
# with the raise_keyboard_interrupt=True argument in the prompt.prompt() calls
# NOTE: When PyInquirer 1.0.3 is released we can capture keyboard interruptions
# in a nicer way # with the raise_keyboard_interrupt=True argument in the PyInquirer.prompt() calls
# It also allows list selections to have a default set.
#
# Waiting for a release of version of >1.0.3 of PyInquirer.
# See https://github.com/CITGuru/PyInquirer/issues/90
# Until then we have workarounds:
# * Default list item is moved to the top of the list
# * We manually raise a KeyboardInterrupt if we get None back from a question
#
# When available, update setup.py to use regular pip version


class Launch(object):
Expand Down Expand Up @@ -114,11 +112,7 @@ def launch_pipeline(self):
# Check if the output file exists already
if os.path.exists(self.params_out):
log.warning("Parameter output file already exists! {}".format(os.path.relpath(self.params_out)))
if click.confirm(
click.style("Do you want to overwrite this file? ", fg="yellow") + click.style("[y/N]", fg="red"),
default=False,
show_default=False,
):
if Confirm.ask("[yellow]Do you want to overwrite this file?"):
os.remove(self.params_out)
log.info("Deleted {}\n".format(self.params_out))
else:
Expand Down Expand Up @@ -250,17 +244,20 @@ def merge_nxf_flag_schema(self):

def prompt_web_gui(self):
""" Ask whether to use the web-based or cli wizard to collect params """
click.secho(
"\nWould you like to enter pipeline parameters using a web-based interface or a command-line wizard?\n",
fg="magenta",
log.info(
"[magenta]Would you like to enter pipeline parameters using a web-based interface or a command-line wizard?",
extra={"markup": True},
)
question = {
"type": "list",
"name": "use_web_gui",
"message": "Choose launch method",
"choices": ["Web based", "Command line"],
}
answer = prompt.prompt([question], raise_keyboard_interrupt=True)
answer = PyInquirer.prompt([question])
# TODO: use raise_keyboard_interrupt=True when PyInquirer 1.0.3 is released
if answer == {}:
raise KeyboardInterrupt
return answer["use_web_gui"] == "Web based"

def launch_web_gui(self):
Expand Down Expand Up @@ -399,12 +396,18 @@ def prompt_param(self, param_id, param_obj, is_required, answers):

# Print the question
question = self.single_param_to_pyinquirer(param_id, param_obj, answers)
answer = prompt.prompt([question], raise_keyboard_interrupt=True)
answer = PyInquirer.prompt([question])
# TODO: use raise_keyboard_interrupt=True when PyInquirer 1.0.3 is released
if answer == {}:
raise KeyboardInterrupt

# If required and got an empty reponse, ask again
while type(answer[param_id]) is str and answer[param_id].strip() == "" and is_required:
click.secho("Error - this property is required.", fg="red", err=True)
answer = prompt.prompt([question], raise_keyboard_interrupt=True)
log.error("This property is required.")
answer = PyInquirer.prompt([question])
# TODO: use raise_keyboard_interrupt=True when PyInquirer 1.0.3 is released
if answer == {}:
raise KeyboardInterrupt

# Don't return empty answers
if answer[param_id] == "":
Expand All @@ -426,7 +429,7 @@ def prompt_group(self, param_id, param_obj):
"type": "list",
"name": param_id,
"message": param_id,
"choices": ["Continue >>", Separator()],
"choices": ["Continue >>", PyInquirer.Separator()],
}

for child_param, child_param_obj in param_obj["properties"].items():
Expand All @@ -445,7 +448,10 @@ def prompt_group(self, param_id, param_obj):
answers = {}
while not while_break:
self.print_param_header(param_id, param_obj)
answer = prompt.prompt([question], raise_keyboard_interrupt=True)
answer = PyInquirer.prompt([question])
# TODO: use raise_keyboard_interrupt=True when PyInquirer 1.0.3 is released
if answer == {}:
raise KeyboardInterrupt
if answer[param_id] == "Continue >>":
while_break = True
# Check if there are any required parameters that don't have answers
Expand All @@ -454,7 +460,7 @@ def prompt_group(self, param_id, param_obj):
req_default = self.schema_obj.input_params.get(p_required, "")
req_answer = answers.get(p_required, "")
if req_default == "" and req_answer == "":
click.secho("Error - '{}' is required.".format(p_required), fg="red", err=True)
log.error("'{}' is required.".format(p_required))
while_break = False
else:
child_param = answer[param_id]
Expand Down Expand Up @@ -620,6 +626,21 @@ def validate_pattern(val):

question["validate"] = validate_pattern

# WORKAROUND - PyInquirer <1.0.3 cannot have a default position in a list
# For now, move the default option to the top.
# TODO: Delete this code when PyInquirer <1.0.3 is released.
apeltzer marked this conversation as resolved.
Show resolved Hide resolved
if question["type"] == "list" and "default" in question:
try:
question["choices"].remove(question["default"])
question["choices"].insert(0, question["default"])
except ValueError:
log.warning(
"Default value `{}` not found in list of choices: {}".format(
question["default"], ", ".join(question["choices"])
)
)
### End of workaround code

return question

def print_param_header(self, param_id, param_obj):
Expand Down Expand Up @@ -683,14 +704,9 @@ def build_command(self):
def launch_workflow(self):
""" Launch nextflow if required """
log.info(
"[bold underline]Nextflow command:{}[/]\n [magenta]{}\n\n".format(self.nextflow_cmd),
extra={"markup": True},
"[bold underline]Nextflow command:[/]\n[magenta]{}\n\n".format(self.nextflow_cmd), extra={"markup": True},
)

if click.confirm(
"Do you want to run this command now? " + click.style("[y/N]", fg="green"),
default=False,
show_default=False,
):
log.info("Launching workflow!")
if Confirm.ask("Do you want to run this command now? "):
log.info("Launching workflow! :rocket:", extra={"markup": True})
subprocess.call(self.nextflow_cmd, shell=True)
37 changes: 11 additions & 26 deletions nf_core/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
""" Code to deal with pipeline JSON Schema """

from __future__ import print_function
from rich.prompt import Confirm

import click
import copy
import jinja2
import json
Expand Down Expand Up @@ -244,7 +244,7 @@ def build_schema(self, pipeline_dir, no_prompts, web_only, url):

# If running interactively, send to the web for customisation
if not self.no_prompts:
if click.confirm(click.style("\nLaunch web builder for customisation and editing?", fg="magenta"), True):
if Confirm.ask(":rocket: Launch web builder for customisation and editing?"):
try:
self.launch_web_builder()
except AssertionError as e:
Expand Down Expand Up @@ -334,13 +334,6 @@ def remove_schema_notfound_configs(self):
log.debug("Removing '{}' from JSON Schema".format(p_key))
params_removed.append(p_key)

if len(params_removed) > 0:
log.info(
"Removed {} params from existing JSON Schema that were not found with `nextflow config`:\n {}\n".format(
len(params_removed), ", ".join(params_removed)
)
)

return params_removed

def prompt_remove_schema_notfound_config(self, p_key):
Expand All @@ -350,13 +343,11 @@ def prompt_remove_schema_notfound_config(self, p_key):
Returns True if it should be removed, False if not.
"""
if p_key not in self.pipeline_params.keys():
p_key_nice = click.style("params.{}".format(p_key), fg="white", bold=True)
remove_it_nice = click.style("Remove it?", fg="yellow")
if (
self.no_prompts
or self.schema_from_scratch
or click.confirm(
"Unrecognised '{}' found in schema but not pipeline. {}".format(p_key_nice, remove_it_nice), True
if self.no_prompts or self.schema_from_scratch:
return True
if Confirm.ask(
":question: Unrecognised [white bold]'params.{}'[/] found in schema but not pipeline! [yellow]Remove it?".format(
p_key
)
):
return True
Expand All @@ -372,24 +363,18 @@ def add_schema_found_configs(self):
if not p_key in self.schema["properties"].keys():
# Check if key is in group-level params
if not any([p_key in param.get("properties", {}) for k, param in self.schema["properties"].items()]):
p_key_nice = click.style("params.{}".format(p_key), fg="white", bold=True)
add_it_nice = click.style("Add to JSON Schema?", fg="cyan")
if (
self.no_prompts
or self.schema_from_scratch
or click.confirm(
"Found '{}' in pipeline but not in schema. {}".format(p_key_nice, add_it_nice), True
or Confirm.ask(
":sparkles: Found [white bold]'params.{}'[/] in pipeline but not in schema! [blue]Add to JSON Schema?".format(
p_key
)
)
):
self.schema["properties"][p_key] = self.build_schema_param(p_val)
log.debug("Adding '{}' to JSON Schema".format(p_key))
params_added.append(p_key)
if len(params_added) > 0:
log.info(
"Added {} params to JSON Schema that were found with `nextflow config`:\n {}".format(
len(params_added), ", ".join(params_added)
)
)

return params_added

Expand Down
5 changes: 5 additions & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pytest
pytest-datafiles
pytest-cov
mock
black
6 changes: 2 additions & 4 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,11 @@
"GitPython",
"jinja2",
"jsonschema",
# 'PyInquirer>1.0.3',
# Need the new release of PyInquirer, see nf_core/launch.py for details
"PyInquirer @ https://github.com/CITGuru/PyInquirer/archive/master.zip",
"PyInquirer==1.0.2",
"pyyaml",
"requests",
"requests_cache",
"rich",
"rich>=3.4.0",
"tabulate",
],
setup_requires=["twine>=1.11.0", "setuptools>=38.6."],
Expand Down
12 changes: 6 additions & 6 deletions tests/test_launch.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ def test_launch_pipeline(self, mock_webbrowser, mock_lauch_web_gui):
""" Test the main launch function """
self.launcher.launch_pipeline()

@mock.patch("click.confirm", side_effect=[False])
def test_launch_file_exists(self, mock_click_confirm):
@mock.patch.object(nf_core.launch.Confirm, "ask", side_effect=[False])
def test_launch_file_exists(self, mock_confirm):
""" Test that we detect an existing params file and return """
# Make an empty params file to be overwritten
open(self.nf_params_fn, "a").close()
Expand All @@ -39,8 +39,8 @@ def test_launch_file_exists(self, mock_click_confirm):

@mock.patch.object(nf_core.launch.Launch, "prompt_web_gui", side_effect=[True])
@mock.patch.object(nf_core.launch.Launch, "launch_web_gui")
@mock.patch("click.confirm", side_effect=[True])
def test_launch_file_exists_overwrite(self, mock_webbrowser, mock_lauch_web_gui, mock_click_confirm):
@mock.patch.object(nf_core.launch.Confirm, "ask", side_effect=[False])
def test_launch_file_exists_overwrite(self, mock_webbrowser, mock_lauch_web_gui, mock_confirm):
""" Test that we detect an existing params file and we overwrite it """
# Make an empty params file to be overwritten
open(self.nf_params_fn, "a").close()
Expand Down Expand Up @@ -104,12 +104,12 @@ def test_ob_to_pyinquirer_string(self):
result = self.launcher.single_param_to_pyinquirer("input", sc_obj)
assert result == {"type": "input", "name": "input", "message": "input", "default": "data/*{1,2}.fastq.gz"}

@mock.patch("PyInquirer.prompt.prompt", side_effect=[{"use_web_gui": "Web based"}])
@mock.patch("PyInquirer.prompt", side_effect=[{"use_web_gui": "Web based"}])
def test_prompt_web_gui_true(self, mock_prompt):
""" Check the prompt to launch the web schema or use the cli """
assert self.launcher.prompt_web_gui() == True

@mock.patch("PyInquirer.prompt.prompt", side_effect=[{"use_web_gui": "Command line"}])
@mock.patch("PyInquirer.prompt", side_effect=[{"use_web_gui": "Command line"}])
def test_prompt_web_gui_false(self, mock_prompt):
""" Check the prompt to launch the web schema or use the cli """
assert self.launcher.prompt_web_gui() == False
Expand Down