From e35d5f5879a422af61cd9fbb0c081c58a4385a19 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 21 Jan 2024 23:58:49 +0100 Subject: [PATCH 01/10] Deprecate strings as values in config. --- docs/source/changes.md | 1 + src/_pytask/build.py | 5 ++--- src/_pytask/dag_command.py | 6 +----- src/_pytask/parameters.py | 8 +++++--- src/_pytask/shared.py | 15 +-------------- tests/test_config.py | 5 ++--- 6 files changed, 12 insertions(+), 28 deletions(-) diff --git a/docs/source/changes.md b/docs/source/changes.md index 9c89f1f8..4d2cee36 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -12,6 +12,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`551` removes the deprecated `@pytask.mark.depends_on` and `@pytask.mark.produces`. - {pull}`552` removes the deprecated `@pytask.mark.task`. +- {pull}`553` deprecates `paths` as a string in configuration. ## 0.4.5 - 2024-01-09 diff --git a/src/_pytask/build.py b/src/_pytask/build.py index 45444992..83a06ddb 100644 --- a/src/_pytask/build.py +++ b/src/_pytask/build.py @@ -80,7 +80,7 @@ def build( # noqa: C901, PLR0912, PLR0913, PLR0915 marker_expression: str = "", max_failures: float = float("inf"), n_entries_in_table: int = 15, - paths: str | Path | Iterable[str | Path] = (), + paths: Path | Iterable[Path] = (), pdb: bool = False, pdb_cls: str = "", s: bool = False, @@ -225,13 +225,12 @@ def build( # noqa: C901, PLR0912, PLR0913, PLR0915 raw_config = {**DEFAULTS_FROM_CLI, **raw_config} - raw_config["paths"] = parse_paths(raw_config.get("paths")) + raw_config["paths"] = parse_paths(raw_config["paths"]) if raw_config["config"] is not None: raw_config["config"] = Path(raw_config["config"]).resolve() raw_config["root"] = raw_config["config"].parent else: - raw_config["paths"] = parse_paths(raw_config["paths"]) ( raw_config["root"], raw_config["config"], diff --git a/src/_pytask/dag_command.py b/src/_pytask/dag_command.py index 1dbcdb92..8906bb4c 100644 --- a/src/_pytask/dag_command.py +++ b/src/_pytask/dag_command.py @@ -154,16 +154,12 @@ def build_dag(raw_config: dict[str, Any]) -> nx.DiGraph: raw_config = {**DEFAULTS_FROM_CLI, **raw_config} - raw_config["paths"] = parse_paths(raw_config.get("paths")) + raw_config["paths"] = parse_paths(raw_config["paths"]) if raw_config["config"] is not None: raw_config["config"] = Path(raw_config["config"]).resolve() raw_config["root"] = raw_config["config"].parent else: - if raw_config["paths"] is None: - raw_config["paths"] = (Path.cwd(),) - - raw_config["paths"] = parse_paths(raw_config["paths"]) ( raw_config["root"], raw_config["config"], diff --git a/src/_pytask/parameters.py b/src/_pytask/parameters.py index 555327fc..7805ed32 100644 --- a/src/_pytask/parameters.py +++ b/src/_pytask/parameters.py @@ -56,7 +56,7 @@ _PATH_ARGUMENT = click.Argument( ["paths"], nargs=-1, - type=click.Path(exists=True, resolve_path=True), + type=click.Path(exists=True, resolve_path=True, path_type=Path), is_eager=True, ) """click.Argument: An argument for paths.""" @@ -180,9 +180,11 @@ def pytask_add_hooks(pm: PluginManager) -> None: def pytask_extend_command_line_interface(cli: click.Group) -> None: """Register general markers.""" for command in ("build", "clean", "collect", "dag", "profile"): - cli.commands[command].params.extend((_PATH_ARGUMENT, _DATABASE_URL_OPTION)) + cli.commands[command].params.extend((_DATABASE_URL_OPTION,)) for command in ("build", "clean", "collect", "dag", "markers", "profile"): - cli.commands[command].params.extend((_CONFIG_OPTION, _HOOK_MODULE_OPTION)) + cli.commands[command].params.extend( + (_CONFIG_OPTION, _HOOK_MODULE_OPTION, _PATH_ARGUMENT) + ) for command in ("build", "clean", "collect", "profile"): cli.commands[command].params.extend([_IGNORE_OPTION, _EDITOR_URL_SCHEME_OPTION]) for command in ("build",): diff --git a/src/_pytask/shared.py b/src/_pytask/shared.py index 058e51ae..40e10970 100644 --- a/src/_pytask/shared.py +++ b/src/_pytask/shared.py @@ -2,7 +2,6 @@ from __future__ import annotations import glob -import warnings from pathlib import Path from typing import Any from typing import Iterable @@ -56,20 +55,8 @@ def to_list(scalar_or_iter: Any) -> list[Any]: ) -def parse_paths(x: Any | None) -> list[Path] | None: +def parse_paths(x: Path | list[Path]) -> list[Path]: """Parse paths.""" - if x is None: - return None - - if isinstance(x, str): - msg = ( - "Specifying paths as a string in 'pyproject.toml' is deprecated and will " - "result in an error in v0.5. Please use a list of strings instead: " - f'["{x}"].' - ) - warnings.warn(msg, category=FutureWarning, stacklevel=1) - x = [x] - paths = [Path(p) for p in to_list(x)] for p in paths: if not p.exists(): diff --git a/tests/test_config.py b/tests/test_config.py index 5e7adf29..50517fd4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -65,10 +65,9 @@ def test_passing_paths_via_configuration_file(tmp_path, file_or_folder): def test_not_existing_path_in_config(runner, tmp_path): config = """ [tool.pytask.ini_options] - paths = "not_existing_path" + paths = ["not_existing_path"] """ tmp_path.joinpath("pyproject.toml").write_text(textwrap.dedent(config)) - with pytest.warns(FutureWarning, match="Specifying paths as a string"): - result = runner.invoke(cli, [tmp_path.as_posix()]) + result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.CONFIGURATION_FAILED From e5740c5b3fd53592fac3d8d2430a24af7c76e36f Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 21 Jan 2024 23:59:29 +0100 Subject: [PATCH 02/10] update config. --- docs/source/reference_guides/configuration.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/source/reference_guides/configuration.md b/docs/source/reference_guides/configuration.md index 378f0138..9564298f 100644 --- a/docs/source/reference_guides/configuration.md +++ b/docs/source/reference_guides/configuration.md @@ -187,13 +187,7 @@ command line, you can add the paths to the configuration file. Paths passed via command line will overwrite the configuration value. ```toml - -# For single entries only. -paths = "src" - -# Or single and multiple entries. paths = ["folder_1", "folder_2/task_2.py"] - ``` ```` From a8d709f618e597f300a651c6c47b83af29b8c2a0 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 22 Jan 2024 00:38:17 +0100 Subject: [PATCH 03/10] Add test for nested path parsing from config. --- tests/test_config.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index 50517fd4..29c97c4a 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,6 @@ from __future__ import annotations +import subprocess import textwrap import pytest @@ -71,3 +72,23 @@ def test_not_existing_path_in_config(runner, tmp_path): result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.CONFIGURATION_FAILED + + +def test_paths_are_relative_to_configuration_file(tmp_path): + tmp_path.joinpath("src").mkdir() + tmp_path.joinpath("tasks").mkdir() + config = """ + [tool.pytask.ini_options] + paths = ["tasks"] + """ + tmp_path.joinpath("src", "pyproject.toml").write_text(textwrap.dedent(config)) + + source = "def task_example(): ..." + tmp_path.joinpath("tasks", "task_example.py").write_text(source) + + result = subprocess.run( + ("pytask", "src"), cwd=tmp_path, check=False, capture_output=True + ) + + assert result.returncode == ExitCode.OK + assert "1 Succeeded" in result.stdout.decode() From 5ec37fc2146f2bd27bcd76a305a0bad77b2e0d5d Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Wed, 24 Jan 2024 19:44:39 +0100 Subject: [PATCH 04/10] Fix. --- src/_pytask/config_utils.py | 5 +++++ tests/test_config.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/_pytask/config_utils.py b/src/_pytask/config_utils.py index 2b6bf6f1..b85c5321 100644 --- a/src/_pytask/config_utils.py +++ b/src/_pytask/config_utils.py @@ -56,6 +56,11 @@ def set_defaults_from_config( return None config_from_file = read_config(context.params["config"]) + if "paths" in config_from_file: + config_from_file["paths"] = [ + context.params["config"].parent.joinpath(p).resolve() + for p in config_from_file.get("paths", []) + ] if context.default_map is None: context.default_map = {} diff --git a/tests/test_config.py b/tests/test_config.py index 29c97c4a..42d4d402 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -79,7 +79,7 @@ def test_paths_are_relative_to_configuration_file(tmp_path): tmp_path.joinpath("tasks").mkdir() config = """ [tool.pytask.ini_options] - paths = ["tasks"] + paths = ["../tasks"] """ tmp_path.joinpath("src", "pyproject.toml").write_text(textwrap.dedent(config)) From acf833b1fcc7ff8869dcca9b69527ad97d08f642 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Wed, 24 Jan 2024 22:15:35 +0100 Subject: [PATCH 05/10] Skip errors. --- src/_pytask/config_utils.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/_pytask/config_utils.py b/src/_pytask/config_utils.py index b85c5321..6fd3488e 100644 --- a/src/_pytask/config_utils.py +++ b/src/_pytask/config_utils.py @@ -57,10 +57,14 @@ def set_defaults_from_config( config_from_file = read_config(context.params["config"]) if "paths" in config_from_file: - config_from_file["paths"] = [ - context.params["config"].parent.joinpath(p).resolve() - for p in config_from_file.get("paths", []) - ] + try: + config_from_file["paths"] = [ + context.params["config"].parent.joinpath(p).resolve() + for p in config_from_file["paths"] + ] + except Exception: # noqa: BLE001 + msg = "'paths' in config must be a list of strings" + raise click.BadParameter(msg) from None if context.default_map is None: context.default_map = {} From 49eff4d071ca36e8a7e56c00e9eeeb7a22702d17 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 27 Jan 2024 11:29:38 +0100 Subject: [PATCH 06/10] Fix. --- src/_pytask/config_utils.py | 18 +++++++++--------- tests/test_config.py | 31 +++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/_pytask/config_utils.py b/src/_pytask/config_utils.py index 6fd3488e..7c2129e2 100644 --- a/src/_pytask/config_utils.py +++ b/src/_pytask/config_utils.py @@ -56,15 +56,6 @@ def set_defaults_from_config( return None config_from_file = read_config(context.params["config"]) - if "paths" in config_from_file: - try: - config_from_file["paths"] = [ - context.params["config"].parent.joinpath(p).resolve() - for p in config_from_file["paths"] - ] - except Exception: # noqa: BLE001 - msg = "'paths' in config must be a list of strings" - raise click.BadParameter(msg) from None if context.default_map is None: context.default_map = {} @@ -149,4 +140,13 @@ def read_config( for section in sections_: config = config[section] + if "paths" in config: + try: + config["paths"] = [ + path.parent.joinpath(p).resolve() for p in config["paths"] + ] + except Exception: # noqa: BLE001 + msg = "'paths' in config must be a list of strings" + raise click.BadParameter(msg) from None + return config diff --git a/tests/test_config.py b/tests/test_config.py index 42d4d402..edeb4573 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -47,7 +47,7 @@ def test_pass_config_to_cli(tmp_path): def test_passing_paths_via_configuration_file(tmp_path, file_or_folder): config = f""" [tool.pytask.ini_options] - paths = "{file_or_folder}" + paths = ["{file_or_folder}"] """ tmp_path.joinpath("pyproject.toml").write_text(textwrap.dedent(config)) @@ -74,7 +74,7 @@ def test_not_existing_path_in_config(runner, tmp_path): assert result.exit_code == ExitCode.CONFIGURATION_FAILED -def test_paths_are_relative_to_configuration_file(tmp_path): +def test_paths_are_relative_to_configuration_file_cli(tmp_path): tmp_path.joinpath("src").mkdir() tmp_path.joinpath("tasks").mkdir() config = """ @@ -92,3 +92,30 @@ def test_paths_are_relative_to_configuration_file(tmp_path): assert result.returncode == ExitCode.OK assert "1 Succeeded" in result.stdout.decode() + + +def test_paths_are_relative_to_configuration_file(tmp_path): + tmp_path.joinpath("src").mkdir() + tmp_path.joinpath("tasks").mkdir() + config = """ + [tool.pytask.ini_options] + paths = ["../tasks"] + """ + tmp_path.joinpath("src", "pyproject.toml").write_text(textwrap.dedent(config)) + + source = "def task_example(): ..." + tmp_path.joinpath("tasks", "task_example.py").write_text(source) + + source = """ + from pytask import build + from pathlib import Path + + session = build(paths=[Path("src")]) + """ + tmp_path.joinpath("script.py").write_text(source) + result = subprocess.run( + ("python", "script.py"), cwd=tmp_path, check=False, capture_output=True + ) + + assert result.returncode == ExitCode.OK + assert "1 Succeeded" in result.stdout.decode() From fc9a70362b86afc0560f1b7a70151c97ae493973 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 28 Jan 2024 14:12:50 +0100 Subject: [PATCH 07/10] Simplify. --- src/_pytask/config_utils.py | 16 ++++++++-------- tests/test_config.py | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/_pytask/config_utils.py b/src/_pytask/config_utils.py index 7c2129e2..efeecfaf 100644 --- a/src/_pytask/config_utils.py +++ b/src/_pytask/config_utils.py @@ -140,13 +140,13 @@ def read_config( for section in sections_: config = config[section] - if "paths" in config: - try: - config["paths"] = [ - path.parent.joinpath(p).resolve() for p in config["paths"] - ] - except Exception: # noqa: BLE001 - msg = "'paths' in config must be a list of strings" - raise click.BadParameter(msg) from None + # Only convert paths when possible. Otherwise, we defer the error until the click + # takes over. + if ( + "paths" in config + and isinstance(config["paths"], list) + and all(isinstance(p, str) for p in config["paths"]) + ): + config["paths"] = [path.parent.joinpath(p).resolve() for p in config["paths"]] return config diff --git a/tests/test_config.py b/tests/test_config.py index edeb4573..c4eb2412 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -112,7 +112,7 @@ def test_paths_are_relative_to_configuration_file(tmp_path): session = build(paths=[Path("src")]) """ - tmp_path.joinpath("script.py").write_text(source) + tmp_path.joinpath("script.py").write_text(textwrap.dedent(source)) result = subprocess.run( ("python", "script.py"), cwd=tmp_path, check=False, capture_output=True ) From 93b1f79b486125f3bb911fcccb01d4c83d79dfbd Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 28 Jan 2024 14:13:43 +0100 Subject: [PATCH 08/10] Clarify changes. --- docs/source/changes.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/source/changes.md b/docs/source/changes.md index 4d2cee36..c22df635 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -12,7 +12,9 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`551` removes the deprecated `@pytask.mark.depends_on` and `@pytask.mark.produces`. - {pull}`552` removes the deprecated `@pytask.mark.task`. -- {pull}`553` deprecates `paths` as a string in configuration. +- {pull}`553` deprecates `paths` as a string in configuration and ensures that paths + passed via the command line are relative to CWD and paths in the configuration + relative to the config file. ## 0.4.5 - 2024-01-09 From 2bba918df4996e16843224b09a955b8f1348a344 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 28 Jan 2024 14:23:04 +0100 Subject: [PATCH 09/10] fix. --- tests/test_config.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_config.py b/tests/test_config.py index c4eb2412..fea11a1b 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,6 +1,8 @@ from __future__ import annotations +import os import subprocess +import sys import textwrap import pytest @@ -94,6 +96,10 @@ def test_paths_are_relative_to_configuration_file_cli(tmp_path): assert "1 Succeeded" in result.stdout.decode() +@pytest.mark.skipif( + sys.platform == "win32" and os.environ.get("CI"), + reason="Windows does not pick up the right Python interpreter.", +) def test_paths_are_relative_to_configuration_file(tmp_path): tmp_path.joinpath("src").mkdir() tmp_path.joinpath("tasks").mkdir() From 5a5294cf0d21f3d091be98987a16f492720d649d Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 28 Jan 2024 14:28:46 +0100 Subject: [PATCH 10/10] fix. --- tests/test_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index fea11a1b..2eb5300d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -97,7 +97,7 @@ def test_paths_are_relative_to_configuration_file_cli(tmp_path): @pytest.mark.skipif( - sys.platform == "win32" and os.environ.get("CI"), + sys.platform == "win32" and os.environ.get("CI") == "true", reason="Windows does not pick up the right Python interpreter.", ) def test_paths_are_relative_to_configuration_file(tmp_path):