From 2051c5020074a2498f5a683ce9b0aa88a6d50c9d Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Thu, 13 Aug 2020 18:53:00 +0200 Subject: [PATCH 1/7] Better config parsing. --- src/pytask_parallel/config.py | 6 +++++- tests/test_config.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/pytask_parallel/config.py b/src/pytask_parallel/config.py index 865bd4b..b0c8726 100644 --- a/src/pytask_parallel/config.py +++ b/src/pytask_parallel/config.py @@ -11,13 +11,17 @@ def pytask_parse_config(config, config_from_cli, config_from_file): ) if config["n_workers"] == "auto": config["n_workers"] = max(os.cpu_count() - 1, 1) + else: + config["n_workers"] = int(config["n_workers"]) config["delay"] = get_first_not_none_value( - config_from_cli, config_from_file, key="delay", default=0.1 + config_from_cli, config_from_file, key="delay", default=0.1, callback=float ) config["parallel_backend"] = get_first_not_none_value( config_from_cli, config_from_file, key="parallel_backend", default="processes" ) + if config["parallel_backend"] not in ["processes", "threads"]: + raise ValueError("parallel_backend has to be 'processes' or 'threads'.") @hookimpl diff --git a/tests/test_config.py b/tests/test_config.py index 7b73c64..fe33a39 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,4 +1,5 @@ import os +import textwrap import pytest from pytask import main @@ -18,3 +19,35 @@ def test_interplay_between_debugging_and_parallel(tmp_path, pdb, n_workers, expected): session = main({"paths": tmp_path, "pdb": pdb, "n_workers": n_workers}) assert session.config["n_workers"] == expected + + +@pytest.mark.parametrize("config_file", ["pytask.ini", "tox.ini", "setup.cfg"]) +@pytest.mark.parametrize( + "name, value", + [ + ("n_workers", "auto"), + ("n_workers", 1), + ("n_workers", 2), + ("delay", 0.1), + ("delay", 1), + ("parallel_backend", "threads"), + ("parallel_backend", "processes"), + ("parallel_backend", "unknown_backend"), + ], +) +def test_reading_values_from_config_file(tmp_path, config_file, name, value): + config = f""" + [pytask] + {name} = {value} + """ + tmp_path.joinpath(config_file).write_text(textwrap.dedent(config)) + + try: + session = main({"paths": tmp_path}) + except Exception as e: + assert "Error while configuring pytask" in str(e) + else: + assert session.exit_code == 0 + if value == "auto": + value = os.cpu_count() - 1 + assert session.config[name] == value From 44b9518094689d4cce46e3023f37d5b0604b165f Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Thu, 13 Aug 2020 18:53:15 +0200 Subject: [PATCH 2/7] more. --- src/pytask_parallel/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pytask_parallel/config.py b/src/pytask_parallel/config.py index b0c8726..d737df5 100644 --- a/src/pytask_parallel/config.py +++ b/src/pytask_parallel/config.py @@ -26,6 +26,6 @@ def pytask_parse_config(config, config_from_cli, config_from_file): @hookimpl def pytask_post_parse(config): - # Disable parallelization if debugging is enabled. + """Disable parallelization if debugging is enabled.""" if config["pdb"] or config["trace"]: config["n_workers"] = 1 From 9285a7b27dae69220c6d01725407bdb752170f6a Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 14 Aug 2020 07:27:11 +0200 Subject: [PATCH 3/7] more tests. --- src/pytask_parallel/callbacks.py | 28 ++++++++++++++++++++++++++++ src/pytask_parallel/cli.py | 16 +++++----------- src/pytask_parallel/config.py | 24 ++++++++++++++++++------ tests/test_callbacks.py | 27 +++++++++++++++++++++++++++ tests/test_cli.py | 29 ++++++++++++----------------- tests/test_execute.py | 28 ++++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 src/pytask_parallel/callbacks.py create mode 100644 tests/test_callbacks.py diff --git a/src/pytask_parallel/callbacks.py b/src/pytask_parallel/callbacks.py new file mode 100644 index 0000000..5e540e9 --- /dev/null +++ b/src/pytask_parallel/callbacks.py @@ -0,0 +1,28 @@ +import click + + +def n_workers_click_callback(ctx, name, value): # noqa: U100 + return n_workers_callback(value) + + +def n_workers_callback(value): + error_occurred = False + if value == "auto": + pass + elif value is None: + pass + elif isinstance(value, int) and 1 <= value: + pass + else: + try: + value = int(value) + except ValueError: + error_occurred = True + else: + if value < 1: + error_occurred = True + + if error_occurred: + raise click.UsageError("n-processes can either be an integer >= 1 or 'auto'.") + + return value diff --git a/src/pytask_parallel/cli.py b/src/pytask_parallel/cli.py index d3e10ac..2c3e2a4 100644 --- a/src/pytask_parallel/cli.py +++ b/src/pytask_parallel/cli.py @@ -1,21 +1,22 @@ import click from _pytask.config import hookimpl +from pytask_parallel.callbacks import n_workers_click_callback @hookimpl def pytask_add_parameters_to_cli(command): additional_parameters = [ click.Option( - ["-n", "--n-processes"], + ["-n", "--n-workers"], help=( "Max. number of pytask_parallel tasks. Integer >= 1 or 'auto' which is " "os.cpu_count() - 1. [default: 1 (no parallelization)]" ), - callback=_validate_n_workers, - metavar="", + metavar="[INTEGER|auto]", + callback=n_workers_click_callback, ), click.Option( - ["--pytask_parallel-backend"], + ["--parallel-backend"], type=click.Choice(["processes", "threads"]), help="Backend for the parallelization. [default: processes]", ), @@ -29,10 +30,3 @@ def pytask_add_parameters_to_cli(command): ), ] command.params.extend(additional_parameters) - - -def _validate_n_workers(ctx, param, value): # noqa: U100 - if (isinstance(value, int) and value >= 1) or value == "auto": - pass - else: - raise click.UsageError("n-processes can either be an integer >= 1 or 'auto'.") diff --git a/src/pytask_parallel/config.py b/src/pytask_parallel/config.py index d737df5..7172c8a 100644 --- a/src/pytask_parallel/config.py +++ b/src/pytask_parallel/config.py @@ -2,26 +2,32 @@ from _pytask.config import hookimpl from _pytask.shared import get_first_not_none_value +from pytask_parallel.callbacks import n_workers_callback @hookimpl def pytask_parse_config(config, config_from_cli, config_from_file): config["n_workers"] = get_first_not_none_value( - config_from_cli, config_from_file, key="n_workers", default=1 + config_from_cli, + config_from_file, + key="n_workers", + default=1, + callback=n_workers_callback, ) if config["n_workers"] == "auto": config["n_workers"] = max(os.cpu_count() - 1, 1) - else: - config["n_workers"] = int(config["n_workers"]) + config["delay"] = get_first_not_none_value( config_from_cli, config_from_file, key="delay", default=0.1, callback=float ) config["parallel_backend"] = get_first_not_none_value( - config_from_cli, config_from_file, key="parallel_backend", default="processes" + config_from_cli, + config_from_file, + key="parallel_backend", + default="processes", + callback=_parallel_backend_callback, ) - if config["parallel_backend"] not in ["processes", "threads"]: - raise ValueError("parallel_backend has to be 'processes' or 'threads'.") @hookimpl @@ -29,3 +35,9 @@ def pytask_post_parse(config): """Disable parallelization if debugging is enabled.""" if config["pdb"] or config["trace"]: config["n_workers"] = 1 + + +def _parallel_backend_callback(x): + if x not in ["processes", "threads"]: + raise ValueError("parallel_backend has to be 'processes' or 'threads'.") + return x diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py new file mode 100644 index 0000000..f60c8d2 --- /dev/null +++ b/tests/test_callbacks.py @@ -0,0 +1,27 @@ +import functools +from contextlib import ExitStack as does_not_raise # noqa: N813 + +import click +import pytest +from pytask_parallel.callbacks import n_workers_callback +from pytask_parallel.callbacks import n_workers_click_callback + + +partialed_callback = functools.partial(n_workers_click_callback, ctx=None, name=None) + + +@pytest.mark.unit +@pytest.mark.parametrize( + "value, expectation", + [ + (0, pytest.raises(click.UsageError)), + (1, does_not_raise()), + (2, does_not_raise()), + ("auto", does_not_raise()), + ("asdad", pytest.raises(click.UsageError)), + ], +) +@pytest.mark.parametrize("func", [n_workers_callback, partialed_callback]) +def test_validate_n_workers(func, value, expectation): + with expectation: + func(value=value) diff --git a/tests/test_cli.py b/tests/test_cli.py index 4aec14a..9f43f7b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,20 +1,15 @@ -from contextlib import ExitStack as does_not_raise # noqa: N813 +import subprocess +import textwrap -import click -import pytest -from pytask_parallel.cli import _validate_n_workers +def test_delay_via_cli(tmp_path): + source = """ + import pytask -@pytest.mark.unit -@pytest.mark.parametrize( - "value, expectation", - [ - (0, pytest.raises(click.UsageError)), - (1, does_not_raise()), - (2, does_not_raise()), - ("auto", does_not_raise()), - ], -) -def test_validate_n_workers(value, expectation): - with expectation: - _validate_n_workers(None, None, value) + @pytask.mark.produces("out_1.txt") + def task_1(produces): + produces.write_text("1") + """ + tmp_path.joinpath("task_dummy.py").write_text(textwrap.dedent(source)) + + subprocess.run(["pytask", tmp_path.as_posix(), "-n", "2", "--delay", "5"]) diff --git a/tests/test_execute.py b/tests/test_execute.py index de1e7d2..7eeb836 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -94,3 +94,31 @@ def myfunc(): executor.shutdown() assert future.result() is None + + +@pytest.mark.end_to_end +@pytest.mark.parametrize("parallel_backend", ["processes", "threads"]) +def test_parallel_execution_delay(tmp_path, parallel_backend): + source = """ + import pytask + + @pytask.mark.produces("out_1.txt") + def task_1(produces): + produces.write_text("1") + + @pytask.mark.produces("out_2.txt") + def task_2(produces): + produces.write_text("2") + """ + tmp_path.joinpath("task_dummy.py").write_text(textwrap.dedent(source)) + + session = main( + { + "paths": tmp_path, + "delay": 3, + "n_workers": 2, + "parallel_backend": parallel_backend, + } + ) + + assert 3 < session.execution_end - session.execution_start < 10 From abe0a6a858955bea7aa0e68fdf81e73a5ef8b735 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 14 Aug 2020 07:44:46 +0200 Subject: [PATCH 4/7] more callbacks. --- src/pytask_parallel/callbacks.py | 30 ++++++++++++++++++++++++++++++ src/pytask_parallel/cli.py | 4 +++- src/pytask_parallel/config.py | 9 ++------- tests/test_callbacks.py | 20 +++++++++++++++++++- 4 files changed, 54 insertions(+), 9 deletions(-) diff --git a/src/pytask_parallel/callbacks.py b/src/pytask_parallel/callbacks.py index 5e540e9..44f2035 100644 --- a/src/pytask_parallel/callbacks.py +++ b/src/pytask_parallel/callbacks.py @@ -26,3 +26,33 @@ def n_workers_callback(value): raise click.UsageError("n-processes can either be an integer >= 1 or 'auto'.") return value + + +def parallel_backend_callback(value): + if value not in ["processes", "threads"]: + raise click.UsageError("parallel_backend has to be 'processes' or 'threads'.") + return value + + +def delay_click_callback(ctx, name, value): # noqa: U100 + return delay_callback(value) + + +def delay_callback(value): + if isinstance(value, float) and 0 < value: + pass + elif value is None: + pass + else: + try: + value = float(value) + except ValueError: + error_occurred = True + else: + if value < 0: + error_occurred = True + + if error_occurred: + raise click.UsageError("delay has to be a number greater than 0.") + + return value diff --git a/src/pytask_parallel/cli.py b/src/pytask_parallel/cli.py index 2c3e2a4..bb1fec8 100644 --- a/src/pytask_parallel/cli.py +++ b/src/pytask_parallel/cli.py @@ -1,5 +1,6 @@ import click from _pytask.config import hookimpl +from pytask_parallel.callbacks import delay_click_callback from pytask_parallel.callbacks import n_workers_click_callback @@ -22,11 +23,12 @@ def pytask_add_parameters_to_cli(command): ), click.Option( ["--delay"], - type=float, help=( "Delay between checking whether tasks have finished. [default: 0.1 " "(seconds)]" ), + metavar="NUMBER > 0", + callback=delay_click_callback, ), ] command.params.extend(additional_parameters) diff --git a/src/pytask_parallel/config.py b/src/pytask_parallel/config.py index 7172c8a..e06050f 100644 --- a/src/pytask_parallel/config.py +++ b/src/pytask_parallel/config.py @@ -3,6 +3,7 @@ from _pytask.config import hookimpl from _pytask.shared import get_first_not_none_value from pytask_parallel.callbacks import n_workers_callback +from pytask_parallel.callbacks import parallel_backend_callback @hookimpl @@ -26,7 +27,7 @@ def pytask_parse_config(config, config_from_cli, config_from_file): config_from_file, key="parallel_backend", default="processes", - callback=_parallel_backend_callback, + callback=parallel_backend_callback, ) @@ -35,9 +36,3 @@ def pytask_post_parse(config): """Disable parallelization if debugging is enabled.""" if config["pdb"] or config["trace"]: config["n_workers"] = 1 - - -def _parallel_backend_callback(x): - if x not in ["processes", "threads"]: - raise ValueError("parallel_backend has to be 'processes' or 'threads'.") - return x diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index f60c8d2..7ec04a5 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -5,6 +5,7 @@ import pytest from pytask_parallel.callbacks import n_workers_callback from pytask_parallel.callbacks import n_workers_click_callback +from pytask_parallel.callbacks import parallel_backend_callback partialed_callback = functools.partial(n_workers_click_callback, ctx=None, name=None) @@ -19,9 +20,26 @@ (2, does_not_raise()), ("auto", does_not_raise()), ("asdad", pytest.raises(click.UsageError)), + (None, does_not_raise()), ], ) @pytest.mark.parametrize("func", [n_workers_callback, partialed_callback]) -def test_validate_n_workers(func, value, expectation): +def test_validate_n_workers_callback(func, value, expectation): with expectation: func(value=value) + + +@pytest.mark.unit +@pytest.mark.parametrize( + "value, expectation", + [ + ("threads", does_not_raise()), + ("processes", does_not_raise()), + (1, pytest.raises(click.UsageError)), + ("asdad", pytest.raises(click.UsageError)), + (None, does_not_raise()), + ], +) +def test_parallel_backend_callback(value, expectation): + with expectation: + parallel_backend_callback(value) From cf26c78c998677b94ce1641a052fd42e517ebac8 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 14 Aug 2020 07:48:50 +0200 Subject: [PATCH 5/7] fix. --- src/pytask_parallel/callbacks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pytask_parallel/callbacks.py b/src/pytask_parallel/callbacks.py index 44f2035..fb21104 100644 --- a/src/pytask_parallel/callbacks.py +++ b/src/pytask_parallel/callbacks.py @@ -29,7 +29,7 @@ def n_workers_callback(value): def parallel_backend_callback(value): - if value not in ["processes", "threads"]: + if value not in ["processes", "threads", None]: raise click.UsageError("parallel_backend has to be 'processes' or 'threads'.") return value From a89a025fd67c4760bdda436330b1d908cc0b8158 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 14 Aug 2020 08:27:02 +0200 Subject: [PATCH 6/7] more tests. --- CHANGES.rst | 3 ++- src/pytask_parallel/callbacks.py | 1 + tests/test_callbacks.py | 30 +++++++++++++++++++++++++++--- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 5ee5a50..0c04115 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,7 +9,8 @@ all releases are available on `Anaconda.org Date: Fri, 14 Aug 2020 08:35:18 +0200 Subject: [PATCH 7/7] categorize tests. --- tests/test_cli.py | 3 +++ tests/test_config.py | 1 + 2 files changed, 4 insertions(+) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9f43f7b..26f90aa 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,7 +1,10 @@ import subprocess import textwrap +import pytest + +@pytest.mark.end_to_end def test_delay_via_cli(tmp_path): source = """ import pytask diff --git a/tests/test_config.py b/tests/test_config.py index fe33a39..e1180c8 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -21,6 +21,7 @@ def test_interplay_between_debugging_and_parallel(tmp_path, pdb, n_workers, expe assert session.config["n_workers"] == expected +@pytest.mark.end_to_end @pytest.mark.parametrize("config_file", ["pytask.ini", "tox.ini", "setup.cfg"]) @pytest.mark.parametrize( "name, value",