Forbid strings as values for paths in config file. (#553)

Author: tobiasraabe

docs/source/changes.md

@@ -12,6 +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 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
 

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"]
-
 ```
 ````
 

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"],

src/_pytask/config_utils.py

@@ -140,4 +140,13 @@ def read_config(
     for section in sections_:
         config = config[section]
 
+    # 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

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"],

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",):

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():

tests/test_config.py

@@ -1,5 +1,8 @@
 from __future__ import annotations
 
+import os
+import subprocess
+import sys
 import textwrap
 
 import pytest
@@ -46,7 +49,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))
 
@@ -65,10 +68,60 @@ 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
+
+
+def test_paths_are_relative_to_configuration_file_cli(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()
+
+
+@pytest.mark.skipif(
+    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):
+    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(textwrap.dedent(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()