From 1c9cd5db7b382aaeb3ef5d6341b231b4495107f3 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 11 Jun 2023 12:52:25 +0200 Subject: [PATCH 01/12] Remove Python 3.7 support and add a new action for mamba. (#323) --- .github/workflows/main.yml | 28 +++++++++++++++------------- .pre-commit-config.yaml | 2 +- docs/rtd_environment.yml | 2 +- docs/source/changes.md | 6 +++++- environment.yml | 2 +- pyproject.toml | 2 +- setup.cfg | 2 +- src/_pytask/capture.py | 9 +-------- src/_pytask/nodes.py | 2 +- src/_pytask/path.py | 2 +- tests/test_debugging.py | 1 - 11 files changed, 28 insertions(+), 30 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 1a9975035..fbbdbc997 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -27,20 +27,22 @@ jobs: fail-fast: false matrix: os: ['ubuntu-latest', 'macos-latest', 'windows-latest'] - python-version: ['3.7', '3.8', '3.9', '3.10', '3.11'] + python-version: ['3.8', '3.9', '3.10', '3.11'] steps: - uses: actions/checkout@v3 - - uses: conda-incubator/setup-miniconda@v2.2.0 + - uses: mamba-org/setup-micromamba@v1 with: - auto-update-conda: false - python-version: ${{ matrix.python-version }} - channels: conda-forge,nodefaults - miniforge-variant: Mambaforge - - - name: Install core dependencies. - shell: bash -l {0} - run: mamba install -c conda-forge tox-conda coverage + environment-name: gha-testing + condarc: | + channels: + - nodefaults + - conda-forge + create-args: >- + python=${{ matrix.python-version }} + mamba + tox-conda + cache-environment: true # Unit, integration, and end-to-end tests. @@ -49,7 +51,7 @@ jobs: run: tox -e pytest -- -m "unit or (not integration and not end_to_end)" --cov=src --cov=tests --cov-report=xml -n auto - name: Upload coverage report for unit tests and doctests. - if: runner.os == 'Linux' && matrix.python-version == '3.8' + if: runner.os == 'Linux' && matrix.python-version == '3.10' shell: bash -l {0} run: bash <(curl -s https://codecov.io/bash) -F unit -c @@ -58,7 +60,7 @@ jobs: run: tox -e pytest -- -m integration --cov=src --cov=tests --cov-report=xml -n auto - name: Upload coverage reports of integration tests. - if: runner.os == 'Linux' && matrix.python-version == '3.8' + if: runner.os == 'Linux' && matrix.python-version == '3.10' shell: bash -l {0} run: bash <(curl -s https://codecov.io/bash) -F integration -c @@ -67,6 +69,6 @@ jobs: run: tox -e pytest -- -m end_to_end --cov=src --cov=tests --cov-report=xml -n auto - name: Upload coverage reports of end-to-end tests. - if: runner.os == 'Linux' && matrix.python-version == '3.8' + if: runner.os == 'Linux' && matrix.python-version == '3.10' shell: bash -l {0} run: bash <(curl -s https://codecov.io/bash) -F end_to_end -c diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 329859e21..dac512006 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,7 +30,7 @@ repos: rev: v3.9.0 hooks: - id: reorder-python-imports - args: [--py37-plus, --add-import, 'from __future__ import annotations'] + args: [--py38-plus, --add-import, 'from __future__ import annotations'] - repo: https://github.com/asottile/setup-cfg-fmt rev: v2.3.0 hooks: diff --git a/docs/rtd_environment.yml b/docs/rtd_environment.yml index 0808dd98f..803d0221b 100644 --- a/docs/rtd_environment.yml +++ b/docs/rtd_environment.yml @@ -3,7 +3,7 @@ channels: - nodefaults dependencies: - - python >=3.7 + - python >=3.8 - pip - setuptools_scm - toml diff --git a/docs/source/changes.md b/docs/source/changes.md index f0b23b6e3..c0b83b04e 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -5,7 +5,11 @@ chronological order. Releases follow [semantic versioning](https://semver.org/) releases are available on [PyPI](https://pypi.org/project/pytask) and [Anaconda.org](https://anaconda.org/conda-forge/pytask). -## 0.3.2 - 2023-xx-xx +## 0.4.0 - 2023-xx-xx + +- {pull}`323` remove Python 3.7 support and use a new Github action to provide mamba. + +## 0.3.2 - 2023-06-07 - {pull}`345` updates the version numbers in animations. - {pull}`352` publishes `db` that is required by pytask-environment. diff --git a/environment.yml b/environment.yml index 9d8019694..09a9838d6 100644 --- a/environment.yml +++ b/environment.yml @@ -5,7 +5,7 @@ channels: - nodefaults dependencies: - - python >=3.7 + - python >=3.8 - pip - setuptools_scm - toml diff --git a/pyproject.toml b/pyproject.toml index fe63803ce..8dc8092d3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,7 +29,7 @@ ignore-words-list = "falsy, hist, ines, unparseable" [tool.ruff] -target-version = "py37" +target-version = "py38" select = ["ALL"] fix = true extend-ignore = [ diff --git a/setup.cfg b/setup.cfg index 6c065c0ee..09600d21f 100644 --- a/setup.cfg +++ b/setup.cfg @@ -40,7 +40,7 @@ install_requires = pybaum>=0.1.1 rich tomli>=1.0.0 -python_requires = >=3.7 +python_requires = >=3.8 include_package_data = True package_dir = =src diff --git a/src/_pytask/capture.py b/src/_pytask/capture.py index a4bf76e68..777c76c79 100644 --- a/src/_pytask/capture.py +++ b/src/_pytask/capture.py @@ -34,6 +34,7 @@ from tempfile import TemporaryFile from typing import Any from typing import AnyStr +from typing import final from typing import Generator from typing import Generic from typing import Iterator @@ -46,14 +47,6 @@ from _pytask.nodes import Task -if sys.version_info >= (3, 8): - from typing import final -else: - - def final(f: Any) -> Any: - return f - - class _CaptureMethod(enum.Enum): FD = "fd" NO = "no" diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index 53bb5e745..2d330f02d 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -98,7 +98,7 @@ def state(self) -> str | None: return None @classmethod - @functools.lru_cache() + @functools.lru_cache def from_path(cls, path: Path) -> FilePathNode: """Instantiate class from path to file. diff --git a/src/_pytask/path.py b/src/_pytask/path.py index 3e7864c93..576f2f798 100644 --- a/src/_pytask/path.py +++ b/src/_pytask/path.py @@ -100,7 +100,7 @@ def find_common_ancestor(*paths: str | Path) -> Path: return common_ancestor -@functools.lru_cache() +@functools.lru_cache def find_case_sensitive_path(path: Path, platform: str) -> Path: """Find the case-sensitive path. diff --git a/tests/test_debugging.py b/tests/test_debugging.py index 41a15f840..6687bd0d4 100644 --- a/tests/test_debugging.py +++ b/tests/test_debugging.py @@ -133,7 +133,6 @@ def task_example(depends_on): @pytest.mark.end_to_end() @pytest.mark.skipif(not IS_PEXPECT_INSTALLED, reason="pexpect is not installed.") @pytest.mark.skipif(sys.platform == "win32", reason="pexpect cannot spawn on Windows.") -@pytest.mark.skipif(sys.version_info < (3, 7), reason="breakpoint is Python 3.7+ only.") def test_breakpoint(tmp_path): source = """ def task_example(): From 82441cce59787e124414fc6db88ffd482df181d0 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 11 Jun 2023 14:09:51 +0200 Subject: [PATCH 02/12] Replace pony with sqlalchemy>=1.4.36. (#387) --- docs/rtd_environment.yml | 2 +- docs/source/changes.md | 1 + docs/source/reference_guides/configuration.md | 17 ++++ environment.yml | 2 +- setup.cfg | 2 +- src/_pytask/clean.py | 5 +- src/_pytask/click.py | 4 +- src/_pytask/dag.py | 11 +-- src/_pytask/database.py | 91 +++++-------------- src/_pytask/database_utils.py | 68 ++++++++------ src/_pytask/parameters.py | 29 +++++- src/_pytask/profile.py | 39 ++++---- src/pytask/__init__.py | 12 ++- tests/test_clean.py | 18 ++++ tests/test_click.py | 9 +- tests/test_compat.py | 6 +- tests/test_database.py | 27 +++--- tests/test_persist.py | 16 ++-- tests/test_profile.py | 15 +-- tox.ini | 2 +- 20 files changed, 203 insertions(+), 173 deletions(-) diff --git a/docs/rtd_environment.yml b/docs/rtd_environment.yml index 803d0221b..9bf674588 100644 --- a/docs/rtd_environment.yml +++ b/docs/rtd_environment.yml @@ -25,10 +25,10 @@ dependencies: - click-default-group - networkx >=2.4 - pluggy - - pony >=0.7.15 - pybaum >=0.1.1 - pexpect - rich + - sqlalchemy >=1.4.36 - tomli >=1.0.0 - pip: diff --git a/docs/source/changes.md b/docs/source/changes.md index c0b83b04e..216178049 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -8,6 +8,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and ## 0.4.0 - 2023-xx-xx - {pull}`323` remove Python 3.7 support and use a new Github action to provide mamba. +- {pull}`387` replaces pony with sqlalchemy. ## 0.3.2 - 2023-06-07 diff --git a/docs/source/reference_guides/configuration.md b/docs/source/reference_guides/configuration.md index cad669495..407553d60 100644 --- a/docs/source/reference_guides/configuration.md +++ b/docs/source/reference_guides/configuration.md @@ -42,6 +42,23 @@ are welcome to also support macOS. ```` +````{confval} database_url + +pytask uses a database to keep track of tasks, products, and dependencies over runs. By +default, it will create an SQLITE database in the project's root directory called +`.pytask.sqlite3`. If you want to use a different name or a different dialect +[supported by sqlalchemy](https://docs.sqlalchemy.org/en/latest/core/engines.html#backend-specific-urls), +use either {option}`pytask build --database-url` or `database_url` in the config. + +```toml +database_url = "sqlite:///.pytask.sqlite3" +``` + +Relative paths for SQLITE databases are interpreted as either relative to the +configuration file or the root directory. + +```` + ````{confval} editor_url_scheme Depending on your terminal, pytask is able to turn task ids into clickable links to the diff --git a/environment.yml b/environment.yml index 09a9838d6..1c58b9c53 100644 --- a/environment.yml +++ b/environment.yml @@ -16,9 +16,9 @@ dependencies: - click-default-group - networkx >=2.4 - pluggy - - pony >=0.7.15 - pybaum >=0.1.1 - rich + - sqlalchemy >=1.4.36 - tomli >=1.0.0 # Misc diff --git a/setup.cfg b/setup.cfg index 09600d21f..700c89d57 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,9 +36,9 @@ install_requires = networkx>=2.4 packaging pluggy - pony>=0.7.15 pybaum>=0.1.1 rich + sqlalchemy>=1.4.36 tomli>=1.0.0 python_requires = >=3.8 include_package_data = True diff --git a/src/_pytask/clean.py b/src/_pytask/clean.py index 7ec12f4ec..2004670e2 100644 --- a/src/_pytask/clean.py +++ b/src/_pytask/clean.py @@ -195,7 +195,10 @@ def _collect_all_paths_known_to_pytask(session: Session) -> set[Path]: if session.config["config"]: known_paths.add(session.config["config"]) known_paths.add(session.config["root"]) - known_paths.add(session.config["database_filename"]) + + database_url = session.config["database_url"] + if database_url.drivername == "sqlite" and database_url.database: + known_paths.add(Path(database_url.database)) # Add files tracked by git. if is_git_installed(): diff --git a/src/_pytask/click.py b/src/_pytask/click.py index b55fe0b63..d972cd257 100644 --- a/src/_pytask/click.py +++ b/src/_pytask/click.py @@ -242,11 +242,11 @@ def _format_help_text( # noqa: C901, PLR0912, PLR0915 if show_default_is_str or (show_default and (default_value is not None)): if show_default_is_str: - default_string = f"({param.show_default})" # type: ignore[attr-defined] + default_string = param.show_default # type: ignore[attr-defined] elif isinstance(default_value, (list, tuple)): default_string = ", ".join(str(d) for d in default_value) elif inspect.isfunction(default_value): - default_string = _("(dynamic)") + default_string = _("dynamic") elif param.is_bool_flag and param.secondary_opts: # type: ignore[attr-defined] # For boolean flags that have distinct True/False opts, # use the opt without prefix instead of the value. diff --git a/src/_pytask/dag.py b/src/_pytask/dag.py index 102e223d4..c6acf0346 100644 --- a/src/_pytask/dag.py +++ b/src/_pytask/dag.py @@ -16,6 +16,7 @@ from _pytask.dag_utils import node_and_neighbors from _pytask.dag_utils import task_and_descending_tasks from _pytask.dag_utils import TopologicalSorter +from _pytask.database_utils import DatabaseSession from _pytask.database_utils import State from _pytask.exceptions import ResolvingDependenciesError from _pytask.mark import Mark @@ -30,7 +31,6 @@ from _pytask.shared import reduce_names_of_multiple_nodes from _pytask.shared import reduce_node_name from _pytask.traceback import render_exc_info -from pony import orm from pybaum import tree_map from rich.text import Text from rich.tree import Tree @@ -126,7 +126,6 @@ def _have_task_or_neighbors_changed( ) -@orm.db_session @hookimpl(trylast=True) def pytask_dag_has_node_changed(node: MetaNode, task_name: str) -> bool: """Indicate whether a single dependency or product has changed.""" @@ -136,11 +135,11 @@ def pytask_dag_has_node_changed(node: MetaNode, task_name: str) -> bool: if file_state is None: return True + with DatabaseSession() as session: + db_state = session.get(State, (task_name, node.name)) + # If the node is not in the database. - try: - name = node.name - db_state = State[task_name, name] # type: ignore[type-arg, valid-type] - except orm.ObjectNotFound: + if db_state is None: return True # If the modification times match, the node has not been changed. diff --git a/src/_pytask/database.py b/src/_pytask/database.py index c229717e4..ea797e81f 100644 --- a/src/_pytask/database.py +++ b/src/_pytask/database.py @@ -1,86 +1,43 @@ -"""Implement the database managed with pony.""" +"""Contains hooks related to the database.""" from __future__ import annotations -import enum from pathlib import Path from typing import Any -import click -from _pytask.click import EnumChoice from _pytask.config import hookimpl from _pytask.database_utils import create_database -from click import Context - - -class _DatabaseProviders(enum.Enum): - SQLITE = "sqlite" - POSTGRES = "postgres" - MYSQL = "mysql" - ORACLE = "oracle" - COCKROACH = "cockroach" - - -def _database_filename_callback( - ctx: Context, name: str, value: str | None # noqa: ARG001 -) -> str | None: - if value is None: - return ctx.params["root"].joinpath(".pytask.sqlite3") - return value - - -@hookimpl -def pytask_extend_command_line_interface(cli: click.Group) -> None: - """Extend command line interface.""" - additional_parameters = [ - click.Option( - ["--database-provider"], - type=EnumChoice(_DatabaseProviders), - help=( - "Database provider. All providers except sqlite are considered " - "experimental." - ), - default=_DatabaseProviders.SQLITE, - ), - click.Option( - ["--database-filename"], - type=click.Path(file_okay=True, dir_okay=False, path_type=Path), - help=("Path to database relative to root."), - default=Path(".pytask.sqlite3"), - callback=_database_filename_callback, - ), - click.Option( - ["--database-create-db"], - type=bool, - help="Create database if it does not exist.", - default=True, - ), - click.Option( - ["--database-create-tables"], - type=bool, - help="Create tables if they do not exist.", - default=True, - ), - ] - cli.commands["build"].params.extend(additional_parameters) +from sqlalchemy.engine import make_url @hookimpl def pytask_parse_config(config: dict[str, Any]) -> None: """Parse the configuration.""" - if not config["database_filename"].is_absolute(): - config["database_filename"] = config["root"].joinpath( - config["database_filename"] + # Set default. + if not config["database_url"]: + config["database_url"] = make_url( + f"sqlite:///{config['root'].as_posix()}/.pytask.sqlite3" ) - config["database"] = { - "provider": config["database_provider"].value, - "filename": config["database_filename"].as_posix(), - "create_db": config["database_create_db"], - "create_tables": config["database_create_tables"], - } + if ( + config["database_url"].drivername == "sqlite" + and config["database_url"].database + ) and not Path(config["database_url"].database).is_absolute(): + if config["config"]: + full_path = ( + config["config"] + .parent.joinpath(config["database_url"].database) + .resolve() + ) + else: + full_path = ( + config["root"].joinpath(config["database_url"].database).resolve() + ) + config["database_url"] = config["database_url"]._replace( + database=full_path.as_posix() + ) @hookimpl def pytask_post_parse(config: dict[str, Any]) -> None: """Post-parse the configuration.""" - create_database(**config["database"]) + create_database(config["database_url"]) diff --git a/src/_pytask/database_utils.py b/src/_pytask/database_utils.py index 1ad974abe..486e33320 100644 --- a/src/_pytask/database_utils.py +++ b/src/_pytask/database_utils.py @@ -6,54 +6,64 @@ from _pytask.dag_utils import node_and_neighbors from _pytask.nodes import Task from _pytask.session import Session -from pony import orm +from sqlalchemy import Column +from sqlalchemy import create_engine +from sqlalchemy import String +from sqlalchemy.orm import declarative_base +from sqlalchemy.orm import sessionmaker -__all__ = ["create_database", "db", "update_states_in_database"] +__all__ = ["create_database", "update_states_in_database", "DatabaseSession"] -db = orm.Database() +DatabaseSession = sessionmaker() -class State(db.Entity): # type: ignore[name-defined] +Base = declarative_base() + + +class State(Base): # type: ignore[valid-type, misc] """Represent the state of a node in relation to a task.""" - task = orm.Required(str) - node = orm.Required(str) - modification_time = orm.Required(str) - file_hash = orm.Optional(str) + __tablename__ = "state" - orm.PrimaryKey(task, node) + task = Column(String, primary_key=True) + node = Column(String, primary_key=True) + modification_time = Column(String) + file_hash = Column(String) -def create_database( - provider: str, filename: str, *, create_db: bool, create_tables: bool -) -> None: +def create_database(url: str) -> None: """Create the database.""" try: - db.bind(provider=provider, filename=filename, create_db=create_db) - db.generate_mapping(create_tables=create_tables) - except orm.BindingError: - pass + engine = create_engine(url) + Base.metadata.create_all(bind=engine) + DatabaseSession.configure(bind=engine) + except Exception: + raise -@orm.db_session def _create_or_update_state( first_key: str, second_key: str, modification_time: str, file_hash: str ) -> None: """Create or update a state.""" - try: - state_in_db = State[first_key, second_key] # type: ignore[type-arg, valid-type] - except orm.ObjectNotFound: - State( - task=first_key, - node=second_key, - modification_time=modification_time, - file_hash=file_hash, - ) - else: - state_in_db.modification_time = modification_time - state_in_db.file_hash = file_hash + with DatabaseSession() as session: + state_in_db = session.get(State, (first_key, second_key)) + + if not state_in_db: + session.add( + State( + task=first_key, + node=second_key, + modification_time=modification_time, + file_hash=file_hash, + ) + ) + else: + state_in_db.modification_time = modification_time + state_in_db.file_hash = file_hash + + session.commit() def update_states_in_database(session: Session, task_name: str) -> None: diff --git a/src/_pytask/parameters.py b/src/_pytask/parameters.py index 3690b014e..d3fd83599 100644 --- a/src/_pytask/parameters.py +++ b/src/_pytask/parameters.py @@ -6,6 +6,10 @@ import click from _pytask.config import hookimpl from _pytask.config_utils import set_defaults_from_config +from click import Context +from sqlalchemy.engine import make_url +from sqlalchemy.engine import URL +from sqlalchemy.exc import ArgumentError _CONFIG_OPTION = click.Option( @@ -67,11 +71,34 @@ """click.Option: An option to embed URLs in task ids.""" +def _database_url_callback( + ctx: Context, name: str, value: str | None # noqa: ARG001 +) -> URL: + try: + return make_url(value) + except ArgumentError: + raise click.BadParameter( + "The 'database_url' must conform to sqlalchemy's url standard: " + "https://docs.sqlalchemy.org/en/latest/core/engines.html" + "#backend-specific-urls." + ) from None + + +_DATABASE_URL_OPTION = click.Option( + ["--database-url"], + type=str, + help=("Url to the database."), + default=None, + show_default="sqlite:///.../.pytask.sqlite3", + callback=_database_url_callback, +) + + @hookimpl(trylast=True) 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.append(_PATH_ARGUMENT) + cli.commands[command].params.extend([_PATH_ARGUMENT, _DATABASE_URL_OPTION]) for command in ("build", "clean", "collect", "dag", "markers", "profile"): cli.commands[command].params.append(_CONFIG_OPTION) for command in ("build", "clean", "collect", "profile"): diff --git a/src/_pytask/profile.py b/src/_pytask/profile.py index cd8e71102..3b6f93695 100644 --- a/src/_pytask/profile.py +++ b/src/_pytask/profile.py @@ -19,7 +19,8 @@ from _pytask.config import hookimpl from _pytask.console import console from _pytask.console import format_task_id -from _pytask.database_utils import db +from _pytask.database_utils import Base +from _pytask.database_utils import DatabaseSession from _pytask.exceptions import CollectionError from _pytask.exceptions import ConfigurationError from _pytask.nodes import FilePathNode @@ -30,8 +31,10 @@ from _pytask.report import ExecutionReport from _pytask.session import Session from _pytask.traceback import render_exc_info -from pony import orm from rich.table import Table +from sqlalchemy import Column +from sqlalchemy import Float +from sqlalchemy import String if TYPE_CHECKING: @@ -44,12 +47,14 @@ class _ExportFormats(enum.Enum): CSV = "csv" -class Runtime(db.Entity): # type: ignore[name-defined] +class Runtime(Base): # type: ignore[valid-type, misc] """Record of runtimes of tasks.""" - task = orm.PrimaryKey(str) - date = orm.Required(float) - duration = orm.Required(float) + __tablename__ = "runtime" + + task = Column(String, primary_key=True) + date = Column(Float) + duration = Column(Float) @hookimpl(tryfirst=True) @@ -84,16 +89,18 @@ def pytask_execute_task_process_report(report: ExecutionReport) -> None: _create_or_update_runtime(task.name, *duration) -@orm.db_session def _create_or_update_runtime(task_name: str, start: float, end: float) -> None: """Create or update a runtime entry.""" - try: - runtime = Runtime[task_name] # type: ignore[type-arg, valid-type] - except orm.ObjectNotFound: - Runtime(task=task_name, date=start, duration=end - start) - else: - for attr, val in (("date", start), ("duration", end - start)): - setattr(runtime, attr, val) + with DatabaseSession() as session: + runtime = session.get(Runtime, task_name) + + if not runtime: + session.add(Runtime(task=task_name, date=start, duration=end - start)) + else: + for attr, val in (("date", start), ("duration", end - start)): + setattr(runtime, attr, val) + + session.commit() @click.command(cls=ColoredCommand) @@ -198,10 +205,10 @@ def pytask_profile_add_info_on_task( profile[name]["Duration (in s)"] = round(duration, 2) -@orm.db_session def _collect_runtimes(task_names: list[str]) -> dict[str, float]: """Collect runtimes.""" - runtimes = [Runtime.get(task=task_name) for task_name in task_names] + with DatabaseSession() as session: + runtimes = [session.get(Runtime, task_name) for task_name in task_names] runtimes = [r for r in runtimes if r is not None] return {r.task: r.duration for r in runtimes} diff --git a/src/pytask/__init__.py b/src/pytask/__init__.py index 7f1407f3d..3dcdc2a9e 100644 --- a/src/pytask/__init__.py +++ b/src/pytask/__init__.py @@ -13,7 +13,9 @@ from _pytask.compat import import_optional_dependency from _pytask.config import hookimpl from _pytask.console import console -from _pytask.database_utils import db +from _pytask.database_utils import create_database +from _pytask.database_utils import DatabaseSession +from _pytask.database_utils import State from _pytask.exceptions import CollectionError from _pytask.exceptions import ConfigurationError from _pytask.exceptions import ExecutionError @@ -44,6 +46,7 @@ from _pytask.outcomes import SkippedAncestorFailed from _pytask.outcomes import SkippedUnchanged from _pytask.outcomes import TaskOutcome +from _pytask.profile import Runtime from _pytask.report import CollectionReport from _pytask.report import DagReport from _pytask.report import ExecutionReport @@ -69,6 +72,8 @@ "ColoredCommand", "ColoredGroup", "ConfigurationError", + "DagReport", + "DatabaseSession", "EnumChoice", "ExecutionError", "ExecutionReport", @@ -84,11 +89,12 @@ "Persisted", "PytaskError", "ResolvingDependenciesError", - "DagReport", + "Runtime", "Session", "Skipped", "SkippedAncestorFailed", "SkippedUnchanged", + "State", "Task", "TaskOutcome", "WarningReport", @@ -98,7 +104,7 @@ "cli", "console", "count_outcomes", - "db", + "create_database", "depends_on", "format_exception_without_traceback", "get_all_marks", diff --git a/tests/test_clean.py b/tests/test_clean.py index c29a2a074..a74752740 100644 --- a/tests/test_clean.py +++ b/tests/test_clean.py @@ -56,11 +56,29 @@ def task_write_text(produces): return tmp_path +@pytest.mark.end_to_end() +def test_clean_database_ignored(project, runner): + cwd = Path.cwd() + os.chdir(project) + result = runner.invoke(cli, ["build"]) + assert result.exit_code == ExitCode.OK + result = runner.invoke(cli, ["clean"]) + assert result.exit_code == ExitCode.OK + os.chdir(cwd) + + assert result.exit_code == ExitCode.OK + text_without_linebreaks = result.output.replace("\n", "") + assert "to_be_deleted_file_1.txt" in text_without_linebreaks + assert "to_be_deleted_file_2.txt" in text_without_linebreaks + assert ".pytask.sqlite3" not in text_without_linebreaks + + @pytest.mark.end_to_end() def test_clean_with_auto_collect(project, runner): cwd = Path.cwd() os.chdir(project) result = runner.invoke(cli, ["clean"]) + assert result.exit_code == ExitCode.OK os.chdir(cwd) assert result.exit_code == ExitCode.OK diff --git a/tests/test_click.py b/tests/test_click.py index 05b6504c4..2f0055242 100644 --- a/tests/test_click.py +++ b/tests/test_click.py @@ -12,20 +12,13 @@ def test_choices_are_displayed_in_help_page(runner): result = runner.invoke(cli, ["build", "--help"]) assert "[no|stdout|stderr|all]" in result.output - # Test that a long meta var is folded. - assert "[sqlite|postgres|mysql|oracle|cockroach]" not in result.output - assert "sqlite" in result.output - assert "postgres" in result.output - assert "mysql" in result.output - assert "oracle" in result.output - assert "cockroach" not in result.output assert "[fd|no|sys|tee-sys]" in result.output @pytest.mark.end_to_end() def test_defaults_are_displayed(runner): result = runner.invoke(cli, ["build", "--help"]) - assert "[default: True]" in result.output + assert "[default: all]" in result.output @pytest.mark.unit() diff --git a/tests/test_compat.py b/tests/test_compat.py index 394c67980..323087f0b 100644 --- a/tests/test_compat.py +++ b/tests/test_compat.py @@ -91,9 +91,9 @@ def test_import_optional(): @pytest.mark.unit() -def test_pony_version_fallback(): - pytest.importorskip("pony") - import_optional_dependency("pony") +def test_sqlalchemy_version_fallback(): + pytest.importorskip("sqlalchemy") + import_optional_dependency("sqlalchemy") @pytest.mark.unit() diff --git a/tests/test_database.py b/tests/test_database.py index a11852551..36d831d47 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -4,10 +4,11 @@ import pytest from _pytask.database_utils import create_database +from _pytask.database_utils import DatabaseSession from _pytask.database_utils import State -from pony import orm from pytask import cli from pytask import ExitCode +from sqlalchemy.engine import make_url @pytest.mark.end_to_end() @@ -30,14 +31,11 @@ def task_write(produces): assert result.exit_code == ExitCode.OK - with orm.db_session: - create_database( - "sqlite", - tmp_path.joinpath(".pytask.sqlite3").as_posix(), - create_db=True, - create_tables=False, - ) + create_database( + make_url("sqlite:///" + tmp_path.joinpath(".pytask.sqlite3").as_posix()) + ) + with DatabaseSession() as session: task_id = task_path.as_posix() + "::task_write" out_path = tmp_path.joinpath("out.txt") @@ -46,26 +44,29 @@ def task_write(produces): (in_path.as_posix(), in_path), (out_path.as_posix(), out_path), ): - modification_time = State[task_id, id_].modification_time + modification_time = session.get(State, (task_id, id_)).modification_time assert float(modification_time) == path.stat().st_mtime @pytest.mark.end_to_end() def test_rename_database_w_config(tmp_path, runner): """Modification dates of input and output files are stored in database.""" + path_to_db = tmp_path.joinpath(".db.sqlite") tmp_path.joinpath("pyproject.toml").write_text( - "[tool.pytask.ini_options]\ndatabase_filename='.db.sqlite3'" + "[tool.pytask.ini_options]\ndatabase_url='sqlite:///.db.sqlite'" ) result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.OK - tmp_path.joinpath(".db.sqlite3").exists() + assert path_to_db.exists() @pytest.mark.end_to_end() def test_rename_database_w_cli(tmp_path, runner): """Modification dates of input and output files are stored in database.""" + path_to_db = tmp_path.joinpath(".db.sqlite") result = runner.invoke( - cli, ["--database-filename", ".db.sqlite3", tmp_path.as_posix()] + cli, + ["--database-url", "sqlite:///.db.sqlite", tmp_path.as_posix()], ) assert result.exit_code == ExitCode.OK - tmp_path.joinpath(".db.sqlite3").exists() + assert path_to_db.exists() diff --git a/tests/test_persist.py b/tests/test_persist.py index 0b6bf8bb1..071155841 100644 --- a/tests/test_persist.py +++ b/tests/test_persist.py @@ -3,10 +3,10 @@ import textwrap import pytest -from _pytask.database_utils import create_database from _pytask.database_utils import State from _pytask.persist import pytask_execute_task_process_report -from pony import orm +from pytask import create_database +from pytask import DatabaseSession from pytask import ExitCode from pytask import main from pytask import Persisted @@ -63,17 +63,13 @@ def task_dummy(depends_on, produces): assert session.execution_reports[0].outcome == TaskOutcome.PERSISTENCE assert isinstance(session.execution_reports[0].exc_info[1], Persisted) - with orm.db_session: - create_database( - "sqlite", - tmp_path.joinpath(".pytask.sqlite3").as_posix(), - create_db=True, - create_tables=False, - ) + create_database("sqlite:///" + tmp_path.joinpath(".pytask.sqlite3").as_posix()) + + with DatabaseSession() as session: task_id = tmp_path.joinpath("task_module.py").as_posix() + "::task_dummy" node_id = tmp_path.joinpath("out.txt").as_posix() - modification_time = State[task_id, node_id].modification_time + modification_time = session.get(State, (task_id, node_id)).modification_time assert float(modification_time) == tmp_path.joinpath("out.txt").stat().st_mtime session = main({"paths": tmp_path}) diff --git a/tests/test_profile.py b/tests/test_profile.py index 03c5ee41e..fe241aa54 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -6,10 +6,10 @@ import pytest from _pytask.cli import cli -from _pytask.database_utils import create_database from _pytask.profile import _to_human_readable_size from _pytask.profile import Runtime -from pony import orm +from pytask import create_database +from pytask import DatabaseSession from pytask import ExitCode from pytask import main @@ -30,17 +30,12 @@ def task_example(): time.sleep(2) duration = task.attributes["duration"] assert duration[1] - duration[0] > 2 - with orm.db_session: - create_database( - "sqlite", - tmp_path.joinpath(".pytask.sqlite3").as_posix(), - create_db=True, - create_tables=False, - ) + create_database("sqlite:///" + tmp_path.joinpath(".pytask.sqlite3").as_posix()) + with DatabaseSession() as session: task_name = tmp_path.joinpath("task_example.py").as_posix() + "::task_example" - runtime = Runtime[task_name] + runtime = session.get(Runtime, task_name) assert runtime.duration > 2 diff --git a/tox.ini b/tox.ini index 1bce2eb61..e6e8b0ce1 100644 --- a/tox.ini +++ b/tox.ini @@ -21,7 +21,7 @@ conda_deps = click-default-group networkx >=2.4 pluggy - pony >=0.7.15 + sqlalchemy >=1.4.36 pybaum >=0.1.1 rich tomli >=1.0.0 From 1ec428bd723976553335e0e8689ae1d79c6dddd9 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 11 Jun 2023 14:15:25 +0200 Subject: [PATCH 03/12] Polish sqlalchemy code and docs. --- docs/source/reference_guides/configuration.md | 4 ++-- src/_pytask/database_utils.py | 13 +++++++++---- src/_pytask/profile.py | 4 ++-- src/pytask/__init__.py | 2 ++ 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/docs/source/reference_guides/configuration.md b/docs/source/reference_guides/configuration.md index 407553d60..2799d66a5 100644 --- a/docs/source/reference_guides/configuration.md +++ b/docs/source/reference_guides/configuration.md @@ -45,7 +45,7 @@ are welcome to also support macOS. ````{confval} database_url pytask uses a database to keep track of tasks, products, and dependencies over runs. By -default, it will create an SQLITE database in the project's root directory called +default, it will create an SQLite database in the project's root directory called `.pytask.sqlite3`. If you want to use a different name or a different dialect [supported by sqlalchemy](https://docs.sqlalchemy.org/en/latest/core/engines.html#backend-specific-urls), use either {option}`pytask build --database-url` or `database_url` in the config. @@ -54,7 +54,7 @@ use either {option}`pytask build --database-url` or `database_url` in the config database_url = "sqlite:///.pytask.sqlite3" ``` -Relative paths for SQLITE databases are interpreted as either relative to the +Relative paths for SQLite databases are interpreted as either relative to the configuration file or the root directory. ```` diff --git a/src/_pytask/database_utils.py b/src/_pytask/database_utils.py index 486e33320..5e62d2783 100644 --- a/src/_pytask/database_utils.py +++ b/src/_pytask/database_utils.py @@ -13,16 +13,21 @@ from sqlalchemy.orm import sessionmaker -__all__ = ["create_database", "update_states_in_database", "DatabaseSession"] +__all__ = [ + "create_database", + "update_states_in_database", + "BaseTable", + "DatabaseSession", +] DatabaseSession = sessionmaker() -Base = declarative_base() +BaseTable = declarative_base() -class State(Base): # type: ignore[valid-type, misc] +class State(BaseTable): # type: ignore[valid-type, misc] """Represent the state of a node in relation to a task.""" __tablename__ = "state" @@ -37,7 +42,7 @@ def create_database(url: str) -> None: """Create the database.""" try: engine = create_engine(url) - Base.metadata.create_all(bind=engine) + BaseTable.metadata.create_all(bind=engine) DatabaseSession.configure(bind=engine) except Exception: raise diff --git a/src/_pytask/profile.py b/src/_pytask/profile.py index 3b6f93695..faeb1e03a 100644 --- a/src/_pytask/profile.py +++ b/src/_pytask/profile.py @@ -19,7 +19,7 @@ from _pytask.config import hookimpl from _pytask.console import console from _pytask.console import format_task_id -from _pytask.database_utils import Base +from _pytask.database_utils import BaseTable from _pytask.database_utils import DatabaseSession from _pytask.exceptions import CollectionError from _pytask.exceptions import ConfigurationError @@ -47,7 +47,7 @@ class _ExportFormats(enum.Enum): CSV = "csv" -class Runtime(Base): # type: ignore[valid-type, misc] +class Runtime(BaseTable): # type: ignore[valid-type, misc] """Record of runtimes of tasks.""" __tablename__ = "runtime" diff --git a/src/pytask/__init__.py b/src/pytask/__init__.py index 3dcdc2a9e..f589e91ab 100644 --- a/src/pytask/__init__.py +++ b/src/pytask/__init__.py @@ -13,6 +13,7 @@ from _pytask.compat import import_optional_dependency from _pytask.config import hookimpl from _pytask.console import console +from _pytask.database_utils import BaseTable from _pytask.database_utils import create_database from _pytask.database_utils import DatabaseSession from _pytask.database_utils import State @@ -65,6 +66,7 @@ __all__ = [ + "BaseTable", "CollectionError", "CollectionMetadata", "CollectionOutcome", From a99e54ae099dcb01c8c94d8879505490b4455c92 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 23 Jun 2023 23:59:38 +0200 Subject: [PATCH 04/12] Remove `@pytask.mark.parametrize`. (#391) --- docs/source/_static/images/markers.svg | 3 - docs/source/_static/md/markers.md | 4 - docs/source/changes.md | 1 + docs/source/how_to_guides/index.md | 1 - ...ks_with_different_inputs_the_pytest_way.md | 213 ------- docs/source/reference_guides/api.md | 29 - docs/source/reference_guides/hookspecs.md | 15 - .../repeating_tasks_with_different_inputs.md | 45 +- pyproject.toml | 3 + src/_pytask/cli.py | 2 - src/_pytask/collect.py | 35 +- src/_pytask/hookspecs.py | 22 - src/_pytask/mark/structures.py | 12 +- src/_pytask/parametrize.py | 390 ------------- src/_pytask/parametrize_utils.py | 43 -- src/_pytask/task.py | 22 +- src/_pytask/task_utils.py | 58 +- tests/test_collect_command.py | 10 +- tests/test_live.py | 8 +- tests/test_mark.py | 20 +- tests/test_parametrize.py | 531 ------------------ tests/test_task.py | 69 ++- ...arametrize_utils.py => test_task_utils.py} | 4 +- 23 files changed, 171 insertions(+), 1369 deletions(-) delete mode 100644 docs/source/how_to_guides/repeating_tasks_with_different_inputs_the_pytest_way.md delete mode 100644 src/_pytask/parametrize.py delete mode 100644 src/_pytask/parametrize_utils.py delete mode 100644 tests/test_parametrize.py rename tests/{test_parametrize_utils.py => test_task_utils.py} (84%) diff --git a/docs/source/_static/images/markers.svg b/docs/source/_static/images/markers.svg index f27a5e7dd..5febd4cd9 100644 --- a/docs/source/_static/images/markers.svg +++ b/docs/source/_static/images/markers.svg @@ -107,9 +107,6 @@
│ pytask.mark.depends_on │ Add dependencies to a task. See this tutorial for more │
│ │ information: https://bit.ly/3JlxylS. │
│ │ │
-
│ pytask.mark.parametrize │ The marker for pytest's way of repeating tasks which is │
-
│ │ explained in this tutorial: https://bit.ly/3uqZqkk. │
-
│ │ │
│ pytask.mark.persist │ Prevent execution of a task if all products exist and even if │
│ │ something has changed (dependencies, source file, products). │
│ │ This decorator might be useful for expensive tasks where only │
diff --git a/docs/source/_static/md/markers.md b/docs/source/_static/md/markers.md index 60b3d0dfe..2ce8c7d72 100644 --- a/docs/source/_static/md/markers.md +++ b/docs/source/_static/md/markers.md @@ -10,10 +10,6 @@ $ pytask markers │ │ tutorial for more information: │ │ │ https://bit.ly/3JlxylS. │ │ │ │ -│ pytask.mark.parametrize │ The marker for pytest's way of │ -│ │ repeating tasks which is explained in │ -│ │ this tutorial: https://bit.ly/3uqZqkk. │ -│ │ │ │ pytask.mark.persist │ Prevent execution of a task if all │ │ │ products exist and even ifsomething has │ │ │ changed (dependencies, source file, │ diff --git a/docs/source/changes.md b/docs/source/changes.md index 216178049..d7fa682d7 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -9,6 +9,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`323` remove Python 3.7 support and use a new Github action to provide mamba. - {pull}`387` replaces pony with sqlalchemy. +- {pull}`391` removes `@pytask.mark.parametrize`. ## 0.3.2 - 2023-06-07 diff --git a/docs/source/how_to_guides/index.md b/docs/source/how_to_guides/index.md index 298308598..7f3c9aa56 100644 --- a/docs/source/how_to_guides/index.md +++ b/docs/source/how_to_guides/index.md @@ -14,7 +14,6 @@ maxdepth: 1 migrating_from_scripts_to_pytask invoking_pytask_extended capture_warnings -repeating_tasks_with_different_inputs_the_pytest_way how_to_influence_build_order how_to_write_a_plugin ``` diff --git a/docs/source/how_to_guides/repeating_tasks_with_different_inputs_the_pytest_way.md b/docs/source/how_to_guides/repeating_tasks_with_different_inputs_the_pytest_way.md deleted file mode 100644 index 7a81c571f..000000000 --- a/docs/source/how_to_guides/repeating_tasks_with_different_inputs_the_pytest_way.md +++ /dev/null @@ -1,213 +0,0 @@ -# Repeating tasks with different inputs - The pytest way - -:::{important} -This guide shows you how to parametrize tasks with the pytest approach. For the new and -preferred approach, see this -{doc}`tutorial <../tutorials/repeating_tasks_with_different_inputs>`. -::: - -Do you want to define a task repeating an action over a range of inputs? Parametrize -your task function! - -:::{hint} -The process of repeating a function with different inputs is called parametrizations. -::: - -:::{seealso} -If you want to know more about best practices for parametrizations, check out this -{doc}`guide <../how_to_guides/bp_scalable_repetitions_of_tasks>` after you have made -yourself familiar with this tutorial. -::: - -## An example - -We reuse the previous example of a task that generates random data and repeat the same -operation over some seeds to receive multiple, reproducible samples. - -First, we write the task for one seed. - -```python -import numpy as np -import pytask - - -@pytask.mark.produces(BLD / "data_0.pkl") -def task_create_random_data(produces): - rng = np.random.default_rng(0) - ... -``` - -In the next step, we repeat the same task over the numbers 0, 1, and 2 and pass them to -the `seed` argument. We also vary the name of the produced file in every iteration. - -```python -@pytask.mark.parametrize( - "produces, seed", - [(BLD / "data_0.pkl", 0), (BLD / "data_1.pkl", 1), (BLD / "data_2.pkl", 2)], -) -def task_create_random_data(seed, produces): - rng = np.random.default_rng(seed) - ... -``` - -The parametrize decorator receives two arguments. The first argument is -`"produces, seed"` - the signature. It is a comma-separated string where each value -specifies the name of a task function argument. - -:::{seealso} -The signature is explained in detail {ref}`below `. -::: - -The second argument of the parametrize decorator is a list with one element per -iteration. Each element must provide one value for each argument name in the signature - -two in this case. - -pytask executes the task function three times and passes the path from the list to the -argument `produces` and the seed to `seed`. - -:::{note} -If you use `produces` or `depends_on` in the signature of the parametrize decorator, the -values are handled as if they were attached to the function with -{func}`@pytask.mark.depends_on ` or -{func}`@pytask.mark.produces `. -::: - -## Un-parametrized dependencies - -To specify a dependency that is the same for all parametrizations, add it with -{func}`@pytask.mark.depends_on `. - -```python -@pytask.mark.depends_on(SRC / "common_dependency.file") -@pytask.mark.parametrize( - "produces, seed", - [(BLD / "data_0.pkl", 0), (BLD / "data_1.pkl", 1), (BLD / "data_2.pkl", 2)], -) -def task_create_random_data(seed, produces): - rng = np.random.default_rng(seed) - ... -``` - -(parametrize-signature)= - -## The signature - -pytask allows for three different kinds of formats for the signature. - -1. The signature can be a comma-separated string like an entry in a CSV table. Note that - white space is stripped from each name which you can use to separate the names for - readability. Here are some examples: - - ```python - "single_argument" - "first_argument,second_argument" - "first_argument, second_argument" - ``` - -1. The signature can be a tuple of strings where each string is one argument name. Here - is an example. - - ```python - ("first_argument", "second_argument") - ``` - -1. Finally, using a list of strings is also possible. - - ```python - ["first_argument", "second_argument"] - ``` - -## The id - -Every task has a unique id that can be used to -{doc}`select it <../tutorials/selecting_tasks>`. The normal id combines the path to the -module where the task is defined, a double colon, and the name of the task function. -Here is an example. - -``` -../task_example.py::task_example -``` - -This behavior would produce duplicate ids for parametrized tasks. Therefore, there exist -multiple mechanisms to have unique ids. - -(auto-generated-ids)= - -### Auto-generated ids - -pytask construct ids by extending the task name with representations of the values used -for each iteration. Booleans, floats, integers, and strings enter the task id directly. -For example, a task function that receives four arguments, `True`, `1.0`, `2`, and -`"hello"`, one of each dtype, has the following id. - -``` -task_example.py::task_example[True-1.0-2-hello] -``` - -Arguments with other dtypes cannot be converted to strings and, thus, are replaced with -a combination of the argument name and the iteration counter. - -For example, the following function is parametrized with tuples. - -```python -@pytask.mark.parametrize("i", [(0,), (1,)]) -def task_example(i): - pass -``` - -Since the tuples are not converted to strings, the ids of the two tasks are - -``` -task_example.py::task_example[i0] -task_example.py::task_example[i1] -``` - -### User-defined ids - -Instead of a function, you can also pass a list or another iterable of id values via -`ids`. - -This code - -```python -@pytask.mark.parametrize("i", [(0,), (1,)], ids=["first", "second"]) -def task_example(i): - pass -``` - -produces these ids - -``` -task_example.py::task_example[first] # (0,) -task_example.py::task_example[second] # (1,) -``` - -(how-to-parametrize-a-task-convert-other-objects)= - -### Convert other objects - -To change the representation of tuples and other objects, you can pass a function to the -`ids` argument of the {func}`@pytask.mark.parametrize ` -decorator. The function is called for every argument and may return a boolean, number, -or string, which will be integrated into the id. For every other return, the -auto-generated value is used. - -We can use the hash value to get a unique representation of a tuple. - -```python -def tuple_to_hash(value): - if isinstance(value, tuple): - return hash(a) - - -@pytask.mark.parametrize("i", [(0,), (1,)], ids=tuple_to_hash) -def task_example(i): - pass -``` - -The tasks have the following ids: - -``` -task_example.py::task_example[3430018387555] # (0,) -task_example.py::task_example[3430019387558] # (1,) -``` diff --git a/docs/source/reference_guides/api.md b/docs/source/reference_guides/api.md index 6254656b7..8924bb136 100644 --- a/docs/source/reference_guides/api.md +++ b/docs/source/reference_guides/api.md @@ -47,35 +47,6 @@ by the host or by plugins. The following marks are available by default. :func:`_pytask.hookspecs.pytask_collect_node` entry-point. ``` -```{eval-rst} -.. function:: pytask.mark.parametrize(arg_names, arg_values, *, ids) - - Parametrize a task function. - - Parametrizing a task allows to execute the same task with different arguments. - - :type arg_names: str | list[str] | tuple[str, ...] - :param arg_names: - The names of the arguments which can either be given as a comma-separated - string, a tuple of strings, or a list of strings. - :type arg_values: Iterable[Sequence[Any] | Any] - :param arg_values: - The values which correspond to names in ``arg_names``. For one argument, it is a - single iterable. For multiple argument names it is an iterable of iterables. - :type ids: None | (Iterable[None | str | float | int | bool] | Callable[..., Any]) - :param ids: - This argument can either be a list with ids or a function which is called with - every value passed to the parametrized function. - - If you pass an iterable with ids, make sure to only use :obj:`bool`, - :obj:`float`, :obj:`int`, or :obj:`str` as values which are used to create task - ids like ``"task_dummpy.py::task_dummy[first_task_id]"``. - - If you pass a function, the function receives each value of the parametrization - and may return a boolean, number, string or None. For the latter, the - auto-generated value is used. -``` - ```{eval-rst} .. function:: pytask.mark.persist() diff --git a/docs/source/reference_guides/hookspecs.md b/docs/source/reference_guides/hookspecs.md index 474afcb42..20e958e5e 100644 --- a/docs/source/reference_guides/hookspecs.md +++ b/docs/source/reference_guides/hookspecs.md @@ -106,21 +106,6 @@ The following hooks traverse directories and collect tasks from files. ``` -## Parametrization - -The hooks to parametrize a task are called during the collection when a function is -collected. Then, the function is duplicated according to the parametrization and the -duplicates are collected with {func}`pytask_collect_task`. - -```{eval-rst} -.. autofunction:: pytask_parametrize_task -``` - -```{eval-rst} -.. autofunction:: pytask_parametrize_kwarg_to_marker - -``` - ## Resolving Dependencies The following hooks are designed to build a DAG from tasks and dependencies and check diff --git a/docs/source/tutorials/repeating_tasks_with_different_inputs.md b/docs/source/tutorials/repeating_tasks_with_different_inputs.md index 760fc37db..9b3c12f38 100644 --- a/docs/source/tutorials/repeating_tasks_with_different_inputs.md +++ b/docs/source/tutorials/repeating_tasks_with_different_inputs.md @@ -2,16 +2,6 @@ Do you want to repeat a task over a range of inputs? Loop over your task function! -:::{important} -Before v0.2.0, pytask supported only one approach to repeat tasks. It is also called -parametrizations, and similarly to pytest, it uses a -{func}`@pytask.mark.parametrize ` decorator. If you want to -know more about it, you can find it -{doc}`here <../how_to_guides/repeating_tasks_with_different_inputs_the_pytest_way>`. - -Here you find the new and preferred approach. -::: - ## An example We reuse the task from the previous {doc}`tutorial `, which generates @@ -71,9 +61,40 @@ and the name of the task function. Here is an example. ``` This behavior would produce duplicate ids for parametrized tasks. By default, -auto-generated ids are used which are explained {ref}`here `. +auto-generated ids are used. + +(auto-generated-ids)= + +### Auto-generated ids + +pytask construct ids by extending the task name with representations of the values used +for each iteration. Booleans, floats, integers, and strings enter the task id directly. +For example, a task function that receives four arguments, `True`, `1.0`, `2`, and +`"hello"`, one of each dtype, has the following id. + +``` +task_data_preparation.py::task_create_random_data[True-1.0-2-hello] +``` + +Arguments with other dtypes cannot be converted to strings and, thus, are replaced with +a combination of the argument name and the iteration counter. -More powerful are user-defined ids. +For example, the following function is parametrized with tuples. + +```python +for i in [(0,), (1,)]: + + @pytask.mark.task + def task_create_random_data(i=i): + pass +``` + +Since the tuples are not converted to strings, the ids of the two tasks are + +``` +task_data_preparation.py::task_create_random_data[i0] +task_data_preparation.py::task_create_random_data[i1] +``` (ids)= diff --git a/pyproject.toml b/pyproject.toml index 8dc8092d3..d38fcd9ad 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,6 +62,9 @@ extend-ignore = [ "SLF001", # access private members. "S603", "S607", + # Temporary + "TD002", + "TD003", ] diff --git a/src/_pytask/cli.py b/src/_pytask/cli.py index 7d42e3551..4b6dd39e9 100644 --- a/src/_pytask/cli.py +++ b/src/_pytask/cli.py @@ -66,7 +66,6 @@ def pytask_add_hooks(pm: pluggy.PluginManager) -> None: from _pytask import mark from _pytask import nodes from _pytask import parameters - from _pytask import parametrize from _pytask import persist from _pytask import profile from _pytask import dag @@ -89,7 +88,6 @@ def pytask_add_hooks(pm: pluggy.PluginManager) -> None: pm.register(mark) pm.register(nodes) pm.register(parameters) - pm.register(parametrize) pm.register(persist) pm.register(profile) pm.register(dag) diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index 7fd1dff92..dcb22df3e 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -6,7 +6,6 @@ import os import sys import time -import warnings from pathlib import Path from typing import Any from typing import Generator @@ -106,15 +105,6 @@ def pytask_collect_file_protocol( return flat_reports -_PARAMETRIZE_DEPRECATION_WARNING = """\ -The @pytask.mark.parametrize decorator is deprecated and will be removed in pytask \ -v0.4. Either upgrade your code to the new syntax explained in \ -https://tinyurl.com/pytask-loops or silence the warning by setting \ -`silence_parametrize_deprecation = true` in your pyproject.toml under \ -[tool.pytask.ini_options] and pin pytask to <0.4. -""" - - @hookimpl def pytask_collect_file( session: Session, path: Path, reports: list[CollectionReport] @@ -129,26 +119,11 @@ def pytask_collect_file( if has_mark(obj, "task"): continue - if has_mark(obj, "parametrize"): - if not session.config.get("silence_parametrize_deprecation", False): - warnings.warn( - message=_PARAMETRIZE_DEPRECATION_WARNING, - category=FutureWarning, - stacklevel=1, - ) - - names_and_objects = session.hook.pytask_parametrize_task( - session=session, name=name, obj=obj - ) - else: - names_and_objects = [(name, obj)] - - for name_, obj_ in names_and_objects: - report = session.hook.pytask_collect_task_protocol( - session=session, reports=reports, path=path, name=name_, obj=obj_ - ) - if report is not None: - collected_reports.append(report) + report = session.hook.pytask_collect_task_protocol( + session=session, reports=reports, path=path, name=name, obj=obj + ) + if report is not None: + collected_reports.append(report) return collected_reports return None diff --git a/src/_pytask/hookspecs.py b/src/_pytask/hookspecs.py index 9a49206f2..011feb6e6 100644 --- a/src/_pytask/hookspecs.py +++ b/src/_pytask/hookspecs.py @@ -8,7 +8,6 @@ import pathlib from typing import Any -from typing import Callable from typing import TYPE_CHECKING import click @@ -211,27 +210,6 @@ def pytask_collect_log( """ -# Hooks to parametrize tasks. - - -@hookspec(firstresult=True) -def pytask_parametrize_task( - session: Session, name: str, obj: Any -) -> list[tuple[str, Callable[..., Any]]]: - """Generate multiple tasks from name and object with parametrization.""" - - -@hookspec -def pytask_parametrize_kwarg_to_marker(obj: Any, kwargs: dict[Any, Any]) -> None: - """Add some keyword arguments as markers to object. - - This hook moves arguments defined in the parametrization to marks of the same - function. This allows an argument like ``depends_on`` be transformed to the usual - ``@pytask.mark.depends_on`` marker which receives special treatment. - - """ - - # Hooks for resolving dependencies. diff --git a/src/_pytask/mark/structures.py b/src/_pytask/mark/structures.py index d4a770474..4dae51e67 100644 --- a/src/_pytask/mark/structures.py +++ b/src/_pytask/mark/structures.py @@ -196,11 +196,13 @@ def __getattr__(self, name: str) -> MarkDecorator | Any: if self.config is not None and name not in self.config["markers"]: if self.config["strict_markers"]: raise ValueError(f"Unknown pytask.mark.{name}.") - # Raise a specific error for common misspellings of "parametrize". - if name in ("parameterize", "parametrise", "parameterise"): - warnings.warn( - f"Unknown {name!r} mark, did you mean 'parametrize'?", stacklevel=1 - ) + + if name in ("parametrize", "parameterize", "parametrise", "parameterise"): + raise NotImplementedError( + "@pytask.mark.parametrize has been removed since pytask v0.4. " + "Upgrade your parametrized tasks to the new syntax defined in" + "https://tinyurl.com/pytask-loops or revert to v0.3." + ) from None warnings.warn( f"Unknown pytask.mark.{name} - is this a typo? You can register " diff --git a/src/_pytask/parametrize.py b/src/_pytask/parametrize.py deleted file mode 100644 index f3a9c2ca1..000000000 --- a/src/_pytask/parametrize.py +++ /dev/null @@ -1,390 +0,0 @@ -"""This module contains the code for the parametrize plugin.""" -from __future__ import annotations - -import copy -import functools -import itertools -import pprint -import types -from typing import Any -from typing import Callable -from typing import Iterable -from typing import Sequence - -from _pytask.config import hookimpl -from _pytask.console import format_strings_as_flat_tree -from _pytask.console import TASK_ICON -from _pytask.mark import Mark -from _pytask.mark import MARK_GEN as mark # noqa: N811 -from _pytask.mark_utils import remove_marks -from _pytask.parametrize_utils import arg_value_to_id_component -from _pytask.session import Session -from _pytask.shared import find_duplicates - - -def parametrize( - arg_names: str | list[str] | tuple[str, ...], - arg_values: Iterable[Sequence[Any] | Any], - *, - ids: None | (Iterable[None | str | float | int | bool] | Callable[..., Any]) = None, -) -> tuple[ - str | list[str] | tuple[str, ...], - Iterable[Sequence[Any] | Any], - Iterable[None | str | float | int | bool] | Callable[..., Any] | None, -]: - """Parametrize a task function. - - Parametrizing a task allows to execute the same task with different arguments. - - Parameters - ---------- - arg_names : str | list[str] | tuple[str, ...] - The names of the arguments which can either be given as a comma-separated - string, a tuple of strings, or a list of strings. - arg_values : Iterable[Sequence[Any] | Any] - The values which correspond to names in ``arg_names``. For one argument, it is a - single iterable. For multiple argument names it is an iterable of iterables. - ids - This argument can either be a list with ids or a function which is called with - every value passed to the parametrized function. - - If you pass an iterable with ids, make sure to only use :obj:`bool`, - :obj:`float`, :obj:`int`, or :obj:`str` as values which are used to create task - ids like ``"task_dummpy.py::task_dummy[first_task_id]"``. - - If you pass a function, the function receives each value of the parametrization - and may return a boolean, number, string or None. For the latter, the - auto-generated value is used. - - """ - return arg_names, arg_values, ids - - -@hookimpl -def pytask_parse_config(config: dict[str, Any]) -> None: - """Add the marker to the config.""" - config["markers"]["parametrize"] = ( - "The marker for pytest's way of repeating tasks which is explained in this " - "tutorial: [link https://bit.ly/3uqZqkk]https://bit.ly/3uqZqkk[/]." - ) - - -@hookimpl -def pytask_parametrize_task( - session: Session, name: str, obj: Callable[..., Any] -) -> list[tuple[str, Callable[..., Any]]]: - """Parametrize a task. - - This function takes a single Python function and all parametrize decorators and - generates multiple instances of the same task with different arguments. - - Note that, while a single ``@pytask.mark.parametrize`` is handled like a loop or a - :func:`zip`, multiple ``@pytask.mark.parametrize`` decorators form a Cartesian - product. - - We cannot raise an error if the function does not use parametrized arguments since - some plugins will replace functions with their own implementation like pytask-r. - - """ - if callable(obj): - obj, markers = remove_marks(obj, "parametrize") # type: ignore[assignment] - - if len(markers) > 1: - raise NotImplementedError( - "You cannot apply @pytask.mark.parametrize multiple times to a task. " - "Use multiple for-loops, itertools.product or a different strategy to " - "create all combinations of inputs and pass it to a single " - "@pytask.mark.parametrize.\n\nFor improved readability, consider to " - "move the creation of inputs into its own function as shown in the " - "best-practices guide on parametrizations: https://pytask-dev.rtfd.io/" - "en/stable/how_to_guides/bp_scalable_repetitions_of_tasks.html." - ) - - base_arg_names, arg_names, arg_values = _parse_parametrize_markers( - markers, name - ) - - product_arg_names = list(itertools.product(*arg_names)) - product_arg_values = list(itertools.product(*arg_values)) - - names_and_functions: list[tuple[str, Callable[..., Any]]] = [] - for names, values in zip(product_arg_names, product_arg_values): - kwargs = dict( - zip( - itertools.chain.from_iterable(base_arg_names), - itertools.chain.from_iterable(values), - ) - ) - - # Copy function and attributes to allow in-place changes. - func = _copy_func(obj) # type: ignore[arg-type] - func.pytask_meta = copy.deepcopy( # type: ignore[attr-defined] - obj.pytask_meta # type: ignore[attr-defined] - ) - # Convert parametrized dependencies and products to decorator. - session.hook.pytask_parametrize_kwarg_to_marker(obj=func, kwargs=kwargs) - - func.pytask_meta.kwargs = { # type: ignore[attr-defined] - **func.pytask_meta.kwargs, # type: ignore[attr-defined] - **kwargs, - } - - name_ = f"{name}[{'-'.join(itertools.chain.from_iterable(names))}]" - names_and_functions.append((name_, func)) - - all_names = [i[0] for i in names_and_functions] - duplicates = find_duplicates(all_names) - - if duplicates: - text = format_strings_as_flat_tree( - duplicates, "Duplicated task ids", TASK_ICON - ) - raise ValueError( - "The following ids are duplicated while parametrizing task " - f"{name!r}.\n\n{text}\n\nIt might be caused by " - "parametrizing the task with the same combination of arguments " - "multiple times. Change the arguments or change the ids generated by " - "the parametrization." - ) - - return names_and_functions - return None - - -def _parse_parametrize_marker( - marker: Mark, name: str -) -> tuple[tuple[str, ...], list[tuple[str, ...]], list[tuple[Any, ...]]]: - """Parse parametrize marker. - - Parameters - ---------- - marker : Mark - A parametrize mark. - name : str - The name of the task function which is parametrized. - - Returns - ------- - base_arg_names : Tuple[str, ...] - Contains the names of the arguments. - processed_arg_names : List[Tuple[str, ...]] - Each tuple in the list represents the processed names of the arguments suffixed - with a number indicating the iteration. - processed_arg_values : List[Tuple[Any, ...]] - Each tuple in the list represents the values of the arguments for each - iteration. - - """ - arg_names, arg_values, ids = parametrize(*marker.args, **marker.kwargs) - - parsed_arg_names = _parse_arg_names(arg_names) - has_single_arg = len(parsed_arg_names) == 1 - parsed_arg_values = _parse_arg_values(arg_values, has_single_arg) - - _check_if_n_arg_names_matches_n_arg_values( - parsed_arg_names, parsed_arg_values, name - ) - - expanded_arg_names = _create_parametrize_ids_components( - parsed_arg_names, parsed_arg_values, ids - ) - - return parsed_arg_names, expanded_arg_names, parsed_arg_values - - -def _parse_parametrize_markers( - markers: list[Mark], name: str -) -> tuple[ - list[tuple[str, ...]], - list[list[tuple[str, ...]]], - list[list[tuple[Any, ...]]], -]: - """Parse parametrize markers.""" - parsed_markers = [_parse_parametrize_marker(marker, name) for marker in markers] - base_arg_names = [i[0] for i in parsed_markers] - processed_arg_names = [i[1] for i in parsed_markers] - processed_arg_values = [i[2] for i in parsed_markers] - - return base_arg_names, processed_arg_names, processed_arg_values - - -def _parse_arg_names(arg_names: str | list[str] | tuple[str, ...]) -> tuple[str, ...]: - """Parse arg_names argument of parametrize decorator. - - There are three allowed formats: - - 1. comma-separated string representation. - 2. a tuple of strings. - 3. a list of strings. - - All formats are converted to a tuple of strings. - - Parameters - ---------- - arg_names : Union[str, List[str], Tuple[str, ...]] - The names of the arguments which are parametrized. - - Returns - ------- - out : Tuple[str, ...] - The parsed arg_names. - - Example - ------- - >>> _parse_arg_names("i") - ('i',) - >>> _parse_arg_names("i, j") - ('i', 'j') - - """ - if isinstance(arg_names, str): - out = tuple(i.strip() for i in arg_names.split(",")) - elif isinstance(arg_names, (tuple, list)): - out = tuple(arg_names) - else: - raise TypeError( - "The argument 'arg_names' accepts comma-separated strings, tuples and lists" - f" of strings. It cannot accept {arg_names} with type {type(arg_names)}." - ) - - return out - - -def _parse_arg_values( - arg_values: Iterable[Sequence[Any] | Any], has_single_arg: bool -) -> list[tuple[Any, ...]]: - """Parse the values provided for each argument name. - - After processing the values, the return is a list where each value is an iteration - of the parametrization. Each iteration is a tuple of all parametrized arguments. - - Example - ------- - >>> _parse_arg_values(["a", "b", "c"], has_single_arg=True) - [('a',), ('b',), ('c',)] - >>> _parse_arg_values([(0, 0), (0, 1), (1, 0)], has_single_arg=False) - [(0, 0), (0, 1), (1, 0)] - - """ - return [ - tuple(i) - if isinstance(i, Iterable) - and not isinstance(i, str) - and not (isinstance(i, dict) and has_single_arg) - else (i,) - for i in arg_values - ] - - -def _check_if_n_arg_names_matches_n_arg_values( - arg_names: tuple[str, ...], arg_values: list[tuple[Any, ...]], name: str -) -> None: - """Check if the number of argument names matches the number of arguments.""" - n_names = len(arg_names) - n_values = [len(i) for i in arg_values] - unique_n_values = tuple(set(n_values)) - - if not all(i == n_names for i in unique_n_values): - pretty_arg_values = ( - f"{unique_n_values[0]}" - if len(unique_n_values) == 1 - else " or ".join(map(str, unique_n_values)) - ) - idx_example = [i == n_names for i in n_values].index(False) - formatted_example = pprint.pformat(arg_values[idx_example]) - raise ValueError( - f"Task {name!r} is parametrized with {n_names} 'arg_names', {arg_names}, " - f"but the number of provided 'arg_values' is {pretty_arg_values}. For " - f"example, here are the values of parametrization no. {idx_example}:" - f"\n\n{formatted_example}" - ) - - -def _create_parametrize_ids_components( - arg_names: tuple[str, ...], - arg_values: list[tuple[Any, ...]], - ids: None | (Iterable[None | str | float | int | bool] | Callable[..., Any]), -) -> list[tuple[str, ...]]: - """Create the ids for each parametrization. - - Parameters - ---------- - arg_names : Tuple[str, ...] - The names of the arguments of the parametrized function. - arg_values : List[Tuple[Any, ...]] - A list of tuples where each tuple is for one run. - ids - The ids associated with one parametrization. - - Examples - -------- - >>> _create_parametrize_ids_components(["i"], [(0,), (1,)], None) - [('0',), ('1',)] - - >>> _create_parametrize_ids_components(["i", "j"], [(0, (0,)), (1, (1,))], None) - [('0', 'j0'), ('1', 'j1')] - - """ - if isinstance(ids, Iterable): - raw_ids = [(id_,) for id_ in ids] - - if len(raw_ids) != len(arg_values): - raise ValueError("The number of ids must match the number of runs.") - - if not all( - isinstance(id_, (bool, int, float, str)) or id_ is None - for id_ in itertools.chain.from_iterable(raw_ids) - ): - raise ValueError( - "Ids for parametrization can only be of type bool, float, int, str or " - "None." - ) - - parsed_ids: list[tuple[str, ...]] = [ - (str(id_),) for id_ in itertools.chain.from_iterable(raw_ids) - ] - - else: - parsed_ids = [] - for i, _arg_values in enumerate(arg_values): - id_components = tuple( - arg_value_to_id_component(arg_names[j], arg_value, i, ids) - for j, arg_value in enumerate(_arg_values) - ) - parsed_ids.append(id_components) - - return parsed_ids - - -@hookimpl -def pytask_parametrize_kwarg_to_marker(obj: Any, kwargs: dict[str, str]) -> None: - """Add some parametrized keyword arguments as decorator.""" - if callable(obj): - for marker_name in ("depends_on", "produces"): - if marker_name in kwargs: - mark.__getattr__(marker_name)(kwargs.pop(marker_name))(obj) - - -def _copy_func(func: types.FunctionType) -> types.FunctionType: - """Create a copy of a function. - - Based on https://stackoverflow.com/a/13503277/7523785. - - Example - ------- - >>> def _func(): pass - >>> copied_func = _copy_func(_func) - >>> _func is copied_func - False - - """ - new_func = types.FunctionType( - func.__code__, - func.__globals__, - name=func.__name__, - argdefs=func.__defaults__, - closure=func.__closure__, - ) - new_func = functools.update_wrapper(new_func, func) - new_func.__kwdefaults__ = func.__kwdefaults__ - return new_func diff --git a/src/_pytask/parametrize_utils.py b/src/_pytask/parametrize_utils.py deleted file mode 100644 index f259c7b31..000000000 --- a/src/_pytask/parametrize_utils.py +++ /dev/null @@ -1,43 +0,0 @@ -from __future__ import annotations - -from typing import Any -from typing import Callable - - -def arg_value_to_id_component( - arg_name: str, arg_value: Any, i: int, id_func: Callable[..., Any] | None -) -> str: - """Create id component from the name and value of the argument. - - First, transform the value of the argument with a user-defined function if given. - Otherwise, take the original value. Then, if the value is a :obj:`bool`, - :obj:`float`, :obj:`int`, or :obj:`str`, cast it to a string. Otherwise, define a - placeholder value from the name of the argument and the iteration. - - Parameters - ---------- - arg_name : str - Name of the parametrized function argument. - arg_value : Any - Value of the argument. - i : int - The ith iteration of the parametrization. - id_func : Union[Callable[..., Any], None] - A callable which maps argument values to :obj:`bool`, :obj:`float`, :obj:`int`, - or :obj:`str` or anything else. Any object with a different dtype than the first - will be mapped to an auto-generated id component. - - Returns - ------- - id_component : str - A part of the final parametrized id. - - """ - id_component = id_func(arg_value) if id_func is not None else None - if isinstance(id_component, (bool, float, int, str)): - id_component = str(id_component) - elif isinstance(arg_value, (bool, float, int, str)): - id_component = str(arg_value) - else: - id_component = arg_name + str(i) - return id_component diff --git a/src/_pytask/task.py b/src/_pytask/task.py index d71cb6689..7b806af71 100644 --- a/src/_pytask/task.py +++ b/src/_pytask/task.py @@ -5,7 +5,6 @@ from typing import Any from _pytask.config import hookimpl -from _pytask.mark_utils import has_mark from _pytask.report import CollectionReport from _pytask.session import Session from _pytask.task_utils import COLLECTED_TASKS @@ -39,24 +38,11 @@ def pytask_collect_file( collected_reports = [] for name, function in name_to_function.items(): - session.hook.pytask_parametrize_kwarg_to_marker( - obj=function, - kwargs=function.pytask_meta.kwargs, # type: ignore[attr-defined] + report = session.hook.pytask_collect_task_protocol( + session=session, reports=reports, path=path, name=name, obj=function ) - - if has_mark(function, "parametrize"): - names_and_objects = session.hook.pytask_parametrize_task( - session=session, name=name, obj=function - ) - else: - names_and_objects = [(name, function)] - - for name_, obj_ in names_and_objects: - report = session.hook.pytask_collect_task_protocol( - session=session, reports=reports, path=path, name=name_, obj=obj_ - ) - if report is not None: - collected_reports.append(report) + if report is not None: + collected_reports.append(report) return collected_reports return None diff --git a/src/_pytask/task_utils.py b/src/_pytask/task_utils.py index e8e6bd4e8..ad541c9d1 100644 --- a/src/_pytask/task_utils.py +++ b/src/_pytask/task_utils.py @@ -9,7 +9,6 @@ from _pytask.mark import Mark from _pytask.models import CollectionMetadata -from _pytask.parametrize_utils import arg_value_to_id_component from _pytask.shared import find_duplicates @@ -51,6 +50,13 @@ def task( """ def wrapper(func: Callable[..., Any]) -> None: + for arg, arg_name in ((name, "name"), (id, "id")): + if not (isinstance(arg, str) or arg is None): + raise ValueError( + f"Argument {arg_name!r} of @pytask.mark.task must be a str, but it " + f"is {arg!r}." + ) + unwrapped = inspect.unwrap(func) raw_path = inspect.getfile(unwrapped) @@ -104,6 +110,15 @@ def parse_collected_tasks_with_task_marker( else: collected_tasks[name] = [i[1] for i in parsed_tasks if i[0] == name][0] + # TODO: Remove when parsing dependencies and products from all arguments is + # implemented. + for task in collected_tasks.values(): + meta = task.pytask_meta # type: ignore[attr-defined] + for marker_name in ("depends_on", "produces"): + if marker_name in meta.kwargs: + value = meta.kwargs.pop(marker_name) + meta.markers.append(Mark(marker_name, (value,), {})) + return collected_tasks @@ -169,7 +184,7 @@ def _generate_ids_for_tasks( id_ = f"{name}[{i}]" else: stringified_args = [ - arg_value_to_id_component( + _arg_value_to_id_component( arg_name=parameter, arg_value=task.pytask_meta.kwargs.get( # type: ignore[attr-defined] parameter @@ -183,3 +198,42 @@ def _generate_ids_for_tasks( id_ = f"{name}[{id_}]" out[id_] = task return out + + +def _arg_value_to_id_component( + arg_name: str, arg_value: Any, i: int, id_func: Callable[..., Any] | None +) -> str: + """Create id component from the name and value of the argument. + + First, transform the value of the argument with a user-defined function if given. + Otherwise, take the original value. Then, if the value is a :obj:`bool`, + :obj:`float`, :obj:`int`, or :obj:`str`, cast it to a string. Otherwise, define a + placeholder value from the name of the argument and the iteration. + + Parameters + ---------- + arg_name : str + Name of the parametrized function argument. + arg_value : Any + Value of the argument. + i : int + The ith iteration of the parametrization. + id_func : Union[Callable[..., Any], None] + A callable which maps argument values to :obj:`bool`, :obj:`float`, :obj:`int`, + or :obj:`str` or anything else. Any object with a different dtype than the first + will be mapped to an auto-generated id component. + + Returns + ------- + id_component : str + A part of the final parametrized id. + + """ + id_component = id_func(arg_value) if id_func is not None else None + if isinstance(id_component, (bool, float, int, str)): + id_component = str(id_component) + elif isinstance(arg_value, (bool, float, int, str)): + id_component = str(arg_value) + else: + id_component = arg_name + str(i) + return id_component diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index 609594f98..eba87f4cc 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -81,10 +81,12 @@ def test_collect_parametrized_tasks(runner, tmp_path): source = """ import pytask - @pytask.mark.depends_on("in.txt") - @pytask.mark.parametrize("arg, produces", [(0, "out_0.txt"), (1, "out_1.txt")]) - def task_example(arg): - pass + for arg, produces in [(0, "out_0.txt"), (1, "out_1.txt")]: + + @pytask.mark.task + @pytask.mark.depends_on("in.txt") + def task_example(arg=arg, produces=produces): + pass """ tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) tmp_path.joinpath("in.txt").touch() diff --git a/tests/test_live.py b/tests/test_live.py index 851a11a45..abcfbacf7 100644 --- a/tests/test_live.py +++ b/tests/test_live.py @@ -262,9 +262,11 @@ def test_full_execution_table_is_displayed_at_the_end_of_execution(tmp_path, run source = """ import pytask - @pytask.mark.parametrize("produces", [f"{i}.txt" for i in range(4)]) - def task_create_file(produces): - produces.touch() + for produces in [f"{i}.txt" for i in range(4)]: + + @pytask.mark.task + def task_create_file(produces=produces): + produces.touch() """ # Subfolder to reduce task id and be able to check the output later. tmp_path.joinpath("d").mkdir() diff --git a/tests/test_mark.py b/tests/test_mark.py index 2b79367bc..32d468b1a 100644 --- a/tests/test_mark.py +++ b/tests/test_mark.py @@ -172,16 +172,16 @@ def task_no_2(): ], ) def test_keyword_option_parametrize(tmp_path, expr: str, expected_passed: str) -> None: - tmp_path.joinpath("task_module.py").write_text( - textwrap.dedent( - """ - import pytask - @pytask.mark.parametrize("arg", [None, 1.3, "2-3"]) - def task_func(arg): - pass - """ - ) - ) + source = """ + import pytask + + for arg in [None, 1.3, "2-3"]: + + @pytask.mark.task + def task_func(arg=arg): + pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) session = main({"paths": tmp_path, "expression": expr}) assert session.exit_code == ExitCode.OK diff --git a/tests/test_parametrize.py b/tests/test_parametrize.py deleted file mode 100644 index 37994a8b5..000000000 --- a/tests/test_parametrize.py +++ /dev/null @@ -1,531 +0,0 @@ -from __future__ import annotations - -import itertools -import textwrap -from contextlib import ExitStack as does_not_raise # noqa: N813 -from typing import NamedTuple - -import _pytask.parametrize -import pytask -import pytest -from _pytask.parametrize import _check_if_n_arg_names_matches_n_arg_values -from _pytask.parametrize import _parse_arg_names -from _pytask.parametrize import _parse_arg_values -from _pytask.parametrize import _parse_parametrize_markers -from _pytask.parametrize import pytask_parametrize_task -from _pytask.pluginmanager import get_plugin_manager -from pytask import cli -from pytask import ExitCode -from pytask import main -from pytask import Mark -from pytask import Session - - -@pytest.fixture(scope="module") -def session(): - pm = get_plugin_manager() - pm.register(_pytask.parametrize) - session = Session(hook=pm.hook) - return session - - -@pytest.mark.integration() -def test_pytask_generate_tasks_0(session): - @pytask.mark.parametrize("i", range(2)) - def func(i): # noqa: ARG001, pragma: no cover - pass - - names_and_objs = pytask_parametrize_task(session, "func", func) - - assert [i[0] for i in names_and_objs] == ["func[0]", "func[1]"] - assert names_and_objs[0][1].pytask_meta.kwargs["i"] == 0 - assert names_and_objs[1][1].pytask_meta.kwargs["i"] == 1 - - -@pytest.mark.integration() -@pytest.mark.xfail(strict=True, reason="Cartesian task product is disabled.") -def test_pytask_generate_tasks_1(session): - @pytask.mark.parametrize("j", range(2)) - @pytask.mark.parametrize("i", range(2)) - def func(i, j): # noqa: ARG001, pragma: no cover - pass - - pytask_parametrize_task(session, "func", func) - - -@pytest.mark.integration() -@pytest.mark.xfail(strict=True, reason="Cartesian task product is disabled.") -def test_pytask_generate_tasks_2(session): - @pytask.mark.parametrize("j, k", itertools.product(range(2), range(2))) - @pytask.mark.parametrize("i", range(2)) - def func(i, j, k): # noqa: ARG001, pragma: no cover - pass - - pytask_parametrize_task(session, "func", func) - - -@pytest.mark.unit() -@pytest.mark.parametrize( - ("arg_names", "expected"), - [ - ("i", ("i",)), - ("i,j", ("i", "j")), - ("i, j", ("i", "j")), - (("i", "j"), ("i", "j")), - (["i", "j"], ("i", "j")), - ], -) -def test_parse_arg_names(arg_names, expected): - parsed_arg_names = _parse_arg_names(arg_names) - assert parsed_arg_names == expected - - -class TaskArguments(NamedTuple): - a: int - b: int - - -@pytest.mark.unit() -@pytest.mark.parametrize( - ("arg_values", "has_single_arg", "expected"), - [ - (["a", "b", "c"], True, [("a",), ("b",), ("c",)]), - ([(0, 0), (0, 1), (1, 0)], False, [(0, 0), (0, 1), (1, 0)]), - ([[0, 0], [0, 1], [1, 0]], False, [(0, 0), (0, 1), (1, 0)]), - ({"a": 0, "b": 1}, False, [("a",), ("b",)]), - ([{"a": 0, "b": 1}], True, [({"a": 0, "b": 1},)]), - ([TaskArguments(1, 2)], False, [(1, 2)]), - ([TaskArguments(a=1, b=2)], False, [(1, 2)]), - ([TaskArguments(b=2, a=1)], False, [(1, 2)]), - ], -) -def test_parse_arg_values(arg_values, has_single_arg, expected): - parsed_arg_values = _parse_arg_values(arg_values, has_single_arg) - assert parsed_arg_values == expected - - -@pytest.mark.unit() -@pytest.mark.parametrize( - ("arg_names", "expectation"), - [ - ("i", does_not_raise()), - ("i, j", does_not_raise()), - (("i", "j"), does_not_raise()), - (["i", "j"], does_not_raise()), - (range(1, 2), pytest.raises(TypeError)), - ({"i": None, "j": None}, pytest.raises(TypeError)), - ({"i", "j"}, pytest.raises(TypeError)), - ], -) -def test_parse_argnames_raise_error(arg_names, expectation): - with expectation: - _parse_arg_names(arg_names) - - -@pytest.mark.integration() -@pytest.mark.parametrize( - ("markers", "exp_base_arg_names", "exp_arg_names", "exp_arg_values"), - [ - ( - [ - Mark("parametrize", ("i", range(2)), {}), - Mark("parametrize", ("j", range(2)), {}), - ], - [("i",), ("j",)], - [[("0",), ("1",)], [("0",), ("1",)]], - [[(0,), (1,)], [(0,), (1,)]], - ), - ( - [Mark("parametrize", ("i", range(3)), {})], - [("i",)], - [[("0",), ("1",), ("2",)]], - [[(0,), (1,), (2,)]], - ), - ], -) -def test_parse_parametrize_markers( - markers, exp_base_arg_names, exp_arg_names, exp_arg_values -): - base_arg_names, arg_names, arg_values = _parse_parametrize_markers(markers, "task_") - - assert base_arg_names == exp_base_arg_names - assert arg_names == exp_arg_names - assert arg_values == exp_arg_values - - -@pytest.mark.end_to_end() -def test_parametrizing_tasks(tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i, produces', [(1, "1.txt"), (2, "2.txt")]) - def task_write_numbers_to_file(produces, i): - produces.write_text(str(i)) - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.OK - for i in range(1, 3): - assert tmp_path.joinpath(f"{i}.txt").read_text() == str(i) - - -@pytest.mark.end_to_end() -def test_parametrizing_dependencies_and_targets(tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i, produces', [(1, "1.txt"), (2, "2.txt")]) - def task_save_numbers(i, produces): - produces.write_text(str(i)) - - @pytask.mark.parametrize("depends_on, produces", [ - ("1.txt", "1_out.txt"), ("2.txt", "2_out.txt") - ]) - def task_save_numbers_again(depends_on, produces): - produces.write_text(depends_on.read_text()) - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.OK - - -@pytest.mark.end_to_end() -def test_parametrize_iterator(tmp_path): - """`parametrize` should work with generators.""" - source = """ - import pytask - def gen(): - yield 1 - yield 2 - yield 3 - @pytask.mark.parametrize('a', gen()) - def task_func(a): - pass - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - session = main({"paths": tmp_path}) - assert session.exit_code == ExitCode.OK - assert len(session.execution_reports) == 3 - - -@pytest.mark.end_to_end() -def test_raise_error_if_function_does_not_use_parametrized_arguments(tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i', range(2)) - def task_func(): - pass - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.FAILED - assert isinstance(session.execution_reports[0].exc_info[1], TypeError) - assert isinstance(session.execution_reports[1].exc_info[1], TypeError) - - -@pytest.mark.end_to_end() -@pytest.mark.parametrize( - ("arg_values", "ids"), - [ - (range(2), ["first_trial", "second_trial"]), - ([True, False], ["first_trial", "second_trial"]), - ], -) -def test_parametrize_w_ids(tmp_path, arg_values, ids): - tmp_path.joinpath("task_module.py").write_text( - textwrap.dedent( - f""" - import pytask - - @pytask.mark.parametrize('i', {arg_values}, ids={ids}) - def task_func(i): - pass - """ - ) - ) - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.OK - for task, id_ in zip(session.tasks, ids): - assert id_ in task.name - - -@pytest.mark.end_to_end() -def test_two_parametrize_w_ids(runner, tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i', range(2), ids=["2.1", "2.2"]) - @pytask.mark.parametrize('j', range(2), ids=["1.1", "1.2"]) - def task_func(i, j): - pass - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - result = runner.invoke(cli, [tmp_path.as_posix()]) - - assert result.exit_code == ExitCode.COLLECTION_FAILED - assert "You cannot apply @pytask.mark.parametrize multiple" in result.output - - -@pytest.mark.end_to_end() -@pytest.mark.parametrize("ids", [["a"], list("abc"), ((1,), (2,)), ({0}, {1})]) -def test_raise_error_for_irregular_ids(tmp_path, ids): - tmp_path.joinpath("task_module.py").write_text( - textwrap.dedent( - f""" - import pytask - - @pytask.mark.parametrize('i', range(2), ids={ids}) - def task_func(): - pass - """ - ) - ) - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.COLLECTION_FAILED - assert isinstance(session.collection_reports[0].exc_info[1], ValueError) - - -@pytest.mark.end_to_end() -def test_raise_error_if_parametrization_produces_non_unique_tasks(tmp_path): - tmp_path.joinpath("task_module.py").write_text( - textwrap.dedent( - """ - import pytask - - @pytask.mark.parametrize('i', [0, 0]) - def task_func(i): - pass - """ - ) - ) - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.COLLECTION_FAILED - assert isinstance(session.collection_reports[0].exc_info[1], ValueError) - - -@pytest.mark.end_to_end() -@pytest.mark.parametrize( - ("arg_names", "arg_values", "content"), - [ - ( - ("i", "j"), - [1, 2, 3], - [ - "ValueError", - "with 2 'arg_names', ('i', 'j'),", - "'arg_values' is 1.", - "parametrization no. 0:", - "(1,)", - ], - ), - ( - ("i", "j"), - [(1, 2, 3)], - [ - "ValueError", - "with 2 'arg_names', ('i', 'j'),", - "'arg_values' is 3.", - "parametrization no. 0:", - "(1, 2, 3)", - ], - ), - ( - ("i", "j"), - [(1, 2), (1, 2, 3)], - [ - "ValueError", - "with 2 'arg_names', ('i', 'j'),", - "'arg_values' is 2 or 3.", - "parametrization no. 1:", - "(1, 2, 3)", - ], - ), - ], -) -def test_wrong_number_of_names_and_wrong_number_of_arguments( - tmp_path, runner, arg_names, arg_values, content -): - source = f""" - import pytask - - @pytask.mark.parametrize({arg_names}, {arg_values}) - def task_func(): - pass - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - result = runner.invoke(cli, [tmp_path.as_posix()]) - - assert result.exit_code == ExitCode.COLLECTION_FAILED - for c in content: - assert c in result.output - - -@pytest.mark.end_to_end() -def test_generators_are_removed_from_depends_on_produces(tmp_path): - source = """ - from pathlib import Path - import pytask - - @pytask.mark.parametrize("produces", [ - ((x for x in ["out.txt", "out_2.txt"]),), - ["in.txt"], - ]) - def task_example(produces): - produces = {0: produces} if isinstance(produces, Path) else produces - for p in produces.values(): - p.write_text("hihi") - """ - tmp_path.joinpath("task_dummy.py").write_text(textwrap.dedent(source)) - - session = main({"paths": tmp_path}) - assert session.exit_code == ExitCode.OK - assert session.tasks[0].function.pytask_meta.markers == [] - - -@pytest.mark.end_to_end() -def test_parametrizing_tasks_with_namedtuples(runner, tmp_path): - source = """ - from typing import NamedTuple - import pytask - from pathlib import Path - - - class Task(NamedTuple): - i: int - produces: Path - - - @pytask.mark.parametrize('i, produces', [ - Task(i=1, produces="1.txt"), Task(produces="2.txt", i=2), - ]) - def task_write_numbers_to_file(produces, i): - produces.write_text(str(i)) - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - result = runner.invoke(cli, [tmp_path.as_posix()]) - - assert result.exit_code == ExitCode.OK - for i in range(1, 3): - assert tmp_path.joinpath(f"{i}.txt").read_text() == str(i) - - -@pytest.mark.end_to_end() -def test_parametrization_with_different_n_of_arg_names_and_arg_values(runner, tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i, produces', [(1, "1.txt"), (2, 3, "2.txt")]) - def task_write_numbers_to_file(produces, i): - produces.write_text(str(i)) - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - result = runner.invoke(cli, [tmp_path.as_posix()]) - - assert result.exit_code == ExitCode.COLLECTION_FAILED - assert "Task 'task_write_numbers_to_file' is parametrized with 2" in result.output - - -@pytest.mark.unit() -@pytest.mark.parametrize( - ("arg_names", "arg_values", "name", "expectation"), - [ - pytest.param( - ("a",), - [(1,), (2,)], - "task_name", - does_not_raise(), - id="normal one argument parametrization", - ), - pytest.param( - ("a", "b"), - [(1, 2), (3, 4)], - "task_name", - does_not_raise(), - id="normal two argument argument parametrization", - ), - pytest.param( - ("a",), - [(1, 2), (2,)], - "task_name", - pytest.raises(ValueError, match="Task 'task_name' is parametrized with 1"), - id="error with one argument parametrization", - ), - pytest.param( - ("a", "b"), - [(1, 2), (3, 4, 5)], - "task_name", - pytest.raises(ValueError, match="Task 'task_name' is parametrized with 2"), - id="error with two argument argument parametrization", - ), - ], -) -def test_check_if_n_arg_names_matches_n_arg_values( - arg_names, arg_values, name, expectation -): - with expectation: - _check_if_n_arg_names_matches_n_arg_values(arg_names, arg_values, name) - - -@pytest.mark.end_to_end() -def test_parametrize_with_single_dict(tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i', [{"a": 1}, {"a": 1.0}]) - def task_write_numbers_to_file(i): - assert i["a"] == 1 - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.OK - - -@pytest.mark.end_to_end() -def test_deprecation_warning_for_parametrizing_tasks(runner, tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i, produces', [(1, "1.txt"), (2, "2.txt")]) - def task_write_numbers_to_file(produces, i): - produces.write_text(str(i)) - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - - result = runner.invoke(cli, [tmp_path.as_posix()]) - - assert result.exit_code == ExitCode.OK - assert "FutureWarning" in result.output - - -@pytest.mark.end_to_end() -def test_silence_deprecation_warning_for_parametrizing_tasks(runner, tmp_path): - source = """ - import pytask - - @pytask.mark.parametrize('i, produces', [(1, "1.txt"), (2, "2.txt")]) - def task_write_numbers_to_file(produces, i): - produces.write_text(str(i)) - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - tmp_path.joinpath("pyproject.toml").write_text( - "[tool.pytask.ini_options]\nsilence_parametrize_deprecation = true" - ) - - result = runner.invoke(cli, [tmp_path.as_posix()]) - - assert result.exit_code == ExitCode.OK - assert "FutureWarning" not in result.output diff --git a/tests/test_task.py b/tests/test_task.py index 33905c26a..3313fdb7b 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -33,35 +33,6 @@ def {func_name}(produces): assert session.tasks[0].name.endswith(f"task_module.py::{func_name}") -@pytest.mark.end_to_end() -@pytest.mark.parametrize("func_name", ["task_example", "func"]) -@pytest.mark.parametrize("task_name", ["the_only_task", None]) -def test_task_with_task_decorator_with_parametrize(tmp_path, func_name, task_name): - task_decorator_input = f"{task_name!r}" if task_name else task_name - source = f""" - import pytask - - @pytask.mark.task({task_decorator_input}) - @pytask.mark.parametrize("produces", ["out_1.txt", "out_2.txt"]) - def {func_name}(produces): - produces.write_text("Hello. It's me.") - """ - path_to_module = tmp_path.joinpath("task_module.py") - path_to_module.write_text(textwrap.dedent(source)) - - session = main({"paths": tmp_path}) - - assert session.exit_code == ExitCode.OK - - file_name = path_to_module.name - if task_name: - assert session.tasks[0].name.endswith(f"{file_name}::{task_name}[out_1.txt]") - assert session.tasks[1].name.endswith(f"{file_name}::{task_name}[out_2.txt]") - else: - assert session.tasks[0].name.endswith(f"{file_name}::{func_name}[out_1.txt]") - assert session.tasks[1].name.endswith(f"{file_name}::{func_name}[out_2.txt]") - - @pytest.mark.end_to_end() def test_parametrization_in_for_loop(tmp_path, runner): source = """ @@ -178,7 +149,7 @@ def test_parametrization_in_for_loop_with_ids(tmp_path, runner): for i in range(2): @pytask.mark.task( - "deco_task", id=i, kwargs={"i": i, "produces": f"out_{i}.txt"} + "deco_task", id=str(i), kwargs={"i": i, "produces": f"out_{i}.txt"} ) def example(produces, i): produces.write_text(str(i)) @@ -429,3 +400,41 @@ def task_example(): assert "task_example[0]" in result.output assert "task_example[1]" in result.output assert "Collected 2 tasks" in result.output + + +@pytest.mark.end_to_end() +@pytest.mark.parametrize( + "irregular_id", [1, (1,), [1], {1}, ["a"], list("abc"), ((1,), (2,)), ({0}, {1})] +) +def test_raise_errors_for_irregular_ids(runner, tmp_path, irregular_id): + source = f""" + import pytask + + @pytask.mark.task(id={irregular_id}) + def task_example(): + pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + result = runner.invoke(cli, [tmp_path.as_posix()]) + + assert result.exit_code == ExitCode.COLLECTION_FAILED + assert "Argument 'id' of @pytask.mark.task" in result.output + + +@pytest.mark.end_to_end() +@pytest.mark.xfail(reason="Should fail. Mandatory products will fix the issue.") +def test_raise_error_if_parametrization_produces_non_unique_tasks(tmp_path): + source = """ + import pytask + + for i in [0, 0]: + @pytask.mark.task(id=str(i)) + def task_func(i=i): + pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + session = main({"paths": tmp_path}) + + assert session.exit_code == ExitCode.COLLECTION_FAILED + assert isinstance(session.collection_reports[0].exc_info[1], ValueError) diff --git a/tests/test_parametrize_utils.py b/tests/test_task_utils.py similarity index 84% rename from tests/test_parametrize_utils.py rename to tests/test_task_utils.py index 8b4832081..779cccd13 100644 --- a/tests/test_parametrize_utils.py +++ b/tests/test_task_utils.py @@ -1,7 +1,7 @@ from __future__ import annotations import pytest -from _pytask.parametrize_utils import arg_value_to_id_component +from _pytask.task_utils import _arg_value_to_id_component @pytest.mark.unit() @@ -22,5 +22,5 @@ ], ) def test_arg_value_to_id_component(arg_name, arg_value, i, id_func, expected): - result = arg_value_to_id_component(arg_name, arg_value, i, id_func) + result = _arg_value_to_id_component(arg_name, arg_value, i, id_func) assert result == expected From ae31b651da48515ce2e89f412f28009fa11f9896 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Wed, 5 Jul 2023 20:19:21 +0200 Subject: [PATCH 05/12] Parse dependencies from all args if `depends_on` is not used. (#384) --- docs/source/changes.md | 2 + .../defining_dependencies_products.md | 26 +-- src/_pytask/collect.py | 25 ++- src/_pytask/collect_command.py | 25 ++- src/_pytask/collect_utils.py | 153 ++++++++++++++++-- src/_pytask/dag.py | 5 +- src/_pytask/execute.py | 13 +- src/_pytask/nodes.py | 20 ++- src/_pytask/task_utils.py | 7 +- tests/test_clean.py | 66 +++++--- tests/test_collect.py | 149 +++++++++++------ tests/test_collect_command.py | 60 ++++++- tests/test_database.py | 2 +- tests/test_dry_run.py | 4 +- tests/test_graph.py | 2 +- tests/test_pybaum.py | 2 + tests/test_skipping.py | 5 +- tests/test_task.py | 12 ++ 18 files changed, 437 insertions(+), 141 deletions(-) diff --git a/docs/source/changes.md b/docs/source/changes.md index d7fa682d7..19698bc51 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -8,6 +8,8 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and ## 0.4.0 - 2023-xx-xx - {pull}`323` remove Python 3.7 support and use a new Github action to provide mamba. +- {pull}`384` allows to parse dependencies from every function argument if `depends_on` + is not present. - {pull}`387` replaces pony with sqlalchemy. - {pull}`391` removes `@pytask.mark.parametrize`. diff --git a/docs/source/tutorials/defining_dependencies_products.md b/docs/source/tutorials/defining_dependencies_products.md index 98086b5ce..d245b2615 100644 --- a/docs/source/tutorials/defining_dependencies_products.md +++ b/docs/source/tutorials/defining_dependencies_products.md @@ -4,8 +4,8 @@ To ensure pytask executes all tasks in the correct order, define which dependenc required and which products are produced by a task. :::{important} -If you do not specify dependencies and products as explained below, pytask will not be able -to build a graph, a {term}`DAG`, and will not be able to execute all tasks in the +If you do not specify dependencies and products as explained below, pytask will not be +able to build a graph, a {term}`DAG`, and will not be able to execute all tasks in the project correctly! ::: @@ -19,15 +19,16 @@ def task_create_random_data(produces): ... ``` -The {func}`@pytask.mark.produces ` marker attaches a -product to a task which is a {class}`pathlib.Path` to file. After the task has finished, -pytask will check whether the file exists. +The {func}`@pytask.mark.produces ` marker attaches a product to a +task which is a {class}`pathlib.Path` to file. After the task has finished, pytask will +check whether the file exists. -Optionally, you can use `produces` as an argument of the task function and get access to -the same path inside the task function. +Add `produces` as an argument of the task function to get access to the same path inside +the task function. :::{tip} -If you do not know about {mod}`pathlib` check out [^id3] and [^id4]. The module is beneficial for handling paths conveniently and across platforms. +If you do not know about {mod}`pathlib` check out [^id3] and [^id4]. The module is +beneficial for handling paths conveniently and across platforms. ::: ## Dependencies @@ -44,7 +45,7 @@ def task_plot_data(depends_on, produces): ... ``` -Use `depends_on` as a function argument to work with the dependency path and, for +Add `depends_on` as a function argument to work with the path of the dependency and, for example, load the data. ## Conversion @@ -61,9 +62,6 @@ def task_create_random_data(produces): ... ``` -If you use `depends_on` or `produces` as arguments for the task function, you will have -access to the paths of the targets as {class}`pathlib.Path`. - ## Multiple dependencies and products The easiest way to attach multiple dependencies or products to a task is to pass a @@ -108,7 +106,9 @@ Why does pytask recommend dictionaries and convert lists, tuples, or other iterators to dictionaries? First, dictionaries with positions as keys behave very similarly to lists. -Secondly, dictionaries use keys instead of positions that are more verbose and descriptive and do not assume a fixed ordering. Both attributes are especially desirable in complex projects. +Secondly, dictionaries use keys instead of positions that are more verbose and +descriptive and do not assume a fixed ordering. Both attributes are especially desirable +in complex projects. ## Multiple decorators diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index dcb22df3e..8403a9e8d 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -12,7 +12,9 @@ from typing import Iterable from _pytask.collect_utils import depends_on +from _pytask.collect_utils import parse_dependencies_from_task_function from _pytask.collect_utils import parse_nodes +from _pytask.collect_utils import parse_products_from_task_function from _pytask.collect_utils import produces from _pytask.config import hookimpl from _pytask.config import IS_FILE_SYSTEM_CASE_SENSITIVE @@ -22,6 +24,7 @@ from _pytask.exceptions import CollectionError from _pytask.mark_utils import has_mark from _pytask.nodes import FilePathNode +from _pytask.nodes import PythonNode from _pytask.nodes import Task from _pytask.outcomes import CollectionOutcome from _pytask.outcomes import count_outcomes @@ -167,11 +170,20 @@ def pytask_collect_task( """ if (name.startswith("task_") or has_mark(obj, "task")) and callable(obj): - dependencies = parse_nodes(session, path, name, obj, depends_on) - products = parse_nodes(session, path, name, obj, produces) + if has_mark(obj, "depends_on"): + nodes = parse_nodes(session, path, name, obj, depends_on) + dependencies = {"depends_on": nodes} + else: + dependencies = parse_dependencies_from_task_function( + session, path, name, obj + ) + + if has_mark(obj, "produces"): + products = parse_nodes(session, path, name, obj, produces) + else: + products = parse_products_from_task_function(session, path, name, obj) markers = obj.pytask_meta.markers if hasattr(obj, "pytask_meta") else [] - kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} # Get the underlying function to avoid having different states of the function, # e.g. due to pytask_meta, in different layers of the wrapping. @@ -184,7 +196,6 @@ def pytask_collect_task( depends_on=dependencies, produces=products, markers=markers, - kwargs=kwargs, ) return None @@ -205,7 +216,7 @@ def pytask_collect_task( @hookimpl(trylast=True) def pytask_collect_node( session: Session, path: Path, node: str | Path -) -> FilePathNode | None: +) -> FilePathNode | PythonNode: """Collect a node of a task as a :class:`pytask.nodes.FilePathNode`. Strings are assumed to be paths. This might be a strict assumption, but since this @@ -226,8 +237,6 @@ def pytask_collect_node( handled by this function. """ - if isinstance(node, str): - node = Path(node) if isinstance(node, Path): if not node.is_absolute(): node = path.parent.joinpath(node) @@ -246,7 +255,7 @@ def pytask_collect_node( raise ValueError(_TEMPLATE_ERROR.format(node, case_sensitive_path)) return FilePathNode.from_path(node) - return None + return PythonNode(value=node) def _not_ignored_paths( diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index 9bad3d365..e297c823a 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -20,6 +20,7 @@ from _pytask.exceptions import ResolvingDependenciesError from _pytask.mark import select_by_keyword from _pytask.mark import select_by_mark +from _pytask.nodes import FilePathNode from _pytask.outcomes import ExitCode from _pytask.path import find_common_ancestor from _pytask.path import relative_to @@ -123,7 +124,11 @@ def _find_common_ancestor_of_all_nodes( for task in tasks: all_paths.append(task.path) if show_nodes: - all_paths.extend(x.path for x in tree_just_flatten(task.depends_on)) + all_paths.extend( + x.path + for x in tree_just_flatten(task.depends_on) + if isinstance(x, FilePathNode) + ) all_paths.extend(x.path for x in tree_just_flatten(task.produces)) common_ancestor = find_common_ancestor(*all_paths, *paths) @@ -160,14 +165,14 @@ def _print_collected_tasks( Parameters ---------- - dictionary : Dict[Path, List["Task"]] + dictionary A dictionary with path on the first level, tasks on the second, dependencies and products on the third. - show_nodes : bool + show_nodes Indicator for whether dependencies and products should be displayed. - editor_url_scheme : str + editor_url_scheme The scheme to create an url. - common_ancestor : Path + common_ancestor The path common to all tasks and nodes. """ @@ -197,9 +202,13 @@ def _print_collected_tasks( ) if show_nodes: - for node in sorted( - tree_just_flatten(task.depends_on), key=lambda x: x.path - ): + file_path_nodes = [ + i + for i in tree_just_flatten(task.depends_on) + if isinstance(i, FilePathNode) + ] + sorted_nodes = sorted(file_path_nodes, key=lambda x: x.path) + for node in sorted_nodes: reduced_node_name = relative_to(node.path, common_ancestor) url_style = create_url_style_for_path(node.path, editor_url_scheme) task_branch.add( diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index ca15e624a..5eb6cff9a 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -1,6 +1,7 @@ """This module provides utility functions for :mod:`_pytask.collect`.""" from __future__ import annotations +import inspect import itertools import uuid from pathlib import Path @@ -12,7 +13,9 @@ from _pytask.exceptions import NodeNotCollectedError from _pytask.mark_utils import remove_marks +from _pytask.nodes import PythonNode from _pytask.shared import find_duplicates +from _pytask.task_utils import parse_keyword_arguments_from_signature_defaults from attrs import define from attrs import field from pybaum.tree_util import tree_map @@ -23,7 +26,12 @@ from _pytask.nodes import MetaNode -__all__ = ["depends_on", "parse_nodes", "produces"] +__all__ = [ + "depends_on", + "parse_dependencies_from_task_function", + "parse_nodes", + "produces", +] def depends_on( @@ -64,7 +72,7 @@ def parse_nodes( """Parse nodes from object.""" objects = _extract_nodes_from_function_markers(obj, parser) nodes = _convert_objects_to_node_dictionary(objects, parser.__name__) - nodes = tree_map(lambda x: _collect_node(session, path, name, x), nodes) + nodes = tree_map(lambda x: _collect_old_dependencies(session, path, name, x), nodes) return nodes @@ -194,25 +202,89 @@ def _merge_dictionaries(list_of_dicts: list[dict[Any, Any]]) -> dict[Any, Any]: return out -def _collect_node( +def parse_dependencies_from_task_function( + session: Session, path: Path, name: str, obj: Any +) -> dict[str, Any]: + """Parse dependencies from task function.""" + task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} + signature_defaults = parse_keyword_arguments_from_signature_defaults(obj) + kwargs = {**signature_defaults, **task_kwargs} + kwargs.pop("produces", None) + + dependencies = {} + for name, value in kwargs.items(): + parsed_value = tree_map( + lambda x: _collect_dependencies(session, path, name, x), value # noqa: B023 + ) + dependencies[name] = ( + PythonNode(value=None) if parsed_value is None else parsed_value + ) + + return dependencies + + +def parse_products_from_task_function( + session: Session, path: Path, name: str, obj: Any +) -> dict[str, Any]: + """Parse dependencies from task function.""" + task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} + if "produces" in task_kwargs: + return tree_map( + lambda x: _collect_product(session, path, name, x, is_string_allowed=True), + task_kwargs["produces"], + ) + + parameters = inspect.signature(obj).parameters + if "produces" in parameters: + parameter = parameters["produces"] + if parameter.default is not parameter.empty: + # Use _collect_new_node to not collect strings. + return tree_map( + lambda x: _collect_product( + session, path, name, x, is_string_allowed=False + ), + parameter.default, + ) + return {} + + +def _collect_old_dependencies( session: Session, path: Path, name: str, node: str | Path ) -> dict[str, MetaNode]: """Collect nodes for a task. - Parameters - ---------- - session - The session. - path - The path to the task whose nodes are collected. - name - The name of the task. - nodes - A dictionary of nodes parsed from the ``depends_on`` or ``produces`` markers. - - Returns - ------- - A dictionary of node names and their paths. + Raises + ------ + NodeNotCollectedError + If the node could not collected. + + """ + if not isinstance(node, (str, Path)): + raise ValueError( + "'@pytask.mark.depends_on' and '@pytask.mark.produces' can only accept " + "values of type 'str' and 'pathlib.Path' or the same values nested in " + f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}." + ) + + if isinstance(node, str): + node = Path(node) + + collected_node = session.hook.pytask_collect_node( + session=session, path=path, node=node + ) + if collected_node is None: + raise NodeNotCollectedError( + f"{node!r} cannot be parsed as a dependency or product for task " + f"{name!r} in {path!r}." + ) + + return collected_node + + +def _collect_dependencies( + session: Session, path: Path, name: str, node: Any +) -> dict[str, MetaNode]: + """Collect nodes for a task. Raises ------ @@ -220,6 +292,53 @@ def _collect_node( If the node could not collected. """ + collected_node = session.hook.pytask_collect_node( + session=session, path=path, node=node + ) + if collected_node is None: + raise NodeNotCollectedError( + f"{node!r} cannot be parsed as a dependency for task " + f"{name!r} in {path!r}." + ) + return collected_node + + +def _collect_product( + session: Session, + path: Path, + name: str, + node: str | Path, + is_string_allowed: bool = False, +) -> dict[str, MetaNode]: + """Collect products for a task. + + Defining products with strings is only allowed when using the decorator. Parameter + defaults can only be :class:`pathlib.Path`s. + + Raises + ------ + NodeNotCollectedError + If the node could not collected. + + """ + # For historical reasons, task.kwargs is like the deco and supports str and Path. + if not isinstance(node, (str, Path)) and is_string_allowed: + raise ValueError( + "`@pytask.mark.task(kwargs={'produces': ...}` can only accept values of " + "type 'str' and 'pathlib.Path' or the same values nested in " + f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}." + ) + # The parameter defaults only support Path objects. + if not isinstance(node, Path) and not is_string_allowed: + raise ValueError( + "If you use 'produces' as an argument of a task, it can only accept values " + "of type 'pathlib.Path' or the same value nested in " + f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}." + ) + + if isinstance(node, str): + node = Path(node) + collected_node = session.hook.pytask_collect_node( session=session, path=path, node=node ) diff --git a/src/_pytask/dag.py b/src/_pytask/dag.py index c6acf0346..bbf199a5d 100644 --- a/src/_pytask/dag.py +++ b/src/_pytask/dag.py @@ -190,7 +190,10 @@ def _format_cycles(cycles: list[tuple[str, ...]]) -> str: "tree which shows which dependencies are missing for which tasks.\n\n{}" ) if IS_FILE_SYSTEM_CASE_SENSITIVE: - _TEMPLATE_ERROR += "\n\n(Hint: Sometimes case sensitivity is at fault.)" + _TEMPLATE_ERROR += ( + "\n\n(Hint: Your file-system is case-sensitive. Check the paths' " + "capitalization carefully.)" + ) def _check_if_root_nodes_are_available(dag: nx.DiGraph) -> None: diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index 1922e27aa..40e8e4b42 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -143,13 +143,14 @@ def pytask_execute_task(session: Session, task: Task) -> bool: if session.config["dry_run"]: raise WouldBeExecuted - kwargs = {**task.kwargs} + parameters = inspect.signature(task.function).parameters - func_arg_names = set(inspect.signature(task.function).parameters) - for arg_name in ("depends_on", "produces"): - if arg_name in func_arg_names: - attribute = getattr(task, arg_name) - kwargs[arg_name] = tree_map(lambda x: x.value, attribute) + kwargs = {} + for name, value in task.depends_on.items(): + kwargs[name] = tree_map(lambda x: x.value, value) + + if task.produces and "produces" in parameters: + kwargs["produces"] = tree_map(lambda x: x.value, task.produces) task.execute(**kwargs) return True diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index 2d330f02d..47f7b9f19 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -51,8 +51,6 @@ class Task(MetaNode): """A list of products of task.""" markers: list[Mark] = field(factory=list) """A list of markers attached to the task function.""" - kwargs: dict[str, Any] = field(factory=dict) - """A dictionary with keyword arguments supplied to the task.""" _report_sections: list[tuple[str, str, str]] = field(factory=list) """Reports with entries for when, what, and content.""" attributes: dict[Any, Any] = field(factory=dict) @@ -108,3 +106,21 @@ def from_path(cls, path: Path) -> FilePathNode: if not path.is_absolute(): raise ValueError("FilePathNode must be instantiated from absolute path.") return cls(name=path.as_posix(), value=path, path=path) + + +@define(kw_only=True) +class PythonNode(MetaNode): + """The class for a node which is a Python object.""" + + value: Any + hash: bool = False # noqa: A003 + name: str = "" + + def __attrs_post_init__(self) -> None: + if not self.name: + self.name = str(self.value) + + def state(self) -> str | None: + if self.hash: + return str(hash(self.value)) + return str(0) diff --git a/src/_pytask/task_utils.py b/src/_pytask/task_utils.py index ad541c9d1..750605a51 100644 --- a/src/_pytask/task_utils.py +++ b/src/_pytask/task_utils.py @@ -12,6 +12,9 @@ from _pytask.shared import find_duplicates +__all__ = ["parse_keyword_arguments_from_signature_defaults"] + + COLLECTED_TASKS: dict[Path, list[Callable[..., Any]]] = defaultdict(list) """A container for collecting tasks. @@ -149,7 +152,7 @@ def _parse_task(task: Callable[..., Any]) -> tuple[str, Callable[..., Any]]: parsed_name = task.__name__ if name is None else name - signature_kwargs = _parse_keyword_arguments_from_signature_defaults(task) + signature_kwargs = parse_keyword_arguments_from_signature_defaults(task) task.pytask_meta.kwargs = { # type: ignore[attr-defined] **task.pytask_meta.kwargs, # type: ignore[attr-defined] **signature_kwargs, @@ -158,7 +161,7 @@ def _parse_task(task: Callable[..., Any]) -> tuple[str, Callable[..., Any]]: return parsed_name, task -def _parse_keyword_arguments_from_signature_defaults( +def parse_keyword_arguments_from_signature_defaults( task: Callable[..., Any] ) -> dict[str, Any]: """Parse keyword arguments from signature defaults.""" diff --git a/tests/test_clean.py b/tests/test_clean.py index a74752740..62a7e6220 100644 --- a/tests/test_clean.py +++ b/tests/test_clean.py @@ -11,18 +11,29 @@ from pytask import ExitCode -@pytest.fixture() -def project(tmp_path): - """Create a sample project to be cleaned.""" - source = """ - import pytask +_PROJECT_TASK = """ +import pytask - @pytask.mark.depends_on("in.txt") - @pytask.mark.produces("out.txt") - def task_write_text(produces): - produces.write_text("a") - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) +@pytask.mark.depends_on("in.txt") +@pytask.mark.produces("out.txt") +def task_write_text(depends_on, produces): + produces.write_text("a") +""" + + +_PROJECT_TASK_NEW_INTERFACE = """ +import pytask +from pathlib import Path + +def task_write_text(in_path=Path("in.txt"), produces=Path("out.txt")): + produces.write_text("a") +""" + + +@pytest.fixture(params=[_PROJECT_TASK, _PROJECT_TASK_NEW_INTERFACE]) +def project(request, tmp_path): + """Create a sample project to be cleaned.""" + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(request.param)) tmp_path.joinpath("in.txt").touch() tmp_path.joinpath("to_be_deleted_file_1.txt").touch() @@ -32,18 +43,29 @@ def task_write_text(produces): return tmp_path -@pytest.fixture() -def git_project(tmp_path): - """Create a sample project to be cleaned.""" - source = """ - import pytask +_GIT_PROJECT_TASK = """ +import pytask - @pytask.mark.depends_on("in_tracked.txt") - @pytask.mark.produces("out.txt") - def task_write_text(produces): - produces.write_text("a") - """ - tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) +@pytask.mark.depends_on("in_tracked.txt") +@pytask.mark.produces("out.txt") +def task_write_text(depends_on, produces): + produces.write_text("a") +""" + + +_GIT_PROJECT_TASK_NEW_INTERFACE = """ +import pytask +from pathlib import Path + +def task_write_text(in_path=Path("in_tracked.txt"), produces=Path("out.txt")): + produces.write_text("a") +""" + + +@pytest.fixture(params=[_GIT_PROJECT_TASK, _GIT_PROJECT_TASK_NEW_INTERFACE]) +def git_project(request, tmp_path): + """Create a sample project to be cleaned.""" + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(request.param)) tmp_path.joinpath("in_tracked.txt").touch() tmp_path.joinpath("tracked.txt").touch() diff --git a/tests/test_collect.py b/tests/test_collect.py index 9a10831a5..95c2ef226 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -3,13 +3,11 @@ import sys import textwrap import warnings -from contextlib import ExitStack as does_not_raise # noqa: N813 from pathlib import Path import pytest from _pytask.collect import _find_shortest_uniquely_identifiable_name_for_tasks from _pytask.collect import pytask_collect_node -from _pytask.exceptions import NodeNotCollectedError from pytask import cli from pytask import CollectionOutcome from pytask import ExitCode @@ -38,52 +36,33 @@ def task_write_text(depends_on, produces): @pytest.mark.end_to_end() -def test_collect_tasks_from_modules_with_the_same_name(tmp_path): - """We need to check that task modules can have the same name. See #373 and #374.""" - tmp_path.joinpath("a").mkdir() - tmp_path.joinpath("b").mkdir() - tmp_path.joinpath("a", "task_module.py").write_text("def task_a(): pass") - tmp_path.joinpath("b", "task_module.py").write_text("def task_a(): pass") - session = main({"paths": tmp_path}) - assert len(session.collection_reports) == 2 - assert all( - report.outcome == CollectionOutcome.SUCCESS - for report in session.collection_reports - ) - assert { - report.node.function.__module__ for report in session.collection_reports - } == {"a.task_module", "b.task_module"} - - -@pytest.mark.end_to_end() -def test_collect_module_name(tmp_path): - """We need to add a task module to the sys.modules. See #373 and #374.""" +def test_collect_depends_on_that_is_not_str_or_path(tmp_path): + """If a node cannot be parsed because unknown type, raise an error.""" source = """ - # without this import, everything works fine - from __future__ import annotations - - import dataclasses - - @dataclasses.dataclass - class Data: - x: int + import pytask - def task_my_task(): + @pytask.mark.depends_on(True) + def task_with_non_path_dependency(): pass """ tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + session = main({"paths": tmp_path}) - outcome = session.collection_reports[0].outcome - assert outcome == CollectionOutcome.SUCCESS + + assert session.exit_code == ExitCode.COLLECTION_FAILED + assert session.collection_reports[0].outcome == CollectionOutcome.FAIL + exc_info = session.collection_reports[0].exc_info + assert isinstance(exc_info[1], ValueError) + assert "'@pytask.mark.depends_on'" in str(exc_info[1]) @pytest.mark.end_to_end() -def test_collect_filepathnode_with_unknown_type(tmp_path): +def test_collect_produces_that_is_not_str_or_path(tmp_path): """If a node cannot be parsed because unknown type, raise an error.""" source = """ import pytask - @pytask.mark.depends_on(True) + @pytask.mark.produces(True) def task_with_non_path_dependency(): pass """ @@ -94,7 +73,8 @@ def task_with_non_path_dependency(): assert session.exit_code == ExitCode.COLLECTION_FAILED assert session.collection_reports[0].outcome == CollectionOutcome.FAIL exc_info = session.collection_reports[0].exc_info - assert isinstance(exc_info[1], NodeNotCollectedError) + assert isinstance(exc_info[1], ValueError) + assert "'@pytask.mark.depends_on'" in str(exc_info[1]) @pytest.mark.end_to_end() @@ -129,7 +109,7 @@ def task_1(depends_on, produces): @pytest.mark.end_to_end() @pytest.mark.parametrize("path_extension", ["", "task_module.py"]) -def test_collect_same_test_different_ways(tmp_path, path_extension): +def test_collect_same_task_different_ways(tmp_path, path_extension): tmp_path.joinpath("task_module.py").write_text("def task_passes(): pass") session = main({"paths": tmp_path.joinpath(path_extension)}) @@ -167,13 +147,12 @@ def test_collect_files_w_custom_file_name_pattern( @pytest.mark.unit() @pytest.mark.parametrize( - ("session", "path", "node", "expectation", "expected"), + ("session", "path", "node", "expected"), [ pytest.param( Session({"check_casing_of_paths": False}, None), Path(), Path.cwd() / "text.txt", - does_not_raise(), Path.cwd() / "text.txt", id="test with absolute string path", ), @@ -181,19 +160,17 @@ def test_collect_files_w_custom_file_name_pattern( Session({"check_casing_of_paths": False}, None), Path(), 1, - does_not_raise(), - None, - id="test cannot collect node", + "1", + id="test with python node", ), ], ) -def test_pytask_collect_node(session, path, node, expectation, expected): - with expectation: - result = pytask_collect_node(session, path, node) - if result is None: - assert result is expected - else: - assert str(result.path) == str(expected) +def test_pytask_collect_node(session, path, node, expected): + result = pytask_collect_node(session, path, node) + if result is None: + assert result is expected + else: + assert str(result.value) == str(expected) @pytest.mark.unit() @@ -220,7 +197,7 @@ def test_pytask_collect_node_does_not_raise_error_if_path_is_not_normalized( task_path = tmp_path / "task_example.py" real_node = tmp_path / "text.txt" - collected_node = f"../{tmp_path.name}/text.txt" + collected_node = Path("..", tmp_path.name, "text.txt") if is_absolute: collected_node = tmp_path / collected_node @@ -270,3 +247,75 @@ def test_find_shortest_uniquely_identifiable_names_for_tasks(tmp_path): result = _find_shortest_uniquely_identifiable_name_for_tasks(tasks) assert result == expected + + +def test_collect_dependencies_from_args_if_depends_on_is_missing(tmp_path): + source = """ + from pathlib import Path + + def task_example(path_in = Path("in.txt"), produces = Path("out.txt")): + produces.write_text(path_in.read_text()) + """ + tmp_path.joinpath("task_example.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.txt").write_text("hello") + + session = main({"paths": tmp_path}) + + assert session.exit_code == ExitCode.OK + assert len(session.tasks) == 1 + assert session.tasks[0].depends_on["path_in"].path == tmp_path.joinpath("in.txt") + + +@pytest.mark.end_to_end() +def test_collect_tasks_from_modules_with_the_same_name(tmp_path): + """We need to check that task modules can have the same name. See #373 and #374.""" + tmp_path.joinpath("a").mkdir() + tmp_path.joinpath("b").mkdir() + tmp_path.joinpath("a", "task_module.py").write_text("def task_a(): pass") + tmp_path.joinpath("b", "task_module.py").write_text("def task_a(): pass") + session = main({"paths": tmp_path}) + assert len(session.collection_reports) == 2 + assert all( + report.outcome == CollectionOutcome.SUCCESS + for report in session.collection_reports + ) + assert { + report.node.function.__module__ for report in session.collection_reports + } == {"a.task_module", "b.task_module"} + + +@pytest.mark.end_to_end() +def test_collect_module_name(tmp_path): + """We need to add a task module to the sys.modules. See #373 and #374.""" + source = """ + # without this import, everything works fine + from __future__ import annotations + + import dataclasses + + @dataclasses.dataclass + class Data: + x: int + + def task_my_task(): + pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + session = main({"paths": tmp_path}) + outcome = session.collection_reports[0].outcome + assert outcome == CollectionOutcome.SUCCESS + + +@pytest.mark.end_to_end() +def test_collect_string_product_as_function_default_fails(tmp_path): + source = """ + import pytask + + def task_write_text(produces="out.txt"): + produces.touch() + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + session = main({"paths": tmp_path}) + report = session.collection_reports[0] + assert report.outcome == CollectionOutcome.FAIL + assert "If you use 'produces'" in str(report.exc_info[1]) diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index eba87f4cc..d7f3149e7 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -7,6 +7,7 @@ import pytest from _pytask.collect_command import _find_common_ancestor_of_all_nodes from _pytask.collect_command import _print_collected_tasks +from _pytask.nodes import FilePathNode from attrs import define from pytask import cli from pytask import ExitCode @@ -50,6 +51,43 @@ def task_example(): assert "out.txt>" in captured +@pytest.mark.xfail(reason="Only FilePathNodes are supported.") +@pytest.mark.end_to_end() +def test_collect_task_new_interface(runner, tmp_path): + source = """ + import pytask + from pathlib import Path + + def task_example(depends_on=Path("in.txt"), arg=1, produces=Path("out.txt")): + pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.txt").touch() + + result = runner.invoke(cli, ["collect", tmp_path.as_posix()]) + + assert result.exit_code == ExitCode.OK + captured = result.output.replace("\n", "").replace(" ", "") + assert "" in captured + assert "" in captured + + result = runner.invoke(cli, ["collect", tmp_path.as_posix(), "--nodes"]) + + assert result.exit_code == ExitCode.OK + captured = result.output.replace("\n", "").replace(" ", "") + assert "" in captured + assert "" in captured + assert "" in captured + assert "" in captured + assert "arg" in captured + + @pytest.mark.end_to_end() def test_collect_task_in_root_dir(runner, tmp_path): source = """ @@ -313,7 +351,7 @@ def state(self): ... -def function(): +def function(depends_on, produces): # noqa: ARG001 ... @@ -348,8 +386,16 @@ def test_print_collected_tasks_with_nodes(capsys): base_name="function", path=Path("task_path.py"), function=function, - depends_on={0: MetaNode("in.txt")}, - produces={0: MetaNode("out.txt")}, + depends_on={ + "depends_on": FilePathNode( + name="in.txt", value=Path("in.txt"), path=Path("in.txt") + ) + }, + produces={ + 0: FilePathNode( + name="out.txt", value=Path("out.txt"), path=Path("out.txt") + ) + }, ) ] } @@ -372,9 +418,13 @@ def test_find_common_ancestor_of_all_nodes(show_nodes, expected_add): base_name="function", path=Path.cwd() / "src" / "task_path.py", function=function, - depends_on={0: MetaNode(Path.cwd() / "src" / "in.txt")}, + depends_on={ + "depends_on": FilePathNode.from_path(Path.cwd() / "src" / "in.txt") + }, produces={ - 0: MetaNode(Path.cwd().joinpath("..", "bld", "out.txt").resolve()) + 0: FilePathNode.from_path( + Path.cwd().joinpath("..", "bld", "out.txt").resolve() + ) }, ) ] diff --git a/tests/test_database.py b/tests/test_database.py index 36d831d47..db46e0e4a 100644 --- a/tests/test_database.py +++ b/tests/test_database.py @@ -19,7 +19,7 @@ def test_existence_of_hashes_in_db(tmp_path, runner): @pytask.mark.depends_on("in.txt") @pytask.mark.produces("out.txt") - def task_write(produces): + def task_write(depends_on, produces): produces.touch() """ task_path = tmp_path.joinpath("task_module.py") diff --git a/tests/test_dry_run.py b/tests/test_dry_run.py index d73a5ab69..13114c05a 100644 --- a/tests/test_dry_run.py +++ b/tests/test_dry_run.py @@ -33,7 +33,7 @@ def test_dry_run_w_subsequent_task(runner, tmp_path): @pytask.mark.depends_on("out.txt") @pytask.mark.produces("out_2.txt") - def task_example(produces): + def task_example(depends_on, produces): produces.touch() """ tmp_path.joinpath("task_example_second.py").write_text(textwrap.dedent(source)) @@ -80,7 +80,7 @@ def task_example(produces): @pytask.mark.depends_on("out.txt") @pytask.mark.produces("out_2.txt") - def task_example(produces): + def task_example(depends_on, produces): produces.touch() """ tmp_path.joinpath("task_example_second.py").write_text(textwrap.dedent(source_2)) diff --git a/tests/test_graph.py b/tests/test_graph.py index 38f6158ca..c7aecc590 100644 --- a/tests/test_graph.py +++ b/tests/test_graph.py @@ -86,7 +86,7 @@ def test_create_graph_via_task(tmp_path, runner, format_, layout, rankdir): import networkx as nx @pytask.mark.depends_on("input.txt") - def task_example(): pass + def task_example(depends_on): pass def task_create_graph(): dag = pytask.build_dag({{"paths": Path(__file__).parent}}) diff --git a/tests/test_pybaum.py b/tests/test_pybaum.py index 69ceee349..66ea24e60 100644 --- a/tests/test_pybaum.py +++ b/tests/test_pybaum.py @@ -50,6 +50,8 @@ def task_example(): 2: {0: tmp_path / "list_out.txt"}, 3: {"a": tmp_path / "dict_out.txt", "b": {"c": tmp_path / "dict_out_2.txt"}}, } + if decorator_name == "depends_on": + expected = {"depends_on": expected} assert products == expected diff --git a/tests/test_skipping.py b/tests/test_skipping.py index f7c1ad7c2..bb18d0fae 100644 --- a/tests/test_skipping.py +++ b/tests/test_skipping.py @@ -195,11 +195,10 @@ def test_if_skipif_decorator_is_applied_execute(tmp_path): @pytask.mark.skipif(False, reason="bla") @pytask.mark.produces("out.txt") def task_first(produces): - with open(produces, "w") as f: - f.write("hello world.") + produces.touch() @pytask.mark.depends_on("out.txt") - def task_second(): + def task_second(depends_on): pass """ tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) diff --git a/tests/test_task.py b/tests/test_task.py index 3313fdb7b..bf4c16a37 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -438,3 +438,15 @@ def task_func(i=i): assert session.exit_code == ExitCode.COLLECTION_FAILED assert isinstance(session.collection_reports[0].exc_info[1], ValueError) + + +def test_task_receives_unknown_kwarg(runner, tmp_path): + source = """ + import pytask + + @pytask.mark.task(kwargs={"i": 1}) + def task_example(): pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.FAILED From 2279affcd29656968dc3a36bcaa05c34c38b6131 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 8 Jul 2023 22:17:16 +0200 Subject: [PATCH 06/12] Add products with `typing.Annotation`. (#394) --- .pre-commit-config.yaml | 3 - docs/rtd_environment.yml | 1 + environment.yml | 1 + pyproject.toml | 1 + setup.cfg | 1 + src/_pytask/_inspect.py | 140 +++++++++++++++++++++++++++++++++++ src/_pytask/collect.py | 6 +- src/_pytask/collect_utils.py | 105 ++++++++++++++++++++++++-- src/_pytask/execute.py | 5 +- src/_pytask/nodes.py | 10 ++- src/_pytask/traceback.py | 7 +- src/pytask/__init__.py | 2 + tests/test_collect.py | 40 ++++++++++ tests/test_collect_utils.py | 12 +++ tests/test_execute.py | 45 +++++++++++ tests/test_path.py | 8 ++ tests/test_pybaum.py | 3 +- tests/test_task.py | 1 + 18 files changed, 369 insertions(+), 22 deletions(-) create mode 100644 src/_pytask/_inspect.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index dac512006..21fdb08c1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,10 +21,7 @@ repos: hooks: - id: python-check-blanket-noqa - id: python-check-mock-methods - - id: python-no-eval - exclude: expression.py - id: python-no-log-warn - - id: python-use-type-annotations - id: text-unicode-replacement-char - repo: https://github.com/asottile/reorder-python-imports rev: v3.9.0 diff --git a/docs/rtd_environment.yml b/docs/rtd_environment.yml index 9bf674588..489116b66 100644 --- a/docs/rtd_environment.yml +++ b/docs/rtd_environment.yml @@ -30,6 +30,7 @@ dependencies: - rich - sqlalchemy >=1.4.36 - tomli >=1.0.0 + - typing_extensions - pip: - ../ diff --git a/environment.yml b/environment.yml index 1c58b9c53..586062e76 100644 --- a/environment.yml +++ b/environment.yml @@ -20,6 +20,7 @@ dependencies: - rich - sqlalchemy >=1.4.36 - tomli >=1.0.0 + - typing_extensions # Misc - black diff --git a/pyproject.toml b/pyproject.toml index d38fcd9ad..dc6b12a1e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -83,6 +83,7 @@ convention = "numpy" [tool.pytest.ini_options] +addopts = ["--doctest-modules"] testpaths = ["src", "tests"] markers = [ "wip: Tests that are work-in-progress.", diff --git a/setup.cfg b/setup.cfg index 700c89d57..2e009af94 100644 --- a/setup.cfg +++ b/setup.cfg @@ -40,6 +40,7 @@ install_requires = rich sqlalchemy>=1.4.36 tomli>=1.0.0 + typing-extensions python_requires = >=3.8 include_package_data = True package_dir = diff --git a/src/_pytask/_inspect.py b/src/_pytask/_inspect.py new file mode 100644 index 000000000..84413c1f7 --- /dev/null +++ b/src/_pytask/_inspect.py @@ -0,0 +1,140 @@ +from __future__ import annotations + +import functools +import sys +import types +from typing import Any +from typing import Callable +from typing import Mapping + + +__all__ = ["get_annotations"] + + +if sys.version_info >= (3, 10): + from inspect import get_annotations +else: + + def get_annotations( # noqa: C901, PLR0912, PLR0915 + obj: Callable[..., object] | type[Any] | types.ModuleType, + *, + globals: Mapping[str, Any] | None = None, # noqa: A002 + locals: Mapping[str, Any] | None = None, # noqa: A002 + eval_str: bool = False, + ) -> dict[str, Any]: + """Compute the annotations dict for an object. + + obj may be a callable, class, or module. + Passing in an object of any other type raises TypeError. + + Returns a dict. get_annotations() returns a new dict every time + it's called; calling it twice on the same object will return two + different but equivalent dicts. + + This function handles several details for you: + + * If eval_str is true, values of type str will + be un-stringized using eval(). This is intended + for use with stringized annotations + ("from __future__ import annotations"). + * If obj doesn't have an annotations dict, returns an + empty dict. (Functions and methods always have an + annotations dict; classes, modules, and other types of + callables may not.) + * Ignores inherited annotations on classes. If a class + doesn't have its own annotations dict, returns an empty dict. + * All accesses to object members and dict values are done + using getattr() and dict.get() for safety. + * Always, always, always returns a freshly-created dict. + + eval_str controls whether or not values of type str are replaced + with the result of calling eval() on those values: + + * If eval_str is true, eval() is called on values of type str. + * If eval_str is false (the default), values of type str are unchanged. + + globals and locals are passed in to eval(); see the documentation + for eval() for more information. If either globals or locals is + None, this function may replace that value with a context-specific + default, contingent on type(obj): + + * If obj is a module, globals defaults to obj.__dict__. + * If obj is a class, globals defaults to + sys.modules[obj.__module__].__dict__ and locals + defaults to the obj class namespace. + * If obj is a callable, globals defaults to obj.__globals__, + although if obj is a wrapped function (using + functools.update_wrapper()) it is first unwrapped. + """ + if isinstance(obj, type): + # class + obj_dict = getattr(obj, "__dict__", None) + if obj_dict and hasattr(obj_dict, "get"): + ann = obj_dict.get("__annotations__", None) + if isinstance(ann, types.GetSetDescriptorType): + ann = None + else: + ann = None + + obj_globals = None + module_name = getattr(obj, "__module__", None) + if module_name: + module = sys.modules.get(module_name, None) + if module: + obj_globals = getattr(module, "__dict__", None) + obj_locals = dict(vars(obj)) + unwrap = obj + elif isinstance(obj, types.ModuleType): + # module + ann = getattr(obj, "__annotations__", None) + obj_globals = obj.__dict__ + obj_locals = None + unwrap = None + elif callable(obj): + # this includes types.Function, types.BuiltinFunctionType, + # types.BuiltinMethodType, functools.partial, functools.singledispatch, + # "class funclike" from Lib/test/test_inspect... on and on it goes. + ann = getattr(obj, "__annotations__", None) + obj_globals = getattr(obj, "__globals__", None) + obj_locals = None + unwrap = obj + else: + raise TypeError(f"{obj!r} is not a module, class, or callable.") + + if ann is None: + return {} + + if not isinstance(ann, dict): + raise ValueError(f"{obj!r}.__annotations__ is neither a dict nor None") + + if not ann: + return {} + + if not eval_str: + return dict(ann) + + if unwrap is not None: + while True: + if hasattr(unwrap, "__wrapped__"): + unwrap = unwrap.__wrapped__ + continue + if isinstance(unwrap, functools.partial): + unwrap = unwrap.func + continue + break + if hasattr(unwrap, "__globals__"): + obj_globals = unwrap.__globals__ + + if globals is None: + globals = obj_globals # noqa: A001 + if locals is None: + locals = obj_locals # noqa: A001 + + eval_func = eval + return_value = { + key: value + if not isinstance(value, str) + else eval_func(value, globals, locals) + for key, value in ann.items() + } + return return_value diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index 8403a9e8d..fc65df988 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -15,7 +15,6 @@ from _pytask.collect_utils import parse_dependencies_from_task_function from _pytask.collect_utils import parse_nodes from _pytask.collect_utils import parse_products_from_task_function -from _pytask.collect_utils import produces from _pytask.config import hookimpl from _pytask.config import IS_FILE_SYSTEM_CASE_SENSITIVE from _pytask.console import console @@ -178,10 +177,7 @@ def pytask_collect_task( session, path, name, obj ) - if has_mark(obj, "produces"): - products = parse_nodes(session, path, name, obj, produces) - else: - products = parse_products_from_task_function(session, path, name, obj) + products = parse_products_from_task_function(session, path, name, obj) markers = obj.pytask_meta.markers if hasattr(obj, "pytask_meta") else [] diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 5eb6cff9a..0b179e8dc 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -11,14 +11,19 @@ from typing import Iterable from typing import TYPE_CHECKING +from _pytask._inspect import get_annotations from _pytask.exceptions import NodeNotCollectedError +from _pytask.mark_utils import has_mark from _pytask.mark_utils import remove_marks +from _pytask.nodes import ProductType from _pytask.nodes import PythonNode from _pytask.shared import find_duplicates from _pytask.task_utils import parse_keyword_arguments_from_signature_defaults from attrs import define from attrs import field from pybaum.tree_util import tree_map +from typing_extensions import Annotated +from typing_extensions import get_origin if TYPE_CHECKING: @@ -211,8 +216,12 @@ def parse_dependencies_from_task_function( kwargs = {**signature_defaults, **task_kwargs} kwargs.pop("produces", None) + parameters_with_product_annot = _find_args_with_product_annotation(obj) + dependencies = {} for name, value in kwargs.items(): + if name in parameters_with_product_annot: + continue parsed_value = tree_map( lambda x: _collect_dependencies(session, path, name, x), value # noqa: B023 ) @@ -223,29 +232,108 @@ def parse_dependencies_from_task_function( return dependencies +_ERROR_MULTIPLE_PRODUCT_DEFINITIONS = ( + "The task uses multiple ways to define products. Products should be defined with " + "either\n\n- 'typing.Annotated[Path(...), Product]' (recommended)\n" + "- '@pytask.mark.task(kwargs={'produces': Path(...)})'\n" + "- as a default argument for 'produces': 'produces = Path(...)'\n" + "- '@pytask.mark.produces(Path(...))' (deprecated).\n\n" + "Read more about products in the documentation: https://tinyurl.com/yrezszr4." +) + + def parse_products_from_task_function( session: Session, path: Path, name: str, obj: Any ) -> dict[str, Any]: - """Parse dependencies from task function.""" + """Parse products from task function. + + Raises + ------ + NodeNotCollectedError + If multiple ways were used to specify products. + + """ + has_produces_decorator = False + has_task_decorator = False + has_signature_default = False + has_annotation = False + out = {} + + if has_mark(obj, "produces"): + has_produces_decorator = True + nodes = parse_nodes(session, path, name, obj, produces) + out = {"produces": nodes} + task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} if "produces" in task_kwargs: - return tree_map( + collected_products = tree_map( lambda x: _collect_product(session, path, name, x, is_string_allowed=True), task_kwargs["produces"], ) + out = {"produces": collected_products} parameters = inspect.signature(obj).parameters - if "produces" in parameters: + + if not has_mark(obj, "task") and "produces" in parameters: parameter = parameters["produces"] if parameter.default is not parameter.empty: + has_signature_default = True # Use _collect_new_node to not collect strings. - return tree_map( + collected_products = tree_map( lambda x: _collect_product( session, path, name, x, is_string_allowed=False ), parameter.default, ) - return {} + out = {"produces": collected_products} + + parameters_with_product_annot = _find_args_with_product_annotation(obj) + if parameters_with_product_annot: + has_annotation = True + for parameter_name in parameters_with_product_annot: + parameter = parameters[parameter_name] + if parameter.default is not parameter.empty: + # Use _collect_new_node to not collect strings. + collected_products = tree_map( + lambda x: _collect_product( + session, path, name, x, is_string_allowed=False + ), + parameter.default, + ) + out = {parameter_name: collected_products} + + if ( + sum( + ( + has_produces_decorator, + has_task_decorator, + has_signature_default, + has_annotation, + ) + ) + >= 2 # noqa: PLR2004 + ): + raise NodeNotCollectedError(_ERROR_MULTIPLE_PRODUCT_DEFINITIONS) + + return out + + +def _find_args_with_product_annotation(func: Callable[..., Any]) -> list[str]: + """Find args with product annotation.""" + annotations = get_annotations(func, eval_str=True) + metas = { + name: annotation.__metadata__ + for name, annotation in annotations.items() + if get_origin(annotation) is Annotated + } + + args_with_product_annot = [] + for name, meta in metas.items(): + has_product_annot = any(isinstance(i, ProductType) for i in meta) + if has_product_annot: + args_with_product_annot.append(name) + + return args_with_product_annot def _collect_old_dependencies( @@ -331,9 +419,10 @@ def _collect_product( # The parameter defaults only support Path objects. if not isinstance(node, Path) and not is_string_allowed: raise ValueError( - "If you use 'produces' as an argument of a task, it can only accept values " - "of type 'pathlib.Path' or the same value nested in " - f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}." + "If you use 'produces' as a function argument of a task and pass values as " + "function defaults, it can only accept values of type 'pathlib.Path' or " + "the same value nested in tuples, lists, and dictionaries. Here, " + f"{node!r} has type {type(node)}." ) if isinstance(node, str): diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index 40e8e4b42..cce8a0544 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -149,8 +149,9 @@ def pytask_execute_task(session: Session, task: Task) -> bool: for name, value in task.depends_on.items(): kwargs[name] = tree_map(lambda x: x.value, value) - if task.produces and "produces" in parameters: - kwargs["produces"] = tree_map(lambda x: x.value, task.produces) + for name, value in task.produces.items(): + if name in parameters: + kwargs[name] = tree_map(lambda x: x.value, value) task.execute(**kwargs) return True diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index 47f7b9f19..1af06904b 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -17,7 +17,15 @@ from _pytask.mark import Mark -__all__ = ["FilePathNode", "MetaNode", "Task"] +__all__ = ["FilePathNode", "MetaNode", "Product", "Task"] + + +@define(frozen=True) +class ProductType: + """A class to mark products.""" + + +Product = ProductType() class MetaNode(metaclass=ABCMeta): diff --git a/src/_pytask/traceback.py b/src/_pytask/traceback.py index 38fdeb88a..cc14c1812 100644 --- a/src/_pytask/traceback.py +++ b/src/_pytask/traceback.py @@ -10,6 +10,7 @@ import _pytask import pluggy +import pybaum from rich.traceback import Traceback @@ -22,6 +23,7 @@ _PLUGGY_DIRECTORY = Path(pluggy.__file__).parent +_PYBAUM_DIRECTORY = Path(pybaum.__file__).parent _PYTASK_DIRECTORY = Path(_pytask.__file__).parent @@ -89,7 +91,10 @@ def _is_internal_or_hidden_traceback_frame( return True path = Path(frame.tb_frame.f_code.co_filename) - return any(root in path.parents for root in (_PLUGGY_DIRECTORY, _PYTASK_DIRECTORY)) + return any( + root in path.parents + for root in (_PLUGGY_DIRECTORY, _PYBAUM_DIRECTORY, _PYTASK_DIRECTORY) + ) def _filter_internal_traceback_frames( diff --git a/src/pytask/__init__.py b/src/pytask/__init__.py index f589e91ab..d84ec0f16 100644 --- a/src/pytask/__init__.py +++ b/src/pytask/__init__.py @@ -37,6 +37,7 @@ from _pytask.models import CollectionMetadata from _pytask.nodes import FilePathNode from _pytask.nodes import MetaNode +from _pytask.nodes import Product from _pytask.nodes import Task from _pytask.outcomes import CollectionOutcome from _pytask.outcomes import count_outcomes @@ -89,6 +90,7 @@ "NodeNotCollectedError", "NodeNotFoundError", "Persisted", + "Product", "PytaskError", "ResolvingDependenciesError", "Runtime", diff --git a/tests/test_collect.py b/tests/test_collect.py index 95c2ef226..6728e16ee 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -249,6 +249,7 @@ def test_find_shortest_uniquely_identifiable_names_for_tasks(tmp_path): assert result == expected +@pytest.mark.end_to_end() def test_collect_dependencies_from_args_if_depends_on_is_missing(tmp_path): source = """ from pathlib import Path @@ -306,6 +307,21 @@ def task_my_task(): assert outcome == CollectionOutcome.SUCCESS +@pytest.mark.end_to_end() +def test_collect_string_product_with_task_decorator(tmp_path): + source = """ + import pytask + + @pytask.mark.task + def task_write_text(produces="out.txt"): + produces.touch() + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + session = main({"paths": tmp_path}) + assert session.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").exists() + + @pytest.mark.end_to_end() def test_collect_string_product_as_function_default_fails(tmp_path): source = """ @@ -319,3 +335,27 @@ def task_write_text(produces="out.txt"): report = session.collection_reports[0] assert report.outcome == CollectionOutcome.FAIL assert "If you use 'produces'" in str(report.exc_info[1]) + + +@pytest.mark.end_to_end() +def test_product_cannot_mix_different_product_types(tmp_path): + source = """ + import pytask + from typing_extensions import Annotated + from pytask import Product + from pathlib import Path + + @pytask.mark.produces("out_deco.txt") + def task_example( + path: Annotated[Path, Product], produces: Path = Path("out_sig.txt") + ): + ... + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + session = main({"paths": tmp_path}) + + assert session.exit_code == ExitCode.COLLECTION_FAILED + assert len(session.tasks) == 0 + report = session.collection_reports[0] + assert report.outcome == CollectionOutcome.FAIL + assert "The task uses multiple ways" in str(report.exc_info[1]) diff --git a/tests/test_collect_utils.py b/tests/test_collect_utils.py index 89b995e0e..e4dbc4c10 100644 --- a/tests/test_collect_utils.py +++ b/tests/test_collect_utils.py @@ -9,10 +9,13 @@ from _pytask.collect_utils import _convert_objects_to_node_dictionary from _pytask.collect_utils import _convert_to_dict from _pytask.collect_utils import _extract_nodes_from_function_markers +from _pytask.collect_utils import _find_args_with_product_annotation from _pytask.collect_utils import _merge_dictionaries from _pytask.collect_utils import _Placeholder from pytask import depends_on from pytask import produces +from pytask import Product +from typing_extensions import Annotated ERROR = "'@pytask.mark.depends_on' has nodes with the same name:" @@ -159,3 +162,12 @@ def task_example(): # pragma: no cover parser = depends_on if decorator.name == "depends_on" else produces with pytest.raises(TypeError): list(_extract_nodes_from_function_markers(task_example, parser)) + + +@pytest.mark.unit() +def test_find_args_with_product_annotation(): + def func(a: Annotated[int, Product], b: float, c, d: Annotated[int, float]): + return a, b, c, d + + result = _find_args_with_product_annotation(func) + assert result == ["a"] diff --git a/tests/test_execute.py b/tests/test_execute.py index 9e9e5faa8..4c0f2a7ca 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -8,6 +8,7 @@ from pathlib import Path import pytest +from _pytask.capture import _CaptureMethod from _pytask.exceptions import NodeNotFoundError from pytask import cli from pytask import ExitCode @@ -385,6 +386,7 @@ def task_example(): assert "Collected 1 task" in result.output +@pytest.mark.end_to_end() def test_task_executed_with_force_although_unchanged(tmp_path): tmp_path.joinpath("task_module.py").write_text("def task_example(): pass") session = main({"paths": tmp_path}) @@ -419,3 +421,46 @@ def test_task_is_not_reexecuted_when_modification_changed_file_not(runner, tmp_p result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.OK assert "1 Skipped" in result.output + + +@pytest.mark.end_to_end() +def test_task_with_product_annotation(tmp_path): + source = """ + from pathlib import Path + from typing_extensions import Annotated + from pytask import Product + + def task_example(path_to_file: Annotated[Path, Product] = Path("out.txt")) -> None: + path_to_file.touch() + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + session = main({"paths": tmp_path, "capture": _CaptureMethod.NO}) + + assert session.exit_code == ExitCode.OK + assert len(session.tasks) == 1 + task = session.tasks[0] + assert "path_to_file" in task.produces + + +@pytest.mark.end_to_end() +@pytest.mark.xfail(reason="Nested annotations are not parsed.", raises=AssertionError) +def test_task_with_nested_product_annotation(tmp_path): + source = """ + from pathlib import Path + from typing_extensions import Annotated + from pytask import Product + + def task_example( + paths_to_file: dict[str, Annotated[Path, Product]] = {"a": Path("out.txt")} + ) -> None: + pass + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + session = main({"paths": tmp_path, "capture": _CaptureMethod.NO}) + + assert session.exit_code == ExitCode.OK + assert len(session.tasks) == 1 + task = session.tasks[0] + assert "paths_to_file" in task.produces diff --git a/tests/test_path.py b/tests/test_path.py index 58737a47b..e55be2f79 100644 --- a/tests/test_path.py +++ b/tests/test_path.py @@ -133,6 +133,7 @@ def simple_module(tmp_path: Path) -> Path: return fn +@pytest.mark.unit() def test_importmode_importlib(simple_module: Path, tmp_path: Path) -> None: """`importlib` mode does not change sys.path.""" module = import_path(simple_module, root=tmp_path) @@ -144,6 +145,7 @@ def test_importmode_importlib(simple_module: Path, tmp_path: Path) -> None: assert "_src.project" in sys.modules +@pytest.mark.unit() def test_importmode_twice_is_different_module( simple_module: Path, tmp_path: Path ) -> None: @@ -153,6 +155,7 @@ def test_importmode_twice_is_different_module( assert module1 is not module2 +@pytest.mark.unit() def test_no_meta_path_found( simple_module: Path, monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: @@ -171,6 +174,7 @@ def test_no_meta_path_found( import_path(simple_module, root=tmp_path) +@pytest.mark.unit() def test_importmode_importlib_with_dataclass(tmp_path: Path) -> None: """ Ensure that importlib mode works with a module containing dataclasses (#373, @@ -197,6 +201,7 @@ class Data: assert data.__module__ == "_src.project.task_dataclass" +@pytest.mark.unit() def test_importmode_importlib_with_pickle(tmp_path: Path) -> None: """Ensure that importlib mode works with pickle (#373, pytest#7859).""" fn = tmp_path.joinpath("_src/project/task_pickle.py") @@ -222,6 +227,7 @@ def round_trip(): assert action() == 42 +@pytest.mark.unit() def test_importmode_importlib_with_pickle_separate_modules(tmp_path: Path) -> None: """ Ensure that importlib mode works can load pickles that look similar but are @@ -275,6 +281,7 @@ def round_trip(obj): assert Data2.__module__ == "_src.m2.project.task" +@pytest.mark.unit() def test_module_name_from_path(tmp_path: Path) -> None: result = _module_name_from_path(tmp_path / "src/project/task_foo.py", tmp_path) assert result == "src.project.task_foo" @@ -284,6 +291,7 @@ def test_module_name_from_path(tmp_path: Path) -> None: assert result == "home.foo.task_foo" +@pytest.mark.unit() def test_insert_missing_modules( monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/tests/test_pybaum.py b/tests/test_pybaum.py index 66ea24e60..b6889fbd1 100644 --- a/tests/test_pybaum.py +++ b/tests/test_pybaum.py @@ -50,8 +50,7 @@ def task_example(): 2: {0: tmp_path / "list_out.txt"}, 3: {"a": tmp_path / "dict_out.txt", "b": {"c": tmp_path / "dict_out_2.txt"}}, } - if decorator_name == "depends_on": - expected = {"depends_on": expected} + expected = {decorator_name: expected} assert products == expected diff --git a/tests/test_task.py b/tests/test_task.py index bf4c16a37..ccbadfaef 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -440,6 +440,7 @@ def task_func(i=i): assert isinstance(session.collection_reports[0].exc_info[1], ValueError) +@pytest.mark.end_to_end() def test_task_receives_unknown_kwarg(runner, tmp_path): source = """ import pytask From 47e073e140fca916c3417b4e7fdab5d2c6733d87 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sat, 8 Jul 2023 22:41:06 +0200 Subject: [PATCH 07/12] Refactor pybaum to `_pytask.tree_util`. (#395) --- docs/source/changes.md | 3 +++ src/_pytask/clean.py | 2 +- src/_pytask/collect_command.py | 2 +- src/_pytask/collect_utils.py | 2 +- src/_pytask/dag.py | 2 +- src/_pytask/execute.py | 2 +- src/_pytask/traceback.py | 5 ++--- src/_pytask/tree_util.py | 18 ++++++++++++++++++ tests/{test_pybaum.py => test_tree_util.py} | 8 ++++---- 9 files changed, 32 insertions(+), 12 deletions(-) create mode 100644 src/_pytask/tree_util.py rename tests/{test_pybaum.py => test_tree_util.py} (90%) diff --git a/docs/source/changes.md b/docs/source/changes.md index 19698bc51..6674f1a87 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -12,6 +12,9 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and is not present. - {pull}`387` replaces pony with sqlalchemy. - {pull}`391` removes `@pytask.mark.parametrize`. +- {pull}`394` allows to add products with {obj}`typing.Annotation` and + {obj}`~pytask.Product`. +- {pull}`395` refactors all occurrences of pybaum to {mod}`_pytask.tree_util`. ## 0.3.2 - 2023-06-07 diff --git a/src/_pytask/clean.py b/src/_pytask/clean.py index 2004670e2..d0aee55d2 100644 --- a/src/_pytask/clean.py +++ b/src/_pytask/clean.py @@ -29,8 +29,8 @@ from _pytask.session import Session from _pytask.shared import to_list from _pytask.traceback import render_exc_info +from _pytask.tree_util import tree_just_yield from attrs import define -from pybaum.tree_util import tree_just_yield if TYPE_CHECKING: diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index e297c823a..9fda4258c 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -26,7 +26,7 @@ from _pytask.path import relative_to from _pytask.pluginmanager import get_plugin_manager from _pytask.session import Session -from pybaum.tree_util import tree_just_flatten +from _pytask.tree_util import tree_just_flatten from rich.text import Text from rich.tree import Tree diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 0b179e8dc..c82ceb970 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -19,9 +19,9 @@ from _pytask.nodes import PythonNode from _pytask.shared import find_duplicates from _pytask.task_utils import parse_keyword_arguments_from_signature_defaults +from _pytask.tree_util import tree_map from attrs import define from attrs import field -from pybaum.tree_util import tree_map from typing_extensions import Annotated from typing_extensions import get_origin diff --git a/src/_pytask/dag.py b/src/_pytask/dag.py index bbf199a5d..b0865b7ba 100644 --- a/src/_pytask/dag.py +++ b/src/_pytask/dag.py @@ -31,7 +31,7 @@ from _pytask.shared import reduce_names_of_multiple_nodes from _pytask.shared import reduce_node_name from _pytask.traceback import render_exc_info -from pybaum import tree_map +from _pytask.tree_util import tree_map from rich.text import Text from rich.tree import Tree diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index cce8a0544..bcc3fb418 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -33,7 +33,7 @@ from _pytask.traceback import format_exception_without_traceback from _pytask.traceback import remove_traceback_from_exc_info from _pytask.traceback import render_exc_info -from pybaum.tree_util import tree_map +from _pytask.tree_util import tree_map from rich.text import Text diff --git a/src/_pytask/traceback.py b/src/_pytask/traceback.py index cc14c1812..88c23aef8 100644 --- a/src/_pytask/traceback.py +++ b/src/_pytask/traceback.py @@ -10,7 +10,7 @@ import _pytask import pluggy -import pybaum +from _pytask.tree_util import TREE_UTIL_LIB_DIRECTORY from rich.traceback import Traceback @@ -23,7 +23,6 @@ _PLUGGY_DIRECTORY = Path(pluggy.__file__).parent -_PYBAUM_DIRECTORY = Path(pybaum.__file__).parent _PYTASK_DIRECTORY = Path(_pytask.__file__).parent @@ -93,7 +92,7 @@ def _is_internal_or_hidden_traceback_frame( path = Path(frame.tb_frame.f_code.co_filename) return any( root in path.parents - for root in (_PLUGGY_DIRECTORY, _PYBAUM_DIRECTORY, _PYTASK_DIRECTORY) + for root in (_PLUGGY_DIRECTORY, TREE_UTIL_LIB_DIRECTORY, _PYTASK_DIRECTORY) ) diff --git a/src/_pytask/tree_util.py b/src/_pytask/tree_util.py new file mode 100644 index 000000000..65d17bdd6 --- /dev/null +++ b/src/_pytask/tree_util.py @@ -0,0 +1,18 @@ +from __future__ import annotations + +from pathlib import Path + +import pybaum +from pybaum.tree_util import tree_just_flatten +from pybaum.tree_util import tree_just_yield +from pybaum.tree_util import tree_map + + +__all__ = [ + "tree_just_flatten", + "tree_map", + "tree_just_yield", + "TREE_UTIL_LIB_DIRECTORY", +] + +TREE_UTIL_LIB_DIRECTORY = Path(pybaum.__file__).parent diff --git a/tests/test_pybaum.py b/tests/test_tree_util.py similarity index 90% rename from tests/test_pybaum.py rename to tests/test_tree_util.py index b6889fbd1..be6212a47 100644 --- a/tests/test_pybaum.py +++ b/tests/test_tree_util.py @@ -1,11 +1,11 @@ -"""This module contains tests for pybaum and flexible dependencies and products.""" +"""This module contains tests for tree_util and flexible dependencies and products.""" from __future__ import annotations import textwrap import pytest from _pytask.outcomes import ExitCode -from pybaum import tree_map +from _pytask.tree_util import tree_map from pytask import cli from pytask import main @@ -55,11 +55,11 @@ def task_example(): @pytest.mark.end_to_end() -def test_profile_with_pybaum(tmp_path, runner): +def test_profile_with_pytree(tmp_path, runner): source = """ import time import pytask - from pybaum.tree_util import tree_just_flatten + from _pytask.tree_util import tree_just_flatten @pytask.mark.produces([{"out_1": "out_1.txt"}, {"out_2": "out_2.txt"}]) def task_example(produces): From 05c3b6d7e6f3a05b14c895cacae97b4c8a470479 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Thu, 13 Jul 2023 09:52:46 +0200 Subject: [PATCH 08/12] Replace pybaum with optree and add paths to PythonNode names. (#396) --- .pre-commit-config.yaml | 2 +- docs/rtd_environment.yml | 2 +- docs/source/changes.md | 2 + docs/source/reference_guides/api.md | 6 ++ environment.yml | 2 +- setup.cfg | 2 +- src/_pytask/clean.py | 4 +- src/_pytask/collect.py | 60 ++++-------- src/_pytask/collect_command.py | 48 ++++----- src/_pytask/collect_utils.py | 147 +++++++++++++++++++++------- src/_pytask/dag.py | 30 +++--- src/_pytask/database_utils.py | 20 ++-- src/_pytask/hookspecs.py | 3 +- src/_pytask/models.py | 7 ++ src/_pytask/nodes.py | 64 ++++++++---- src/_pytask/tree_util.py | 27 +++-- src/pytask/__init__.py | 4 + tests/test_collect.py | 17 ++-- tests/test_collect_command.py | 77 ++++++++++++++- tests/test_execute.py | 80 +++++++++++++++ tests/test_tree_util.py | 4 +- tox.ini | 2 +- 22 files changed, 436 insertions(+), 174 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 21fdb08c1..cdc3bb664 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -49,7 +49,7 @@ repos: rev: 1.5.0 hooks: - id: interrogate - args: [-v, --fail-under=75] + args: [-vv, --fail-under=75] exclude: ^(tests/|docs/|scripts/) - repo: https://github.com/pre-commit/mirrors-mypy rev: 'v1.3.0' diff --git a/docs/rtd_environment.yml b/docs/rtd_environment.yml index 489116b66..5ac57126d 100644 --- a/docs/rtd_environment.yml +++ b/docs/rtd_environment.yml @@ -25,7 +25,7 @@ dependencies: - click-default-group - networkx >=2.4 - pluggy - - pybaum >=0.1.1 + - optree >=0.9 - pexpect - rich - sqlalchemy >=1.4.36 diff --git a/docs/source/changes.md b/docs/source/changes.md index 6674f1a87..7998f5748 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -15,6 +15,8 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`394` allows to add products with {obj}`typing.Annotation` and {obj}`~pytask.Product`. - {pull}`395` refactors all occurrences of pybaum to {mod}`_pytask.tree_util`. +- {pull}`396` replaces pybaum with optree and adds paths to the name of + {class}`pytask.PythonNode`'s allowing for better hashing. ## 0.3.2 - 2023-06-07 diff --git a/docs/source/reference_guides/api.md b/docs/source/reference_guides/api.md index 8924bb136..2227723db 100644 --- a/docs/source/reference_guides/api.md +++ b/docs/source/reference_guides/api.md @@ -247,6 +247,12 @@ Then, different kinds of nodes can be implemented. ```{eval-rst} .. autoclass:: pytask.FilePathNode + :members: +``` + +```{eval-rst} +.. autoclass:: pytask.PythonNode + :members: ``` To parse dependencies and products from nodes, use the following functions. diff --git a/environment.yml b/environment.yml index 586062e76..e04446494 100644 --- a/environment.yml +++ b/environment.yml @@ -16,7 +16,7 @@ dependencies: - click-default-group - networkx >=2.4 - pluggy - - pybaum >=0.1.1 + - optree >=0.9 - rich - sqlalchemy >=1.4.36 - tomli >=1.0.0 diff --git a/setup.cfg b/setup.cfg index 2e009af94..1a39bbfb6 100644 --- a/setup.cfg +++ b/setup.cfg @@ -34,9 +34,9 @@ install_requires = click click-default-group networkx>=2.4 + optree>=0.9 packaging pluggy - pybaum>=0.1.1 rich sqlalchemy>=1.4.36 tomli>=1.0.0 diff --git a/src/_pytask/clean.py b/src/_pytask/clean.py index d0aee55d2..ecf515626 100644 --- a/src/_pytask/clean.py +++ b/src/_pytask/clean.py @@ -29,7 +29,7 @@ from _pytask.session import Session from _pytask.shared import to_list from _pytask.traceback import render_exc_info -from _pytask.tree_util import tree_just_yield +from _pytask.tree_util import tree_leaves from attrs import define @@ -218,7 +218,7 @@ def _yield_paths_from_task(task: Task) -> Generator[Path, None, None]: """Yield all paths attached to a task.""" yield task.path for attribute in ("depends_on", "produces"): - for node in tree_just_yield(getattr(task, attribute)): + for node in tree_leaves(getattr(task, attribute)): if hasattr(node, "path") and isinstance(node.path, Path): yield node.path diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index fc65df988..d4d135c96 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -11,9 +11,7 @@ from typing import Generator from typing import Iterable -from _pytask.collect_utils import depends_on from _pytask.collect_utils import parse_dependencies_from_task_function -from _pytask.collect_utils import parse_nodes from _pytask.collect_utils import parse_products_from_task_function from _pytask.config import hookimpl from _pytask.config import IS_FILE_SYSTEM_CASE_SENSITIVE @@ -22,7 +20,9 @@ from _pytask.console import format_task_id from _pytask.exceptions import CollectionError from _pytask.mark_utils import has_mark +from _pytask.models import NodeInfo from _pytask.nodes import FilePathNode +from _pytask.nodes import MetaNode from _pytask.nodes import PythonNode from _pytask.nodes import Task from _pytask.outcomes import CollectionOutcome @@ -169,13 +169,7 @@ def pytask_collect_task( """ if (name.startswith("task_") or has_mark(obj, "task")) and callable(obj): - if has_mark(obj, "depends_on"): - nodes = parse_nodes(session, path, name, obj, depends_on) - dependencies = {"depends_on": nodes} - else: - dependencies = parse_dependencies_from_task_function( - session, path, name, obj - ) + dependencies = parse_dependencies_from_task_function(session, path, name, obj) products = parse_products_from_task_function(session, path, name, obj) @@ -210,9 +204,7 @@ def pytask_collect_task( @hookimpl(trylast=True) -def pytask_collect_node( - session: Session, path: Path, node: str | Path -) -> FilePathNode | PythonNode: +def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> MetaNode: """Collect a node of a task as a :class:`pytask.nodes.FilePathNode`. Strings are assumed to be paths. This might be a strict assumption, but since this @@ -222,17 +214,18 @@ def pytask_collect_node( ``trylast=True`` might be necessary if other plugins try to parse strings themselves like a plugin for downloading files which depends on URLs given as strings. - Parameters - ---------- - session : _pytask.session.Session - The session. - path : Union[str, pathlib.Path] - The path to file where the task and node are specified. - node : Union[str, pathlib.Path] - The value of the node which can be a str, a path or anything which cannot be - handled by this function. - """ + node = node_info.value + + if isinstance(node, PythonNode): + if not node.name: + suffix = "-" + "-".join(map(str, node_info.path)) if node_info.path else "" + node.name = node_info.arg_name + suffix + return node + + if isinstance(node, MetaNode): + return node + if isinstance(node, Path): if not node.is_absolute(): node = path.parent.joinpath(node) @@ -251,7 +244,10 @@ def pytask_collect_node( raise ValueError(_TEMPLATE_ERROR.format(node, case_sensitive_path)) return FilePathNode.from_path(node) - return PythonNode(value=node) + + suffix = "-" + "-".join(map(str, node_info.path)) if node_info.path else "" + node_name = node_info.arg_name + suffix + return PythonNode(value=node, name=node_name) def _not_ignored_paths( @@ -263,18 +259,6 @@ def _not_ignored_paths( directories, all subsequent files and folders are considered, but one level after another, so that files of ignored folders are not checked. - Parameters - ---------- - paths : Iterable[pathlib.Path] - List of paths from which tasks are collected. - session : _pytask.session.Session - The session. - - Yields - ------ - path : pathlib.Path - A path which is not ignored. - """ for path in paths: if not session.hook.pytask_ignore_collect(path=path, config=session.config): @@ -287,11 +271,7 @@ def _not_ignored_paths( @hookimpl(trylast=True) def pytask_collect_modify_tasks(tasks: list[Task]) -> None: - """Given all tasks, assign a short uniquely identifiable name to each task. - - The shorter ids are necessary to display - - """ + """Given all tasks, assign a short uniquely identifiable name to each task.""" id_to_short_id = _find_shortest_uniquely_identifiable_name_for_tasks(tasks) for task in tasks: short_id = id_to_short_id[task.name] diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index 9fda4258c..e615a8ea9 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -26,7 +26,7 @@ from _pytask.path import relative_to from _pytask.pluginmanager import get_plugin_manager from _pytask.session import Session -from _pytask.tree_util import tree_just_flatten +from _pytask.tree_util import tree_leaves from rich.text import Text from rich.tree import Tree @@ -126,10 +126,10 @@ def _find_common_ancestor_of_all_nodes( if show_nodes: all_paths.extend( x.path - for x in tree_just_flatten(task.depends_on) + for x in tree_leaves(task.depends_on) if isinstance(x, FilePathNode) ) - all_paths.extend(x.path for x in tree_just_flatten(task.produces)) + all_paths.extend(x.path for x in tree_leaves(task.produces)) common_ancestor = find_common_ancestor(*all_paths, *paths) @@ -150,7 +150,7 @@ def _organize_tasks(tasks: list[Task]) -> dict[Path, list[Task]]: sorted_dict = {} for k in sorted(dictionary): - sorted_dict[k] = sorted(dictionary[k], key=lambda x: x.path) + sorted_dict[k] = sorted(dictionary[k], key=lambda x: x.name) return sorted_dict @@ -202,36 +202,24 @@ def _print_collected_tasks( ) if show_nodes: - file_path_nodes = [ - i - for i in tree_just_flatten(task.depends_on) - if isinstance(i, FilePathNode) - ] - sorted_nodes = sorted(file_path_nodes, key=lambda x: x.path) + file_path_nodes = list(tree_leaves(task.depends_on)) + sorted_nodes = sorted(file_path_nodes, key=lambda x: x.name) for node in sorted_nodes: - reduced_node_name = relative_to(node.path, common_ancestor) - url_style = create_url_style_for_path(node.path, editor_url_scheme) - task_branch.add( - Text.assemble( - FILE_ICON, - "", + if isinstance(node, FilePathNode): + reduced_node_name = relative_to(node.path, common_ancestor) + url_style = create_url_style_for_path( + node.path, editor_url_scheme ) - ) + text = Text(str(reduced_node_name), style=url_style) + else: + text = node.name - for node in sorted( - tree_just_flatten(task.produces), key=lambda x: x.path - ): + task_branch.add(Text.assemble(FILE_ICON, "")) + + for node in sorted(tree_leaves(task.produces), key=lambda x: x.path): reduced_node_name = relative_to(node.path, common_ancestor) url_style = create_url_style_for_path(node.path, editor_url_scheme) - task_branch.add( - Text.assemble( - FILE_ICON, - "", - ) - ) + text = Text(str(reduced_node_name), style=url_style) + task_branch.add(Text.assemble(FILE_ICON, "")) console.print(tree) diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index c82ceb970..696c4c055 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -11,15 +11,20 @@ from typing import Iterable from typing import TYPE_CHECKING +import attrs from _pytask._inspect import get_annotations from _pytask.exceptions import NodeNotCollectedError from _pytask.mark_utils import has_mark from _pytask.mark_utils import remove_marks +from _pytask.models import NodeInfo +from _pytask.nodes import MetaNode from _pytask.nodes import ProductType from _pytask.nodes import PythonNode from _pytask.shared import find_duplicates from _pytask.task_utils import parse_keyword_arguments_from_signature_defaults +from _pytask.tree_util import tree_leaves from _pytask.tree_util import tree_map +from _pytask.tree_util import tree_map_with_path from attrs import define from attrs import field from typing_extensions import Annotated @@ -28,7 +33,6 @@ if TYPE_CHECKING: from _pytask.session import Session - from _pytask.nodes import MetaNode __all__ = [ @@ -211,35 +215,93 @@ def parse_dependencies_from_task_function( session: Session, path: Path, name: str, obj: Any ) -> dict[str, Any]: """Parse dependencies from task function.""" + if has_mark(obj, "depends_on"): + nodes = parse_nodes(session, path, name, obj, depends_on) + return {"depends_on": nodes} + task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} signature_defaults = parse_keyword_arguments_from_signature_defaults(obj) kwargs = {**signature_defaults, **task_kwargs} kwargs.pop("produces", None) parameters_with_product_annot = _find_args_with_product_annotation(obj) + parameters_with_node_annot = _find_args_with_node_annotation(obj) dependencies = {} - for name, value in kwargs.items(): - if name in parameters_with_product_annot: + for parameter_name, value in kwargs.items(): + if parameter_name in parameters_with_product_annot: continue - parsed_value = tree_map( - lambda x: _collect_dependencies(session, path, name, x), value # noqa: B023 - ) - dependencies[name] = ( - PythonNode(value=None) if parsed_value is None else parsed_value + + if parameter_name in parameters_with_node_annot: + + def _evolve(x: Any) -> Any: + instance = parameters_with_node_annot[parameter_name] # noqa: B023 + return attrs.evolve(instance, value=x) # type: ignore[misc] + + else: + + def _evolve(x: Any) -> Any: + return x + + nodes = tree_map_with_path( + lambda p, x: _collect_dependencies( + session, + path, + name, + NodeInfo(parameter_name, p, _evolve(x)), # noqa: B023 + ), + value, ) + # If all nodes are python nodes, we simplify the parameter value and store it in + # one node. + are_all_nodes_python_nodes_without_hash = all( + isinstance(x, PythonNode) and not x.hash for x in tree_leaves(nodes) + ) + if are_all_nodes_python_nodes_without_hash: + dependencies[parameter_name] = PythonNode(value=value, name=parameter_name) + else: + dependencies[parameter_name] = nodes return dependencies -_ERROR_MULTIPLE_PRODUCT_DEFINITIONS = ( - "The task uses multiple ways to define products. Products should be defined with " - "either\n\n- 'typing.Annotated[Path(...), Product]' (recommended)\n" - "- '@pytask.mark.task(kwargs={'produces': Path(...)})'\n" - "- as a default argument for 'produces': 'produces = Path(...)'\n" - "- '@pytask.mark.produces(Path(...))' (deprecated).\n\n" - "Read more about products in the documentation: https://tinyurl.com/yrezszr4." -) +def _find_args_with_node_annotation(func: Callable[..., Any]) -> dict[str, MetaNode]: + """Find args with node annotations.""" + annotations = get_annotations(func, eval_str=True) + metas = { + name: annotation.__metadata__ + for name, annotation in annotations.items() + if get_origin(annotation) is Annotated + } + + args_with_node_annotation = {} + for name, meta in metas.items(): + annot = [ + i + for i in meta + if not isinstance(i, ProductType) and isinstance(i, MetaNode) + ] + if len(annot) >= 2: # noqa: PLR2004 + raise ValueError( + f"Parameter {name!r} has multiple node annotations although only one " + f"is allowed. Annotations: {annot}" + ) + if annot: + args_with_node_annotation[name] = annot[0] + + return args_with_node_annotation + + +_ERROR_MULTIPLE_PRODUCT_DEFINITIONS = """The task uses multiple ways to define \ +products. Products should be defined with either + +- 'typing.Annotated[Path(...), Product]' (recommended) +- '@pytask.mark.task(kwargs={'produces': Path(...)})' +- as a default argument for 'produces': 'produces = Path(...)' +- '@pytask.mark.produces(Path(...))' (deprecated) + +Read more about products in the documentation: https://tinyurl.com/yrezszr4. +""" def parse_products_from_task_function( @@ -266,8 +328,14 @@ def parse_products_from_task_function( task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} if "produces" in task_kwargs: - collected_products = tree_map( - lambda x: _collect_product(session, path, name, x, is_string_allowed=True), + collected_products = tree_map_with_path( + lambda p, x: _collect_product( + session, + path, + name, + NodeInfo(arg_name="produces", path=p, value=x), + is_string_allowed=True, + ), task_kwargs["produces"], ) out = {"produces": collected_products} @@ -279,9 +347,13 @@ def parse_products_from_task_function( if parameter.default is not parameter.empty: has_signature_default = True # Use _collect_new_node to not collect strings. - collected_products = tree_map( - lambda x: _collect_product( - session, path, name, x, is_string_allowed=False + collected_products = tree_map_with_path( + lambda p, x: _collect_product( + session, + path, + name, + NodeInfo(arg_name="produces", path=p, value=x), + is_string_allowed=False, ), parameter.default, ) @@ -294,9 +366,13 @@ def parse_products_from_task_function( parameter = parameters[parameter_name] if parameter.default is not parameter.empty: # Use _collect_new_node to not collect strings. - collected_products = tree_map( - lambda x: _collect_product( - session, path, name, x, is_string_allowed=False + collected_products = tree_map_with_path( + lambda p, x: _collect_product( + session, + path, + name, + NodeInfo(parameter_name, p, x), # noqa: B023 + is_string_allowed=False, ), parameter.default, ) @@ -319,7 +395,7 @@ def parse_products_from_task_function( def _find_args_with_product_annotation(func: Callable[..., Any]) -> list[str]: - """Find args with product annotation.""" + """Find args with product annotations.""" annotations = get_annotations(func, eval_str=True) metas = { name: annotation.__metadata__ @@ -358,19 +434,18 @@ def _collect_old_dependencies( node = Path(node) collected_node = session.hook.pytask_collect_node( - session=session, path=path, node=node + session=session, path=path, node_info=NodeInfo("produces", (), node) ) if collected_node is None: raise NodeNotCollectedError( - f"{node!r} cannot be parsed as a dependency or product for task " - f"{name!r} in {path!r}." + f"{node!r} cannot be parsed as a dependency for task {name!r} in {path!r}." ) return collected_node def _collect_dependencies( - session: Session, path: Path, name: str, node: Any + session: Session, path: Path, name: str, node_info: NodeInfo ) -> dict[str, MetaNode]: """Collect nodes for a task. @@ -380,8 +455,10 @@ def _collect_dependencies( If the node could not collected. """ + node = node_info.value + collected_node = session.hook.pytask_collect_node( - session=session, path=path, node=node + session=session, path=path, node_info=node_info, node=node ) if collected_node is None: raise NodeNotCollectedError( @@ -394,8 +471,8 @@ def _collect_dependencies( def _collect_product( session: Session, path: Path, - name: str, - node: str | Path, + task_name: str, + node_info: NodeInfo, is_string_allowed: bool = False, ) -> dict[str, MetaNode]: """Collect products for a task. @@ -409,6 +486,7 @@ def _collect_product( If the node could not collected. """ + node = node_info.value # For historical reasons, task.kwargs is like the deco and supports str and Path. if not isinstance(node, (str, Path)) and is_string_allowed: raise ValueError( @@ -429,12 +507,11 @@ def _collect_product( node = Path(node) collected_node = session.hook.pytask_collect_node( - session=session, path=path, node=node + session=session, path=path, node_info=node_info, node=node ) if collected_node is None: raise NodeNotCollectedError( - f"{node!r} cannot be parsed as a dependency or product for task " - f"{name!r} in {path!r}." + f"{node!r} can't be parsed as a product for task {task_name!r} in {path!r}." ) return collected_node diff --git a/src/_pytask/dag.py b/src/_pytask/dag.py index b0865b7ba..bee58b988 100644 --- a/src/_pytask/dag.py +++ b/src/_pytask/dag.py @@ -1,7 +1,6 @@ """This module contains code related to resolving dependencies.""" from __future__ import annotations -import hashlib import itertools import sys @@ -129,21 +128,21 @@ def _have_task_or_neighbors_changed( @hookimpl(trylast=True) def pytask_dag_has_node_changed(node: MetaNode, task_name: str) -> bool: """Indicate whether a single dependency or product has changed.""" - if isinstance(node, (FilePathNode, Task)): - # If node does not exist, we receive None. - file_state = node.state() - if file_state is None: - return True + # If node does not exist, we receive None. + node_state = node.state() + if node_state is None: + return True - with DatabaseSession() as session: - db_state = session.get(State, (task_name, node.name)) + with DatabaseSession() as session: + db_state = session.get(State, (task_name, node.name)) - # If the node is not in the database. - if db_state is None: - return True + # If the node is not in the database. + if db_state is None: + return True + if isinstance(node, (FilePathNode, Task)): # If the modification times match, the node has not been changed. - if file_state == db_state.modification_time: + if node_state == db_state.modification_time: return False # If the modification time changed, quickly return for non-tasks. @@ -152,9 +151,10 @@ def pytask_dag_has_node_changed(node: MetaNode, task_name: str) -> bool: # When modification times changed, we are still comparing the hash of the file # to avoid unnecessary and expensive reexecutions of tasks. - file_hash = hashlib.sha256(node.path.read_bytes()).hexdigest() - return file_hash != db_state.file_hash - return node.state() + hash_ = node.state(hash=True) + return hash_ != db_state.hash_ + + return node_state != db_state.hash_ def _check_if_dag_has_cycles(dag: nx.DiGraph) -> None: diff --git a/src/_pytask/database_utils.py b/src/_pytask/database_utils.py index 5e62d2783..05a09de1b 100644 --- a/src/_pytask/database_utils.py +++ b/src/_pytask/database_utils.py @@ -4,6 +4,7 @@ import hashlib from _pytask.dag_utils import node_and_neighbors +from _pytask.nodes import FilePathNode from _pytask.nodes import Task from _pytask.session import Session from sqlalchemy import Column @@ -35,7 +36,7 @@ class State(BaseTable): # type: ignore[valid-type, misc] task = Column(String, primary_key=True) node = Column(String, primary_key=True) modification_time = Column(String) - file_hash = Column(String) + hash_ = Column(String) def create_database(url: str) -> None: @@ -49,7 +50,7 @@ def create_database(url: str) -> None: def _create_or_update_state( - first_key: str, second_key: str, modification_time: str, file_hash: str + first_key: str, second_key: str, modification_time: str, hash_: str ) -> None: """Create or update a state.""" with DatabaseSession() as session: @@ -61,12 +62,12 @@ def _create_or_update_state( task=first_key, node=second_key, modification_time=modification_time, - file_hash=file_hash, + hash_=hash_, ) ) else: state_in_db.modification_time = modification_time - state_in_db.file_hash = file_hash + state_in_db.hash_ = hash_ session.commit() @@ -76,11 +77,14 @@ def update_states_in_database(session: Session, task_name: str) -> None: for name in node_and_neighbors(session.dag, task_name): node = session.dag.nodes[name].get("task") or session.dag.nodes[name]["node"] - state = node.state() - if isinstance(node, Task): + modification_time = node.state() hash_ = hashlib.sha256(node.path.read_bytes()).hexdigest() - else: + elif isinstance(node, FilePathNode): + modification_time = node.state() hash_ = "" + else: + modification_time = "" + hash_ = node.state() - _create_or_update_state(task_name, node.name, state, hash_) + _create_or_update_state(task_name, node.name, modification_time, hash_) diff --git a/src/_pytask/hookspecs.py b/src/_pytask/hookspecs.py index 011feb6e6..3eb56599c 100644 --- a/src/_pytask/hookspecs.py +++ b/src/_pytask/hookspecs.py @@ -13,6 +13,7 @@ import click import networkx import pluggy +from _pytask.models import NodeInfo if TYPE_CHECKING: @@ -194,7 +195,7 @@ def pytask_collect_task_teardown(session: Session, task: Task) -> None: @hookspec(firstresult=True) def pytask_collect_node( - session: Session, path: pathlib.Path, node: MetaNode + session: Session, path: pathlib.Path, node_info: NodeInfo ) -> MetaNode | None: """Collect a node which is a dependency or a product of a task.""" diff --git a/src/_pytask/models.py b/src/_pytask/models.py index 84a0d1d2e..9b5dc3368 100644 --- a/src/_pytask/models.py +++ b/src/_pytask/models.py @@ -2,6 +2,7 @@ from __future__ import annotations from typing import Any +from typing import NamedTuple from typing import TYPE_CHECKING from attrs import define @@ -23,3 +24,9 @@ class CollectionMetadata: """Contains the markers of the function.""" name: str | None = None """The name of the task function.""" + + +class NodeInfo(NamedTuple): + arg_name: str + path: tuple[str | int, ...] + value: Any diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index 1af06904b..b41c906a9 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -2,6 +2,7 @@ from __future__ import annotations import functools +import hashlib from abc import ABCMeta from abc import abstractmethod from pathlib import Path @@ -9,6 +10,7 @@ from typing import Callable from typing import TYPE_CHECKING +from _pytask.tree_util import PyTree from attrs import define from attrs import field @@ -53,9 +55,9 @@ class Task(MetaNode): """The name of the task.""" short_name: str | None = field(default=None, init=False) """The shortest uniquely identifiable name for task for display.""" - depends_on: dict[str, MetaNode] = field(factory=dict) + depends_on: PyTree[MetaNode] = field(factory=dict) """A list of dependencies of task.""" - produces: dict[str, MetaNode] = field(factory=dict) + produces: PyTree[MetaNode] = field(factory=dict) """A list of products of task.""" markers: list[Mark] = field(factory=list) """A list of markers attached to the task function.""" @@ -72,8 +74,10 @@ def __attrs_post_init__(self: Task) -> None: if self.short_name is None: self.short_name = self.name - def state(self) -> str | None: - if self.path.exists(): + def state(self, hash: bool = False) -> str | None: # noqa: A002 + if hash and self.path.exists(): + return hashlib.sha256(self.path.read_bytes()).hexdigest() + if not hash and self.path.exists(): return str(self.path.stat().st_mtime) return None @@ -91,17 +95,12 @@ def add_report_section(self, when: str, key: str, content: str) -> None: class FilePathNode(MetaNode): """The class for a node which is a path.""" - name: str - """str: Name of the node which makes it identifiable in the DAG.""" - value: Path - """Any: Value passed to the decorator which can be requested inside the function.""" - path: Path - """pathlib.Path: Path to the FilePathNode.""" - - def state(self) -> str | None: - if self.path.exists(): - return str(self.path.stat().st_mtime) - return None + name: str = "" + """Name of the node which makes it identifiable in the DAG.""" + value: Path | None = None + """Value passed to the decorator which can be requested inside the function.""" + path: Path | None = None + """Path to the FilePathNode.""" @classmethod @functools.lru_cache @@ -115,20 +114,43 @@ def from_path(cls, path: Path) -> FilePathNode: raise ValueError("FilePathNode must be instantiated from absolute path.") return cls(name=path.as_posix(), value=path, path=path) + def state(self) -> str | None: + """Calculate the state of the node. + + The state is given by the modification timestamp. + + """ + if self.path.exists(): + return str(self.path.stat().st_mtime) + return None + @define(kw_only=True) class PythonNode(MetaNode): """The class for a node which is a Python object.""" - value: Any - hash: bool = False # noqa: A003 name: str = "" - - def __attrs_post_init__(self) -> None: - if not self.name: - self.name = str(self.value) + """Name of the node.""" + value: Any | None = None + """Value of the node.""" + hash: bool = False # noqa: A003 + """Whether the value should be hashed to determine the state.""" def state(self) -> str | None: + """Calculate state of the node. + + If ``hash = False``, the function returns ``"0"``, a constant hash value, so the + :class:`PythonNode` is ignored when checking for a changed state of the task. + + If ``hash = True``, :func:`hash` is used for all types except strings. + + The hash for strings is calculated using hashlib because ``hash("asd")`` returns + a different value every invocation since the hash of strings is salted with a + random integer and it would confuse users. + + """ if self.hash: + if isinstance(self.value, str): + return str(hashlib.sha256(self.value.encode()).hexdigest()) return str(hash(self.value)) return str(0) diff --git a/src/_pytask/tree_util.py b/src/_pytask/tree_util.py index 65d17bdd6..beb9a0a61 100644 --- a/src/_pytask/tree_util.py +++ b/src/_pytask/tree_util.py @@ -1,18 +1,31 @@ +"""Contains code for tree utilities.""" from __future__ import annotations +import functools from pathlib import Path -import pybaum -from pybaum.tree_util import tree_just_flatten -from pybaum.tree_util import tree_just_yield -from pybaum.tree_util import tree_map +import optree +from optree import PyTree +from optree import tree_leaves as _optree_tree_leaves +from optree import tree_map as _optree_tree_map +from optree import tree_map_with_path as _optree_tree_map_with_path __all__ = [ - "tree_just_flatten", + "tree_leaves", "tree_map", - "tree_just_yield", + "tree_map_with_path", + "PyTree", "TREE_UTIL_LIB_DIRECTORY", ] -TREE_UTIL_LIB_DIRECTORY = Path(pybaum.__file__).parent +TREE_UTIL_LIB_DIRECTORY = Path(optree.__file__).parent + + +tree_leaves = functools.partial( + _optree_tree_leaves, none_is_leaf=True, namespace="pytask" +) +tree_map = functools.partial(_optree_tree_map, none_is_leaf=True, namespace="pytask") +tree_map_with_path = functools.partial( + _optree_tree_map_with_path, none_is_leaf=True, namespace="pytask" +) diff --git a/src/pytask/__init__.py b/src/pytask/__init__.py index d84ec0f16..1b5eca521 100644 --- a/src/pytask/__init__.py +++ b/src/pytask/__init__.py @@ -35,9 +35,11 @@ from _pytask.mark_utils import remove_marks from _pytask.mark_utils import set_marks from _pytask.models import CollectionMetadata +from _pytask.models import NodeInfo from _pytask.nodes import FilePathNode from _pytask.nodes import MetaNode from _pytask.nodes import Product +from _pytask.nodes import PythonNode from _pytask.nodes import Task from _pytask.outcomes import CollectionOutcome from _pytask.outcomes import count_outcomes @@ -87,11 +89,13 @@ "MarkDecorator", "MarkGenerator", "MetaNode", + "NodeInfo", "NodeNotCollectedError", "NodeNotFoundError", "Persisted", "Product", "PytaskError", + "PythonNode", "ResolvingDependenciesError", "Runtime", "Session", diff --git a/tests/test_collect.py b/tests/test_collect.py index 6728e16ee..03bb85466 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -8,6 +8,7 @@ import pytest from _pytask.collect import _find_shortest_uniquely_identifiable_name_for_tasks from _pytask.collect import pytask_collect_node +from _pytask.models import NodeInfo from pytask import cli from pytask import CollectionOutcome from pytask import ExitCode @@ -147,26 +148,26 @@ def test_collect_files_w_custom_file_name_pattern( @pytest.mark.unit() @pytest.mark.parametrize( - ("session", "path", "node", "expected"), + ("session", "path", "node_info", "expected"), [ pytest.param( Session({"check_casing_of_paths": False}, None), Path(), - Path.cwd() / "text.txt", + NodeInfo("", (), Path.cwd() / "text.txt"), Path.cwd() / "text.txt", id="test with absolute string path", ), pytest.param( Session({"check_casing_of_paths": False}, None), Path(), - 1, + NodeInfo("", (), 1), "1", id="test with python node", ), ], ) -def test_pytask_collect_node(session, path, node, expected): - result = pytask_collect_node(session, path, node) +def test_pytask_collect_node(session, path, node_info, expected): + result = pytask_collect_node(session, path, node_info) if result is None: assert result is expected else: @@ -185,7 +186,7 @@ def test_pytask_collect_node_raises_error_if_path_is_not_correctly_cased(tmp_pat collected_node = tmp_path / "TeXt.TxT" with pytest.raises(Exception, match="The provided path of"): - pytask_collect_node(session, task_path, collected_node) + pytask_collect_node(session, task_path, NodeInfo("", (), collected_node)) @pytest.mark.unit() @@ -202,7 +203,9 @@ def test_pytask_collect_node_does_not_raise_error_if_path_is_not_normalized( collected_node = tmp_path / collected_node with warnings.catch_warnings(record=True) as record: - result = pytask_collect_node(session, task_path, collected_node) + result = pytask_collect_node( + session, task_path, NodeInfo("", (), collected_node) + ) assert not record assert str(result.path) == str(real_node) diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index d7f3149e7..6ab330119 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -51,7 +51,6 @@ def task_example(): assert "out.txt>" in captured -@pytest.mark.xfail(reason="Only FilePathNodes are supported.") @pytest.mark.end_to_end() def test_collect_task_new_interface(runner, tmp_path): source = """ @@ -443,3 +442,79 @@ def test_task_name_is_shortened(runner, tmp_path): assert result.exit_code == ExitCode.OK assert "task_example.py::task_example" in result.output assert "a/b/task_example.py::task_example" not in result.output + + +@pytest.mark.end_to_end() +def test_python_node_is_collected(runner, tmp_path): + source = """ + from pytask import Product + from typing_extensions import Annotated + from pathlib import Path + + def task_example( + dependency: int = 1, path: Annotated[Path, Product] = Path("out.txt") + ): + ... + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, ["collect", tmp_path.as_posix(), "--nodes"]) + + assert result.exit_code == ExitCode.OK + captured = result.output.replace("\n", "").replace(" ", "") + assert "" in captured + assert "" in captured + assert "" in result.output + assert "Product" in captured + + +@pytest.mark.end_to_end() +def test_none_is_a_python_node(runner, tmp_path): + source = """ + from pytask import Product + from typing_extensions import Annotated + from pathlib import Path + + def task_example( + dependency = None, path: Annotated[Path, Product] = Path("out.txt") + ): + ... + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, ["collect", tmp_path.as_posix(), "--nodes"]) + + assert result.exit_code == ExitCode.OK + captured = result.output.replace("\n", "").replace(" ", "") + assert "" in captured + assert "" in captured + assert "" in result.output + assert "Product" in captured + + +@pytest.mark.end_to_end() +def test_python_nodes_are_aggregated_into_one(runner, tmp_path): + source = """ + from pytask import Product + from typing_extensions import Annotated + from pathlib import Path + + def task_example( + nested = {"a": (1, 2), 2: {"b": None}}, + path: Annotated[Path, Product] = Path("out.txt") + ): + ... + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, ["collect", tmp_path.as_posix(), "--nodes"]) + + assert result.exit_code == ExitCode.OK + captured = result.output.replace("\n", "").replace(" ", "") + assert "" in captured + assert "" in captured + assert "" in result.output + assert "Product" in captured diff --git a/tests/test_execute.py b/tests/test_execute.py index 4c0f2a7ca..a86dc1768 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -464,3 +464,83 @@ def task_example( assert len(session.tasks) == 1 task = session.tasks[0] assert "paths_to_file" in task.produces + + +@pytest.mark.end_to_end() +@pytest.mark.parametrize( + "definition", + [ + " = PythonNode(value=data['dependency'], hash=True)", + ": Annotated[Any, PythonNode(hash=True)] = data['dependency']", + ], +) +def test_task_with_hashed_python_node(runner, tmp_path, definition): + source = f""" + import json + from pathlib import Path + from pytask import Product, PythonNode + from typing import Any + from typing_extensions import Annotated + + data = json.loads(Path(__file__).parent.joinpath("data.json").read_text()) + + def task_example( + dependency{definition}, + path: Annotated[Path, Product] = Path("out.txt") + ) -> None: + path.write_text(dependency) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("data.json").write_text('{"dependency": "hello"}') + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").read_text() == "hello" + + tmp_path.joinpath("data.json").write_text('{"dependency": "world"}') + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").read_text() == "world" + + +@pytest.mark.end_to_end() +def test_error_with_multiple_dep_annotations(runner, tmp_path): + source = """ + from pathlib import Path + from typing_extensions import Annotated + from pytask import Product, PythonNode + from typing import Any + + def task_example( + dependency: Annotated[Any, PythonNode(), PythonNode()] = "hello", + path: Annotated[Path, Product] = Path("out.txt") + ) -> None: + path.write_text(dependency) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.COLLECTION_FAILED + assert "Parameter 'dependency'" in result.output + + +@pytest.mark.end_to_end() +def test_error_with_multiple_different_dep_annotations(runner, tmp_path): + source = """ + from pathlib import Path + from typing_extensions import Annotated + from pytask import Product, PythonNode, FilePathNode + from typing import Any + + def task_example( + dependency: Annotated[Any, PythonNode(), FilePathNode()] = "hello", + path: Annotated[Path, Product] = Path("out.txt") + ) -> None: + path.write_text(dependency) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.COLLECTION_FAILED + assert "Parameter 'dependency'" in result.output diff --git a/tests/test_tree_util.py b/tests/test_tree_util.py index be6212a47..ed43014ab 100644 --- a/tests/test_tree_util.py +++ b/tests/test_tree_util.py @@ -59,12 +59,12 @@ def test_profile_with_pytree(tmp_path, runner): source = """ import time import pytask - from _pytask.tree_util import tree_just_flatten + from _pytask.tree_util import tree_leaves @pytask.mark.produces([{"out_1": "out_1.txt"}, {"out_2": "out_2.txt"}]) def task_example(produces): time.sleep(2) - for p in tree_just_flatten(produces): + for p in tree_leaves(produces): p.write_text("There are nine billion bicycles in Beijing.") """ tmp_path.joinpath("task_example.py").write_text(textwrap.dedent(source)) diff --git a/tox.ini b/tox.ini index e6e8b0ce1..82a9d52bc 100644 --- a/tox.ini +++ b/tox.ini @@ -22,7 +22,7 @@ conda_deps = networkx >=2.4 pluggy sqlalchemy >=1.4.36 - pybaum >=0.1.1 + optree >=0.9 rich tomli >=1.0.0 From 8f875edcb07623f2a776e0f2c71881b7a7119ce2 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 16 Jul 2023 22:25:02 +0200 Subject: [PATCH 09/12] Add support for `NamedTuple` and attrs classes in `@pytask.mark.task(kwargs=...)`. (#397) --- docs/source/changes.md | 2 + src/_pytask/collect_utils.py | 129 +++++++++++++++++++++-------------- src/_pytask/parameters.py | 5 ++ src/_pytask/task_utils.py | 38 ++++++----- tests/test_collect.py | 39 +++++++++-- tests/test_live.py | 3 +- tests/test_task.py | 60 ++++++++++++++++ tests/test_task_utils.py | 32 +++++++++ 8 files changed, 234 insertions(+), 74 deletions(-) diff --git a/docs/source/changes.md b/docs/source/changes.md index 7998f5748..18c6c6c6d 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -17,6 +17,8 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`395` refactors all occurrences of pybaum to {mod}`_pytask.tree_util`. - {pull}`396` replaces pybaum with optree and adds paths to the name of {class}`pytask.PythonNode`'s allowing for better hashing. +- {class}`397` adds support for {class}`typing.NamedTuple` and attrs classes in + `@pytask.mark.task(kwargs=...)`. ## 0.3.2 - 2023-06-07 diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 696c4c055..19c4d924e 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -1,7 +1,6 @@ """This module provides utility functions for :mod:`_pytask.collect`.""" from __future__ import annotations -import inspect import itertools import uuid from pathlib import Path @@ -79,9 +78,15 @@ def parse_nodes( session: Session, path: Path, name: str, obj: Any, parser: Callable[..., Any] ) -> Any: """Parse nodes from object.""" + arg_name = parser.__name__ objects = _extract_nodes_from_function_markers(obj, parser) - nodes = _convert_objects_to_node_dictionary(objects, parser.__name__) - nodes = tree_map(lambda x: _collect_old_dependencies(session, path, name, x), nodes) + nodes = _convert_objects_to_node_dictionary(objects, arg_name) + nodes = tree_map( + lambda x: _collect_decorator_nodes( + session, path, name, NodeInfo(arg_name, (), x) + ), + nodes, + ) return nodes @@ -211,27 +216,61 @@ def _merge_dictionaries(list_of_dicts: list[dict[Any, Any]]) -> dict[Any, Any]: return out -def parse_dependencies_from_task_function( +_ERROR_MULTIPLE_DEPENDENCY_DEFINITIONS = """The task uses multiple ways to define \ +dependencies. Dependencies should be defined with either + +- '@pytask.mark.depends_on' and a 'depends_on' function argument. +- as default value for the function argument 'depends_on'. + +Use only one of the two ways! + +Hint: You do not need to use 'depends_on' since pytask v0.4. Every function argument \ +that is not a product is treated as a dependency. Read more about dependencies in the \ +documentation: https://tinyurl.com/yrezszr4. +""" + + +def parse_dependencies_from_task_function( # noqa: C901 session: Session, path: Path, name: str, obj: Any ) -> dict[str, Any]: """Parse dependencies from task function.""" + has_depends_on_decorator = False + has_depends_on_argument = False + dependencies = {} + if has_mark(obj, "depends_on"): + has_depends_on_decorator = True nodes = parse_nodes(session, path, name, obj, depends_on) - return {"depends_on": nodes} + dependencies["depends_on"] = nodes task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} signature_defaults = parse_keyword_arguments_from_signature_defaults(obj) kwargs = {**signature_defaults, **task_kwargs} kwargs.pop("produces", None) + # Parse products from task decorated with @task and that uses produces. + if "depends_on" in kwargs: + has_depends_on_argument = True + dependencies["depends_on"] = tree_map( + lambda x: _collect_decorator_nodes( + session, path, name, NodeInfo(arg_name="depends_on", path=(), value=x) + ), + kwargs["depends_on"], + ) + + if has_depends_on_decorator and has_depends_on_argument: + raise NodeNotCollectedError(_ERROR_MULTIPLE_DEPENDENCY_DEFINITIONS) + parameters_with_product_annot = _find_args_with_product_annotation(obj) parameters_with_node_annot = _find_args_with_node_annotation(obj) - dependencies = {} for parameter_name, value in kwargs.items(): if parameter_name in parameters_with_product_annot: continue + if parameter_name == "depends_on": + continue + if parameter_name in parameters_with_node_annot: def _evolve(x: Any) -> Any: @@ -316,18 +355,23 @@ def parse_products_from_task_function( """ has_produces_decorator = False - has_task_decorator = False - has_signature_default = False + has_produces_argument = False has_annotation = False out = {} + # Parse products from decorators. if has_mark(obj, "produces"): has_produces_decorator = True nodes = parse_nodes(session, path, name, obj, produces) out = {"produces": nodes} task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} - if "produces" in task_kwargs: + signature_defaults = parse_keyword_arguments_from_signature_defaults(obj) + kwargs = {**signature_defaults, **task_kwargs} + + # Parse products from task decorated with @task and that uses produces. + if "produces" in kwargs: + has_produces_argument = True collected_products = tree_map_with_path( lambda p, x: _collect_product( session, @@ -336,35 +380,15 @@ def parse_products_from_task_function( NodeInfo(arg_name="produces", path=p, value=x), is_string_allowed=True, ), - task_kwargs["produces"], + kwargs["produces"], ) out = {"produces": collected_products} - parameters = inspect.signature(obj).parameters - - if not has_mark(obj, "task") and "produces" in parameters: - parameter = parameters["produces"] - if parameter.default is not parameter.empty: - has_signature_default = True - # Use _collect_new_node to not collect strings. - collected_products = tree_map_with_path( - lambda p, x: _collect_product( - session, - path, - name, - NodeInfo(arg_name="produces", path=p, value=x), - is_string_allowed=False, - ), - parameter.default, - ) - out = {"produces": collected_products} - parameters_with_product_annot = _find_args_with_product_annotation(obj) if parameters_with_product_annot: has_annotation = True for parameter_name in parameters_with_product_annot: - parameter = parameters[parameter_name] - if parameter.default is not parameter.empty: + if parameter_name in kwargs: # Use _collect_new_node to not collect strings. collected_products = tree_map_with_path( lambda p, x: _collect_product( @@ -374,19 +398,12 @@ def parse_products_from_task_function( NodeInfo(parameter_name, p, x), # noqa: B023 is_string_allowed=False, ), - parameter.default, + kwargs[parameter_name], ) out = {parameter_name: collected_products} if ( - sum( - ( - has_produces_decorator, - has_task_decorator, - has_signature_default, - has_annotation, - ) - ) + sum((has_produces_decorator, has_produces_argument, has_annotation)) >= 2 # noqa: PLR2004 ): raise NodeNotCollectedError(_ERROR_MULTIPLE_PRODUCT_DEFINITIONS) @@ -412,8 +429,15 @@ def _find_args_with_product_annotation(func: Callable[..., Any]) -> list[str]: return args_with_product_annot -def _collect_old_dependencies( - session: Session, path: Path, name: str, node: str | Path +_ERROR_WRONG_TYPE_DECORATOR = """'@pytask.mark.depends_on', '@pytask.mark.produces', \ +and their function arguments can only accept values of type 'str' and 'pathlib.Path' \ +or the same values nested in tuples, lists, and dictionaries. Here, {node} has type \ +{node_type}. +""" + + +def _collect_decorator_nodes( + session: Session, path: Path, name: str, node_info: NodeInfo ) -> dict[str, MetaNode]: """Collect nodes for a task. @@ -423,22 +447,26 @@ def _collect_old_dependencies( If the node could not collected. """ + node = node_info.value + if not isinstance(node, (str, Path)): - raise ValueError( - "'@pytask.mark.depends_on' and '@pytask.mark.produces' can only accept " - "values of type 'str' and 'pathlib.Path' or the same values nested in " - f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}." + raise NodeNotCollectedError( + _ERROR_WRONG_TYPE_DECORATOR.format(node=node, node_type=type(node)) ) if isinstance(node, str): node = Path(node) + node_info = node_info._replace(value=node) collected_node = session.hook.pytask_collect_node( - session=session, path=path, node_info=NodeInfo("produces", (), node) + session=session, path=path, node_info=node_info ) if collected_node is None: + kind = {"depends_on": "dependency", "produces": "product"}.get( + node_info.arg_name + ) raise NodeNotCollectedError( - f"{node!r} cannot be parsed as a dependency for task {name!r} in {path!r}." + f"{node!r} cannot be parsed as a {kind} for task {name!r} in {path!r}." ) return collected_node @@ -458,7 +486,7 @@ def _collect_dependencies( node = node_info.value collected_node = session.hook.pytask_collect_node( - session=session, path=path, node_info=node_info, node=node + session=session, path=path, node_info=node_info ) if collected_node is None: raise NodeNotCollectedError( @@ -505,9 +533,10 @@ def _collect_product( if isinstance(node, str): node = Path(node) + node_info = node_info._replace(value=node) collected_node = session.hook.pytask_collect_node( - session=session, path=path, node_info=node_info, node=node + session=session, path=path, node_info=node_info ) if collected_node is None: raise NodeNotCollectedError( diff --git a/src/_pytask/parameters.py b/src/_pytask/parameters.py index d3fd83599..2ec31f53b 100644 --- a/src/_pytask/parameters.py +++ b/src/_pytask/parameters.py @@ -74,6 +74,11 @@ def _database_url_callback( ctx: Context, name: str, value: str | None # noqa: ARG001 ) -> URL: + """Check the url for the database.""" + # Since sqlalchemy v2.0.19, we need to shortcircuit here. + if value is None: + return None + try: return make_url(value) except ArgumentError: diff --git a/src/_pytask/task_utils.py b/src/_pytask/task_utils.py index 750605a51..cd5a1acef 100644 --- a/src/_pytask/task_utils.py +++ b/src/_pytask/task_utils.py @@ -7,6 +7,7 @@ from typing import Any from typing import Callable +import attrs from _pytask.mark import Mark from _pytask.models import CollectionMetadata from _pytask.shared import find_duplicates @@ -113,15 +114,6 @@ def parse_collected_tasks_with_task_marker( else: collected_tasks[name] = [i[1] for i in parsed_tasks if i[0] == name][0] - # TODO: Remove when parsing dependencies and products from all arguments is - # implemented. - for task in collected_tasks.values(): - meta = task.pytask_meta # type: ignore[attr-defined] - for marker_name in ("depends_on", "produces"): - if marker_name in meta.kwargs: - value = meta.kwargs.pop(marker_name) - meta.markers.append(Mark(marker_name, (value,), {})) - return collected_tasks @@ -143,24 +135,38 @@ def _parse_tasks_with_preliminary_names( def _parse_task(task: Callable[..., Any]) -> tuple[str, Callable[..., Any]]: """Parse a single task.""" - name = task.pytask_meta.name # type: ignore[attr-defined] - if name is None and task.__name__ == "_": + meta = task.pytask_meta # type: ignore[attr-defined] + + if meta.name is None and task.__name__ == "_": raise ValueError( "A task function either needs 'name' passed by the ``@pytask.mark.task`` " "decorator or the function name of the task function must not be '_'." ) - parsed_name = task.__name__ if name is None else name + parsed_name = task.__name__ if meta.name is None else meta.name + parsed_kwargs = _parse_task_kwargs(meta.kwargs) signature_kwargs = parse_keyword_arguments_from_signature_defaults(task) - task.pytask_meta.kwargs = { # type: ignore[attr-defined] - **task.pytask_meta.kwargs, # type: ignore[attr-defined] - **signature_kwargs, - } + meta.kwargs = {**signature_kwargs, **parsed_kwargs} return parsed_name, task +def _parse_task_kwargs(kwargs: Any) -> dict[str, Any]: + """Parse task kwargs.""" + if isinstance(kwargs, dict): + return kwargs + # Handle namedtuples. + if callable(getattr(kwargs, "_asdict", None)): + return kwargs._asdict() + if attrs.has(type(kwargs)): + return attrs.asdict(kwargs) + raise ValueError( + "'@pytask.mark.task(kwargs=...) needs to be a dictionary, namedtuple or an " + "instance of an attrs class." + ) + + def parse_keyword_arguments_from_signature_defaults( task: Callable[..., Any] ) -> dict[str, Any]: diff --git a/tests/test_collect.py b/tests/test_collect.py index 03bb85466..bb90f5d2b 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -8,6 +8,7 @@ import pytest from _pytask.collect import _find_shortest_uniquely_identifiable_name_for_tasks from _pytask.collect import pytask_collect_node +from _pytask.exceptions import NodeNotCollectedError from _pytask.models import NodeInfo from pytask import cli from pytask import CollectionOutcome @@ -53,7 +54,7 @@ def task_with_non_path_dependency(): assert session.exit_code == ExitCode.COLLECTION_FAILED assert session.collection_reports[0].outcome == CollectionOutcome.FAIL exc_info = session.collection_reports[0].exc_info - assert isinstance(exc_info[1], ValueError) + assert isinstance(exc_info[1], NodeNotCollectedError) assert "'@pytask.mark.depends_on'" in str(exc_info[1]) @@ -74,7 +75,7 @@ def task_with_non_path_dependency(): assert session.exit_code == ExitCode.COLLECTION_FAILED assert session.collection_reports[0].outcome == CollectionOutcome.FAIL exc_info = session.collection_reports[0].exc_info - assert isinstance(exc_info[1], ValueError) + assert isinstance(exc_info[1], NodeNotCollectedError) assert "'@pytask.mark.depends_on'" in str(exc_info[1]) @@ -326,7 +327,7 @@ def task_write_text(produces="out.txt"): @pytest.mark.end_to_end() -def test_collect_string_product_as_function_default_fails(tmp_path): +def test_collect_string_product_as_function_default(tmp_path): source = """ import pytask @@ -335,9 +336,8 @@ def task_write_text(produces="out.txt"): """ tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) session = main({"paths": tmp_path}) - report = session.collection_reports[0] - assert report.outcome == CollectionOutcome.FAIL - assert "If you use 'produces'" in str(report.exc_info[1]) + assert session.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").exists() @pytest.mark.end_to_end() @@ -362,3 +362,30 @@ def task_example( report = session.collection_reports[0] assert report.outcome == CollectionOutcome.FAIL assert "The task uses multiple ways" in str(report.exc_info[1]) + + +@pytest.mark.end_to_end() +def test_depends_on_cannot_mix_different_definitions(tmp_path): + source = """ + import pytask + from typing_extensions import Annotated + from pytask import Product + from pathlib import Path + + @pytask.mark.depends_on("input_1.txt") + def task_example( + depends_on: Path = "input_2.txt", + path: Annotated[Path, Product] = Path("out.txt") + ): + ... + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("input_1.txt").touch() + tmp_path.joinpath("input_2.txt").touch() + session = main({"paths": tmp_path}) + + assert session.exit_code == ExitCode.COLLECTION_FAILED + assert len(session.tasks) == 0 + report = session.collection_reports[0] + assert report.outcome == CollectionOutcome.FAIL + assert "The task uses multiple" in str(report.exc_info[1]) diff --git a/tests/test_live.py b/tests/test_live.py index abcfbacf7..4fe76273c 100644 --- a/tests/test_live.py +++ b/tests/test_live.py @@ -1,7 +1,6 @@ from __future__ import annotations import re -import sys import textwrap import pytest @@ -188,7 +187,7 @@ def test_live_execution_displays_subset_of_table(capsys, tmp_path, n_entries_in_ @pytest.mark.unit() -@pytest.mark.xfail(sys.platform == "darwin", reason="See #377.") +@pytest.mark.xfail(reason="See #377.") def test_live_execution_skips_do_not_crowd_out_displayed_tasks(capsys, tmp_path): path = tmp_path.joinpath("task_module.py") task = Task(base_name="task_example", path=path, function=lambda x: x) diff --git a/tests/test_task.py b/tests/test_task.py index ccbadfaef..b5d411fa8 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -451,3 +451,63 @@ def task_example(): pass tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.FAILED + + +@pytest.mark.end_to_end() +def test_task_receives_namedtuple(runner, tmp_path): + source = """ + import pytask + from typing_extensions import NamedTuple, Annotated + from pathlib import Path + from pytask import Product, PythonNode + + class Args(NamedTuple): + path_in: Path + arg: str + path_out: Path + + + args = Args(Path("input.txt"), "world!", Path("output.txt")) + + @pytask.mark.task(kwargs=args) + def task_example( + path_in: Path, + arg: Annotated[str, PythonNode(hash=True)], + path_out: Annotated[Path, Product] + ) -> None: + path_out.write_text(path_in.read_text() + " " + arg) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("input.txt").write_text("Hello") + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("output.txt").read_text() == "Hello world!" + + +@pytest.mark.end_to_end() +def test_task_kwargs_overwrite_default_arguments(runner, tmp_path): + source = """ + import pytask + from pytask import Product + from pathlib import Path + from typing_extensions import Annotated + + @pytask.mark.task(kwargs={ + "in_path": Path("in.txt"), "addition": "world!", "out_path": Path("out.txt") + }) + def task_example( + in_path: Path = Path("not_used_in.txt"), + addition: str = "planet!", + out_path: Annotated[Path, Product] = Path("not_used_out.txt"), + ) -> None: + out_path.write_text(in_path.read_text() + addition) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.txt").write_text("Hello ") + + result = runner.invoke(cli, [tmp_path.as_posix()]) + + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").read_text() == "Hello world!" + assert not tmp_path.joinpath("not_used_out.txt").exists() diff --git a/tests/test_task_utils.py b/tests/test_task_utils.py index 779cccd13..36b2b2086 100644 --- a/tests/test_task_utils.py +++ b/tests/test_task_utils.py @@ -1,7 +1,12 @@ from __future__ import annotations +from contextlib import ExitStack as does_not_raise # noqa: N813 +from typing import NamedTuple + import pytest from _pytask.task_utils import _arg_value_to_id_component +from _pytask.task_utils import _parse_task_kwargs +from attrs import define @pytest.mark.unit() @@ -24,3 +29,30 @@ def test_arg_value_to_id_component(arg_name, arg_value, i, id_func, expected): result = _arg_value_to_id_component(arg_name, arg_value, i, id_func) assert result == expected + + +class ExampleNT(NamedTuple): + a: int = 1 + + +@define +class ExampleAttrs: + b: str = "wonderful" + + +@pytest.mark.unit() +@pytest.mark.parametrize( + ("kwargs", "expectation", "expected"), + [ + ({"hello": 1}, does_not_raise(), {"hello": 1}), + (ExampleNT(), does_not_raise(), {"a": 1}), + (ExampleNT, pytest.raises(TypeError, match=r"(_asdict\(\) missing 1)"), None), + (ExampleAttrs(), does_not_raise(), {"b": "wonderful"}), + (ExampleAttrs, pytest.raises(ValueError, match="@pytask.mark.task"), None), + (1, pytest.raises(ValueError, match="@pytask.mark.task"), None), + ], +) +def test_parse_task_kwargs(kwargs, expectation, expected): + with expectation: + result = _parse_task_kwargs(kwargs) + assert result == expected From 1e66f75807034174b92c7f2d5b3735b92a4526d1 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Fri, 28 Jul 2023 15:15:28 +0200 Subject: [PATCH 10/12] Deprecate decorators for `depends_on` and `produces`. (#398) --- MANIFEST.in | 1 + pyproject.toml | 1 + src/_pytask/collect_utils.py | 50 ++++++++++++++++++++++++---------- src/_pytask/mark/__init__.py | 2 ++ src/_pytask/mark/__init__.pyi | 41 ++++++++++++++++++++++++++++ src/_pytask/mark/structures.py | 13 +++++++++ src/_pytask/profile.py | 2 +- tests/test_collect.py | 49 +++++++++++++++++++++++++++------ tests/test_mark.py | 21 ++++++++++++++ 9 files changed, 157 insertions(+), 23 deletions(-) create mode 100644 src/_pytask/mark/__init__.pyi diff --git a/MANIFEST.in b/MANIFEST.in index 44e6bda9c..84eeefdb0 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,6 +1,7 @@ include CITATION include LICENSE +recursive-include src *.pyi recursive-include src py.typed exclude .coveragerc diff --git a/pyproject.toml b/pyproject.toml index dc6b12a1e..d494d5b14 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -92,3 +92,4 @@ markers = [ "end_to_end: Flag for tests that cover the whole program.", ] norecursedirs = [".idea", ".tox"] +filterwarnings = ["ignore:'@pytask.mark.*. is deprecated:DeprecationWarning"] diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 19c4d924e..b333d2c66 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -3,6 +3,7 @@ import itertools import uuid +import warnings from pathlib import Path from typing import Any from typing import Callable @@ -21,6 +22,7 @@ from _pytask.nodes import PythonNode from _pytask.shared import find_duplicates from _pytask.task_utils import parse_keyword_arguments_from_signature_defaults +from _pytask.tree_util import PyTree from _pytask.tree_util import tree_leaves from _pytask.tree_util import tree_map from _pytask.tree_util import tree_map_with_path @@ -42,9 +44,7 @@ ] -def depends_on( - objects: Any | Iterable[Any] | dict[Any, Any] -) -> Any | Iterable[Any] | dict[Any, Any]: +def depends_on(objects: PyTree[Any]) -> PyTree[Any]: """Specify dependencies for a task. Parameters @@ -58,9 +58,7 @@ def depends_on( return objects -def produces( - objects: Any | Iterable[Any] | dict[Any, Any] -) -> Any | Iterable[Any] | dict[Any, Any]: +def produces(objects: PyTree[Any]) -> PyTree[Any]: """Specify products of a task. Parameters @@ -342,6 +340,18 @@ def _find_args_with_node_annotation(func: Callable[..., Any]) -> dict[str, MetaN Read more about products in the documentation: https://tinyurl.com/yrezszr4. """ +_WARNING_PRODUCES_AS_KWARG = """Using 'produces' as an argument name to specify \ +products is deprecated and won't be available in pytask v0.5. Instead, use the product \ +annotation, described in this tutorial: https://tinyurl.com/yrezszr4. + + from typing_extensions import Annotated + from pytask import Product + + def task_example(produces: Annotated[..., Product]): + ... + +""" + def parse_products_from_task_function( session: Session, path: Path, name: str, obj: Any @@ -369,8 +379,14 @@ def parse_products_from_task_function( signature_defaults = parse_keyword_arguments_from_signature_defaults(obj) kwargs = {**signature_defaults, **task_kwargs} + parameters_with_product_annot = _find_args_with_product_annotation(obj) + # Parse products from task decorated with @task and that uses produces. if "produces" in kwargs: + if "produces" not in parameters_with_product_annot: + warnings.warn( + _WARNING_PRODUCES_AS_KWARG, category=FutureWarning, stacklevel=1 + ) has_produces_argument = True collected_products = tree_map_with_path( lambda p, x: _collect_product( @@ -384,7 +400,6 @@ def parse_products_from_task_function( ) out = {"produces": collected_products} - parameters_with_product_annot = _find_args_with_product_annotation(obj) if parameters_with_product_annot: has_annotation = True for parameter_name in parameters_with_product_annot: @@ -436,6 +451,11 @@ def _find_args_with_product_annotation(func: Callable[..., Any]) -> list[str]: """ +_WARNING_STRING_DEPRECATED = """Using strings to specify a {kind} is deprecated. Pass \ +a 'pathlib.Path' instead with 'Path("{node}")'. +""" + + def _collect_decorator_nodes( session: Session, path: Path, name: str, node_info: NodeInfo ) -> dict[str, MetaNode]: @@ -448,6 +468,7 @@ def _collect_decorator_nodes( """ node = node_info.value + kind = {"depends_on": "dependency", "produces": "product"}.get(node_info.arg_name) if not isinstance(node, (str, Path)): raise NodeNotCollectedError( @@ -455,6 +476,11 @@ def _collect_decorator_nodes( ) if isinstance(node, str): + warnings.warn( + _WARNING_STRING_DEPRECATED.format(kind=kind, node=node), + category=FutureWarning, + stacklevel=1, + ) node = Path(node) node_info = node_info._replace(value=node) @@ -462,9 +488,6 @@ def _collect_decorator_nodes( session=session, path=path, node_info=node_info ) if collected_node is None: - kind = {"depends_on": "dependency", "produces": "product"}.get( - node_info.arg_name - ) raise NodeNotCollectedError( f"{node!r} cannot be parsed as a {kind} for task {name!r} in {path!r}." ) @@ -525,10 +548,9 @@ def _collect_product( # The parameter defaults only support Path objects. if not isinstance(node, Path) and not is_string_allowed: raise ValueError( - "If you use 'produces' as a function argument of a task and pass values as " - "function defaults, it can only accept values of type 'pathlib.Path' or " - "the same value nested in tuples, lists, and dictionaries. Here, " - f"{node!r} has type {type(node)}." + "If you declare products with 'Annotated[..., Product]', only values of " + "type 'pathlib.Path' optionally nested in tuples, lists, and " + f"dictionaries are allowed. Here, {node!r} has type {type(node)}." ) if isinstance(node, str): diff --git a/src/_pytask/mark/__init__.py b/src/_pytask/mark/__init__.py index 25db802b4..83a603291 100644 --- a/src/_pytask/mark/__init__.py +++ b/src/_pytask/mark/__init__.py @@ -39,6 +39,8 @@ "MarkDecorator", "MarkGenerator", "ParseError", + "select_by_keyword", + "select_by_mark", ] diff --git a/src/_pytask/mark/__init__.pyi b/src/_pytask/mark/__init__.pyi new file mode 100644 index 000000000..eca5e6e3b --- /dev/null +++ b/src/_pytask/mark/__init__.pyi @@ -0,0 +1,41 @@ +from pathlib import Path +from typing_extensions import deprecated +from _pytask.mark.expression import Expression +from _pytask.mark.expression import ParseError +from _pytask.mark.structures import Mark +from _pytask.mark.structures import MarkDecorator +from _pytask.mark.structures import MarkGenerator +from _pytask.tree_util import PyTree + +from _pytask.session import Session +import networkx as nx + +def select_by_keyword(session: Session, dag: nx.DiGraph) -> set[str]: ... +def select_by_mark(session: Session, dag: nx.DiGraph) -> set[str]: ... + +class MARK_GEN: # noqa: N801 + @deprecated( + "'@pytask.mark.produces' is deprecated starting pytask v0.4.0 and will be removed in v0.5.0. To upgrade your project to the new syntax, read the tutorial on product and dependencies: https://tinyurl.com/yrezszr4.", # noqa: E501 + category=DeprecationWarning, + stacklevel=1, + ) + @staticmethod + def produces(objects: PyTree[str | Path]) -> None: ... + @deprecated( + "'@pytask.mark.depends_on' is deprecated starting pytask v0.4.0 and will be removed in v0.5.0. To upgrade your project to the new syntax, read the tutorial on product and dependencies: https://tinyurl.com/yrezszr4.", # noqa: E501 + category=DeprecationWarning, + stacklevel=1, + ) + @staticmethod + def depends_on(objects: PyTree[str | Path]) -> None: ... + +__all__ = [ + "Expression", + "MARK_GEN", + "Mark", + "MarkDecorator", + "MarkGenerator", + "ParseError", + "select_by_keyword", + "select_by_mark", +] diff --git a/src/_pytask/mark/structures.py b/src/_pytask/mark/structures.py index 4dae51e67..62109f495 100644 --- a/src/_pytask/mark/structures.py +++ b/src/_pytask/mark/structures.py @@ -167,6 +167,12 @@ def store_mark(obj: Callable[..., Any], mark: Mark) -> None: ) +_DEPRECATION_DECORATOR = """'@pytask.mark.{}' is deprecated starting pytask \ +v0.4.0 and will be removed in v0.5.0. To upgrade your project to the new syntax, read \ +the tutorial on product and dependencies: https://tinyurl.com/yrezszr4. +""" + + class MarkGenerator: """Factory for :class:`MarkDecorator` objects. @@ -191,6 +197,13 @@ def __getattr__(self, name: str) -> MarkDecorator | Any: if name[0] == "_": raise AttributeError("Marker name must NOT start with underscore") + if name in ("depends_on", "produces"): + warnings.warn( + _DEPRECATION_DECORATOR.format(name), + category=DeprecationWarning, + stacklevel=1, + ) + # If the name is not in the set of known marks after updating, # then it really is time to issue a warning or an error. if self.config is not None and name not in self.config["markers"]: diff --git a/src/_pytask/profile.py b/src/_pytask/profile.py index faeb1e03a..e0b29d3bf 100644 --- a/src/_pytask/profile.py +++ b/src/_pytask/profile.py @@ -47,7 +47,7 @@ class _ExportFormats(enum.Enum): CSV = "csv" -class Runtime(BaseTable): # type: ignore[valid-type, misc] +class Runtime(BaseTable): """Record of runtimes of tasks.""" __tablename__ = "runtime" diff --git a/tests/test_collect.py b/tests/test_collect.py index bb90f5d2b..db951b15e 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -312,7 +312,7 @@ def task_my_task(): @pytest.mark.end_to_end() -def test_collect_string_product_with_task_decorator(tmp_path): +def test_collect_string_product_with_task_decorator(runner, tmp_path): source = """ import pytask @@ -321,25 +321,38 @@ def task_write_text(produces="out.txt"): produces.touch() """ tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - session = main({"paths": tmp_path}) - assert session.exit_code == ExitCode.OK + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK assert tmp_path.joinpath("out.txt").exists() @pytest.mark.end_to_end() -def test_collect_string_product_as_function_default(tmp_path): +def test_collect_string_product_as_function_default(runner, tmp_path): source = """ - import pytask - def task_write_text(produces="out.txt"): produces.touch() """ tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) - session = main({"paths": tmp_path}) - assert session.exit_code == ExitCode.OK + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK assert tmp_path.joinpath("out.txt").exists() +@pytest.mark.end_to_end() +def test_collect_string_product_raises_error_with_annotation(runner, tmp_path): + source = """ + from pytask import Product + from typing_extensions import Annotated + + def task_write_text(out: Annotated[str, Product] = "out.txt") -> None: + out.touch() + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.COLLECTION_FAILED + assert "If you declare products with 'Annotated[..., Product]'" in result.output + + @pytest.mark.end_to_end() def test_product_cannot_mix_different_product_types(tmp_path): source = """ @@ -389,3 +402,23 @@ def task_example( report = session.collection_reports[0] assert report.outcome == CollectionOutcome.FAIL assert "The task uses multiple" in str(report.exc_info[1]) + + +@pytest.mark.end_to_end() +def test_deprecation_warning_for_strings_in_depends_on(runner, tmp_path): + source = """ + import pytask + from pathlib import Path + + @pytask.mark.depends_on("in.txt") + @pytask.mark.produces("out.txt") + def task_write_text(depends_on, produces): + produces.touch() + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.txt").touch() + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert "FutureWarning" in result.output + assert "Using strings to specify a dependency" in result.output + assert "Using strings to specify a product" in result.output diff --git a/tests/test_mark.py b/tests/test_mark.py index 32d468b1a..422b7bf09 100644 --- a/tests/test_mark.py +++ b/tests/test_mark.py @@ -1,5 +1,6 @@ from __future__ import annotations +import subprocess import sys import textwrap @@ -354,3 +355,23 @@ def task_example(): assert result.exit_code == ExitCode.OK assert "1 Succeeded" in result.output assert "Warnings" in result.output + + +@pytest.mark.end_to_end() +def test_deprecation_warnings_for_decorators(tmp_path): + source = """ + import pytask + + @pytask.mark.depends_on("in.txt") + @pytask.mark.produces("out.txt") + def task_write_text(depends_on, produces): + ... + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.txt").touch() + + result = subprocess.run( + ("pytest", tmp_path.joinpath("task_module.py").as_posix()), capture_output=True + ) + assert b"DeprecationWarning: '@pytask.mark.depends_on'" in result.stdout + assert b"DeprecationWarning: '@pytask.mark.produces'" in result.stdout From 37d244efeb85ca8d32f6ce2b104c47b62aa38792 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 30 Jul 2023 20:49:56 +0200 Subject: [PATCH 11/12] Use protocols instead of ABCs. (#402) --- docs/source/changes.md | 6 +- docs/source/reference_guides/api.md | 2 +- src/_pytask/collect.py | 14 ++-- src/_pytask/collect_command.py | 25 +++--- src/_pytask/collect_utils.py | 67 +++++++++-------- src/_pytask/dag.py | 11 +-- src/_pytask/database_utils.py | 4 +- src/_pytask/execute.py | 6 +- src/_pytask/hookspecs.py | 5 +- src/_pytask/node_protocols.py | 33 ++++++++ src/_pytask/nodes.py | 53 +++++++------ src/_pytask/profile.py | 4 +- src/_pytask/report.py | 2 +- src/_pytask/shared.py | 11 +-- src/pytask/__init__.py | 6 +- tests/test_collect.py | 21 ++++++ tests/test_collect_command.py | 113 ++++++++++++++++++++++++---- tests/test_console.py | 13 +--- tests/test_dag.py | 9 +-- tests/test_execute.py | 4 +- tests/test_node_protocols.py | 75 ++++++++++++++++++ tests/test_nodes.py | 8 +- 22 files changed, 354 insertions(+), 138 deletions(-) create mode 100644 src/_pytask/node_protocols.py create mode 100644 tests/test_node_protocols.py diff --git a/docs/source/changes.md b/docs/source/changes.md index 18c6c6c6d..7a535918c 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -17,8 +17,12 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`395` refactors all occurrences of pybaum to {mod}`_pytask.tree_util`. - {pull}`396` replaces pybaum with optree and adds paths to the name of {class}`pytask.PythonNode`'s allowing for better hashing. -- {class}`397` adds support for {class}`typing.NamedTuple` and attrs classes in +- {pull}`397` adds support for {class}`typing.NamedTuple` and attrs classes in `@pytask.mark.task(kwargs=...)`. +- {pull}`398` deprecates the decorators `@pytask.mark.depends_on` and + `@pytask.mark.produces`. +- {pull}`402` replaces ABCs with protocols allowing for more flexibility for users + implementing their own nodes. ## 0.3.2 - 2023-06-07 diff --git a/docs/source/reference_guides/api.md b/docs/source/reference_guides/api.md index 2227723db..bfd922e29 100644 --- a/docs/source/reference_guides/api.md +++ b/docs/source/reference_guides/api.md @@ -246,7 +246,7 @@ from {class}`pytask.MetaNode`. Then, different kinds of nodes can be implemented. ```{eval-rst} -.. autoclass:: pytask.FilePathNode +.. autoclass:: pytask.PathNode :members: ``` diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index d4d135c96..26db423fd 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -21,8 +21,8 @@ from _pytask.exceptions import CollectionError from _pytask.mark_utils import has_mark from _pytask.models import NodeInfo -from _pytask.nodes import FilePathNode -from _pytask.nodes import MetaNode +from _pytask.node_protocols import Node +from _pytask.nodes import PathNode from _pytask.nodes import PythonNode from _pytask.nodes import Task from _pytask.outcomes import CollectionOutcome @@ -95,7 +95,7 @@ def pytask_collect_file_protocol( ) flat_reports = list(itertools.chain.from_iterable(new_reports)) except Exception: # noqa: BLE001 - node = FilePathNode.from_path(path) + node = PathNode.from_path(path) flat_reports = [ CollectionReport.from_exception( outcome=CollectionOutcome.FAIL, node=node, exc_info=sys.exc_info() @@ -204,8 +204,8 @@ def pytask_collect_task( @hookimpl(trylast=True) -def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> MetaNode: - """Collect a node of a task as a :class:`pytask.nodes.FilePathNode`. +def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> Node: + """Collect a node of a task as a :class:`pytask.nodes.PathNode`. Strings are assumed to be paths. This might be a strict assumption, but since this hook is executed at last and possible errors will be shown, it seems reasonable and @@ -223,7 +223,7 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> Me node.name = node_info.arg_name + suffix return node - if isinstance(node, MetaNode): + if isinstance(node, Node): return node if isinstance(node, Path): @@ -243,7 +243,7 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> Me if str(node) != str(case_sensitive_path): raise ValueError(_TEMPLATE_ERROR.format(node, case_sensitive_path)) - return FilePathNode.from_path(node) + return PathNode.from_path(node) suffix = "-" + "-".join(map(str, node_info.path)) if node_info.path else "" node_name = node_info.arg_name + suffix diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index e615a8ea9..a8b3fcf75 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -20,7 +20,7 @@ from _pytask.exceptions import ResolvingDependenciesError from _pytask.mark import select_by_keyword from _pytask.mark import select_by_mark -from _pytask.nodes import FilePathNode +from _pytask.node_protocols import PPathNode from _pytask.outcomes import ExitCode from _pytask.path import find_common_ancestor from _pytask.path import relative_to @@ -125,9 +125,7 @@ def _find_common_ancestor_of_all_nodes( all_paths.append(task.path) if show_nodes: all_paths.extend( - x.path - for x in tree_leaves(task.depends_on) - if isinstance(x, FilePathNode) + x.path for x in tree_leaves(task.depends_on) if isinstance(x, PPathNode) ) all_paths.extend(x.path for x in tree_leaves(task.produces)) @@ -202,24 +200,29 @@ def _print_collected_tasks( ) if show_nodes: - file_path_nodes = list(tree_leaves(task.depends_on)) - sorted_nodes = sorted(file_path_nodes, key=lambda x: x.name) + nodes = list(tree_leaves(task.depends_on)) + sorted_nodes = sorted(nodes, key=lambda x: x.name) for node in sorted_nodes: - if isinstance(node, FilePathNode): - reduced_node_name = relative_to(node.path, common_ancestor) + if isinstance(node, PPathNode): + if node.path.as_posix() in node.name: + reduced_node_name = str( + relative_to(node.path, common_ancestor) + ) + else: + reduced_node_name = node.name url_style = create_url_style_for_path( node.path, editor_url_scheme ) - text = Text(str(reduced_node_name), style=url_style) + text = Text(reduced_node_name, style=url_style) else: text = node.name task_branch.add(Text.assemble(FILE_ICON, "")) for node in sorted(tree_leaves(task.produces), key=lambda x: x.path): - reduced_node_name = relative_to(node.path, common_ancestor) + reduced_node_name = str(relative_to(node.path, common_ancestor)) url_style = create_url_style_for_path(node.path, editor_url_scheme) - text = Text(str(reduced_node_name), style=url_style) + text = Text(reduced_node_name, style=url_style) task_branch.add(Text.assemble(FILE_ICON, "")) console.print(tree) diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index b333d2c66..a5e6e507c 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -1,6 +1,7 @@ """This module provides utility functions for :mod:`_pytask.collect`.""" from __future__ import annotations +import functools import itertools import uuid import warnings @@ -11,13 +12,13 @@ from typing import Iterable from typing import TYPE_CHECKING -import attrs from _pytask._inspect import get_annotations from _pytask.exceptions import NodeNotCollectedError from _pytask.mark_utils import has_mark from _pytask.mark_utils import remove_marks from _pytask.models import NodeInfo -from _pytask.nodes import MetaNode +from _pytask.node_protocols import Node +from _pytask.node_protocols import PPathNode from _pytask.nodes import ProductType from _pytask.nodes import PythonNode from _pytask.shared import find_duplicates @@ -80,7 +81,7 @@ def parse_nodes( objects = _extract_nodes_from_function_markers(obj, parser) nodes = _convert_objects_to_node_dictionary(objects, arg_name) nodes = tree_map( - lambda x: _collect_decorator_nodes( + lambda x: _collect_decorator_node( session, path, name, NodeInfo(arg_name, (), x) ), nodes, @@ -228,7 +229,7 @@ def _merge_dictionaries(list_of_dicts: list[dict[Any, Any]]) -> dict[Any, Any]: """ -def parse_dependencies_from_task_function( # noqa: C901 +def parse_dependencies_from_task_function( session: Session, path: Path, name: str, obj: Any ) -> dict[str, Any]: """Parse dependencies from task function.""" @@ -250,7 +251,7 @@ def parse_dependencies_from_task_function( # noqa: C901 if "depends_on" in kwargs: has_depends_on_argument = True dependencies["depends_on"] = tree_map( - lambda x: _collect_decorator_nodes( + lambda x: _collect_decorator_node( session, path, name, NodeInfo(arg_name="depends_on", path=(), value=x) ), kwargs["depends_on"], @@ -269,23 +270,17 @@ def parse_dependencies_from_task_function( # noqa: C901 if parameter_name == "depends_on": continue - if parameter_name in parameters_with_node_annot: - - def _evolve(x: Any) -> Any: - instance = parameters_with_node_annot[parameter_name] # noqa: B023 - return attrs.evolve(instance, value=x) # type: ignore[misc] - - else: - - def _evolve(x: Any) -> Any: - return x + partialed_evolve = functools.partial( + _evolve_instance, + instance_from_annot=parameters_with_node_annot.get(parameter_name), + ) nodes = tree_map_with_path( - lambda p, x: _collect_dependencies( + lambda p, x: _collect_dependency( session, path, name, - NodeInfo(parameter_name, p, _evolve(x)), # noqa: B023 + NodeInfo(parameter_name, p, partialed_evolve(x)), # noqa: B023 ), value, ) @@ -295,14 +290,14 @@ def _evolve(x: Any) -> Any: are_all_nodes_python_nodes_without_hash = all( isinstance(x, PythonNode) and not x.hash for x in tree_leaves(nodes) ) - if are_all_nodes_python_nodes_without_hash: + if not isinstance(nodes, Node) and are_all_nodes_python_nodes_without_hash: dependencies[parameter_name] = PythonNode(value=value, name=parameter_name) else: dependencies[parameter_name] = nodes return dependencies -def _find_args_with_node_annotation(func: Callable[..., Any]) -> dict[str, MetaNode]: +def _find_args_with_node_annotation(func: Callable[..., Any]) -> dict[str, Node]: """Find args with node annotations.""" annotations = get_annotations(func, eval_str=True) metas = { @@ -314,9 +309,7 @@ def _find_args_with_node_annotation(func: Callable[..., Any]) -> dict[str, MetaN args_with_node_annotation = {} for name, meta in metas.items(): annot = [ - i - for i in meta - if not isinstance(i, ProductType) and isinstance(i, MetaNode) + i for i in meta if not isinstance(i, ProductType) and isinstance(i, Node) ] if len(annot) >= 2: # noqa: PLR2004 raise ValueError( @@ -380,6 +373,7 @@ def parse_products_from_task_function( kwargs = {**signature_defaults, **task_kwargs} parameters_with_product_annot = _find_args_with_product_annotation(obj) + parameters_with_node_annot = _find_args_with_node_annotation(obj) # Parse products from task decorated with @task and that uses produces. if "produces" in kwargs: @@ -404,13 +398,17 @@ def parse_products_from_task_function( has_annotation = True for parameter_name in parameters_with_product_annot: if parameter_name in kwargs: - # Use _collect_new_node to not collect strings. + partialed_evolve = functools.partial( + _evolve_instance, + instance_from_annot=parameters_with_node_annot.get(parameter_name), + ) + collected_products = tree_map_with_path( lambda p, x: _collect_product( session, path, name, - NodeInfo(parameter_name, p, x), # noqa: B023 + NodeInfo(parameter_name, p, partialed_evolve(x)), # noqa: B023 is_string_allowed=False, ), kwargs[parameter_name], @@ -456,9 +454,9 @@ def _find_args_with_product_annotation(func: Callable[..., Any]) -> list[str]: """ -def _collect_decorator_nodes( +def _collect_decorator_node( session: Session, path: Path, name: str, node_info: NodeInfo -) -> dict[str, MetaNode]: +) -> Node: """Collect nodes for a task. Raises @@ -495,9 +493,9 @@ def _collect_decorator_nodes( return collected_node -def _collect_dependencies( +def _collect_dependency( session: Session, path: Path, name: str, node_info: NodeInfo -) -> dict[str, MetaNode]: +) -> Node: """Collect nodes for a task. Raises @@ -525,7 +523,7 @@ def _collect_product( task_name: str, node_info: NodeInfo, is_string_allowed: bool = False, -) -> dict[str, MetaNode]: +) -> Node: """Collect products for a task. Defining products with strings is only allowed when using the decorator. Parameter @@ -546,7 +544,7 @@ def _collect_product( f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}." ) # The parameter defaults only support Path objects. - if not isinstance(node, Path) and not is_string_allowed: + if not isinstance(node, (Path, PPathNode)) and not is_string_allowed: raise ValueError( "If you declare products with 'Annotated[..., Product]', only values of " "type 'pathlib.Path' optionally nested in tuples, lists, and " @@ -566,3 +564,12 @@ def _collect_product( ) return collected_node + + +def _evolve_instance(x: Any, instance_from_annot: Node | None) -> Any: + """Evolve a value to a node if it is given by annotations.""" + if not instance_from_annot: + return x + + instance_from_annot.value = x + return instance_from_annot diff --git a/src/_pytask/dag.py b/src/_pytask/dag.py index bee58b988..050aae214 100644 --- a/src/_pytask/dag.py +++ b/src/_pytask/dag.py @@ -21,8 +21,9 @@ from _pytask.mark import Mark from _pytask.mark_utils import get_marks from _pytask.mark_utils import has_mark -from _pytask.nodes import FilePathNode -from _pytask.nodes import MetaNode +from _pytask.node_protocols import MetaNode +from _pytask.node_protocols import Node +from _pytask.node_protocols import PPathNode from _pytask.nodes import Task from _pytask.path import find_common_ancestor_of_nodes from _pytask.report import DagReport @@ -140,13 +141,13 @@ def pytask_dag_has_node_changed(node: MetaNode, task_name: str) -> bool: if db_state is None: return True - if isinstance(node, (FilePathNode, Task)): + if isinstance(node, (PPathNode, Task)): # If the modification times match, the node has not been changed. if node_state == db_state.modification_time: return False # If the modification time changed, quickly return for non-tasks. - if isinstance(node, FilePathNode): + if not isinstance(node, Task): return True # When modification times changed, we are still comparing the hash of the file @@ -238,7 +239,7 @@ def _check_if_root_nodes_are_available(dag: nx.DiGraph) -> None: def _check_if_tasks_are_skipped( - node: MetaNode, dag: nx.DiGraph, is_task_skipped: dict[str, bool] + node: Node, dag: nx.DiGraph, is_task_skipped: dict[str, bool] ) -> tuple[bool, dict[str, bool]]: """Check for a given node whether it is only used by skipped tasks.""" are_all_tasks_skipped = [] diff --git a/src/_pytask/database_utils.py b/src/_pytask/database_utils.py index 05a09de1b..19f5724b7 100644 --- a/src/_pytask/database_utils.py +++ b/src/_pytask/database_utils.py @@ -4,7 +4,7 @@ import hashlib from _pytask.dag_utils import node_and_neighbors -from _pytask.nodes import FilePathNode +from _pytask.node_protocols import PPathNode from _pytask.nodes import Task from _pytask.session import Session from sqlalchemy import Column @@ -80,7 +80,7 @@ def update_states_in_database(session: Session, task_name: str) -> None: if isinstance(node, Task): modification_time = node.state() hash_ = hashlib.sha256(node.path.read_bytes()).hexdigest() - elif isinstance(node, FilePathNode): + elif isinstance(node, PPathNode): modification_time = node.state() hash_ = "" else: diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index bcc3fb418..a4d703117 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -21,7 +21,7 @@ from _pytask.exceptions import NodeNotFoundError from _pytask.mark import Mark from _pytask.mark_utils import has_mark -from _pytask.nodes import FilePathNode +from _pytask.node_protocols import PPathNode from _pytask.nodes import Task from _pytask.outcomes import count_outcomes from _pytask.outcomes import Exit @@ -129,7 +129,7 @@ def pytask_execute_task_setup(session: Session, task: Task) -> None: # method for the node classes. for product in session.dag.successors(task.name): node = session.dag.nodes[product]["node"] - if isinstance(node, FilePathNode): + if isinstance(node, PPathNode): node.path.parent.mkdir(parents=True, exist_ok=True) would_be_executed = has_mark(task, "would_be_executed") @@ -159,7 +159,7 @@ def pytask_execute_task(session: Session, task: Task) -> bool: @hookimpl def pytask_execute_task_teardown(session: Session, task: Task) -> None: - """Check if :class:`_pytask.nodes.FilePathNode` are produced by a task.""" + """Check if :class:`_pytask.nodes.PathNode` are produced by a task.""" missing_nodes = [] for product in session.dag.successors(task.name): node = session.dag.nodes[product]["node"] diff --git a/src/_pytask/hookspecs.py b/src/_pytask/hookspecs.py index 3eb56599c..dfa83f92f 100644 --- a/src/_pytask/hookspecs.py +++ b/src/_pytask/hookspecs.py @@ -14,11 +14,12 @@ import networkx import pluggy from _pytask.models import NodeInfo +from _pytask.node_protocols import MetaNode +from _pytask.node_protocols import Node if TYPE_CHECKING: from _pytask.session import Session - from _pytask.nodes import MetaNode from _pytask.nodes import Task from _pytask.outcomes import CollectionOutcome from _pytask.outcomes import TaskOutcome @@ -196,7 +197,7 @@ def pytask_collect_task_teardown(session: Session, task: Task) -> None: @hookspec(firstresult=True) def pytask_collect_node( session: Session, path: pathlib.Path, node_info: NodeInfo -) -> MetaNode | None: +) -> Node | None: """Collect a node which is a dependency or a product of a task.""" diff --git a/src/_pytask/node_protocols.py b/src/_pytask/node_protocols.py new file mode 100644 index 000000000..f3062cb4c --- /dev/null +++ b/src/_pytask/node_protocols.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +from abc import abstractmethod +from pathlib import Path +from typing import Any +from typing import Protocol +from typing import runtime_checkable + + +@runtime_checkable +class MetaNode(Protocol): + """Protocol for an intersection between nodes and tasks.""" + + name: str | None + """The name of node that must be unique.""" + + @abstractmethod + def state(self) -> Any: + ... + + +@runtime_checkable +class Node(MetaNode, Protocol): + """Protocol for nodes.""" + + value: Any + + +@runtime_checkable +class PPathNode(Node, Protocol): + """Nodes with paths.""" + + path: Path diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index b41c906a9..b86503109 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -3,13 +3,13 @@ import functools import hashlib -from abc import ABCMeta -from abc import abstractmethod from pathlib import Path from typing import Any from typing import Callable from typing import TYPE_CHECKING +from _pytask.node_protocols import MetaNode +from _pytask.node_protocols import Node from _pytask.tree_util import PyTree from attrs import define from attrs import field @@ -19,7 +19,7 @@ from _pytask.mark import Mark -__all__ = ["FilePathNode", "MetaNode", "Product", "Task"] +__all__ = ["PathNode", "Product", "Task"] @define(frozen=True) @@ -30,17 +30,6 @@ class ProductType: Product = ProductType() -class MetaNode(metaclass=ABCMeta): - """Meta class for nodes.""" - - name: str - """str: The name of node that must be unique.""" - - @abstractmethod - def state(self) -> Any: - ... - - @define(kw_only=True) class Task(MetaNode): """The class for tasks which are Python functions.""" @@ -55,9 +44,9 @@ class Task(MetaNode): """The name of the task.""" short_name: str | None = field(default=None, init=False) """The shortest uniquely identifiable name for task for display.""" - depends_on: PyTree[MetaNode] = field(factory=dict) + depends_on: PyTree[Node] = field(factory=dict) """A list of dependencies of task.""" - produces: PyTree[MetaNode] = field(factory=dict) + produces: PyTree[Node] = field(factory=dict) """A list of products of task.""" markers: list[Mark] = field(factory=list) """A list of markers attached to the task function.""" @@ -92,27 +81,41 @@ def add_report_section(self, when: str, key: str, content: str) -> None: @define(kw_only=True) -class FilePathNode(MetaNode): +class PathNode(Node): """The class for a node which is a path.""" name: str = "" """Name of the node which makes it identifiable in the DAG.""" - value: Path | None = None + _value: Path | None = None """Value passed to the decorator which can be requested inside the function.""" - path: Path | None = None - """Path to the FilePathNode.""" + + @property + def path(self) -> Path: + return self.value + + @property + def value(self) -> Path: + return self._value + + @value.setter + def value(self, value: Path) -> None: + if not isinstance(value, Path): + raise TypeError("'value' must be a 'pathlib.Path'.") + if not self.name: + self.name = value.as_posix() + self._value = value @classmethod @functools.lru_cache - def from_path(cls, path: Path) -> FilePathNode: + def from_path(cls, path: Path) -> PathNode: """Instantiate class from path to file. The `lru_cache` decorator ensures that the same object is not collected twice. """ if not path.is_absolute(): - raise ValueError("FilePathNode must be instantiated from absolute path.") - return cls(name=path.as_posix(), value=path, path=path) + raise ValueError("Node must be instantiated from absolute path.") + return cls(name=path.as_posix(), value=path) def state(self) -> str | None: """Calculate the state of the node. @@ -126,7 +129,7 @@ def state(self) -> str | None: @define(kw_only=True) -class PythonNode(MetaNode): +class PythonNode(Node): """The class for a node which is a Python object.""" name: str = "" @@ -153,4 +156,4 @@ def state(self) -> str | None: if isinstance(self.value, str): return str(hashlib.sha256(self.value.encode()).hexdigest()) return str(hash(self.value)) - return str(0) + return "0" diff --git a/src/_pytask/profile.py b/src/_pytask/profile.py index e0b29d3bf..5c8a33e57 100644 --- a/src/_pytask/profile.py +++ b/src/_pytask/profile.py @@ -23,7 +23,7 @@ from _pytask.database_utils import DatabaseSession from _pytask.exceptions import CollectionError from _pytask.exceptions import ConfigurationError -from _pytask.nodes import FilePathNode +from _pytask.node_protocols import PPathNode from _pytask.nodes import Task from _pytask.outcomes import ExitCode from _pytask.outcomes import TaskOutcome @@ -228,7 +228,7 @@ def pytask_profile_add_info_on_task( sum_bytes = 0 for successor in successors: node = session.dag.nodes[successor]["node"] - if isinstance(node, FilePathNode): + if isinstance(node, PPathNode): with suppress(FileNotFoundError): sum_bytes += node.path.stat().st_size diff --git a/src/_pytask/report.py b/src/_pytask/report.py index 0bc0cf03d..7df151fc3 100644 --- a/src/_pytask/report.py +++ b/src/_pytask/report.py @@ -15,7 +15,7 @@ if TYPE_CHECKING: - from _pytask.nodes import MetaNode + from _pytask.node_protocols import MetaNode from _pytask.nodes import Task diff --git a/src/_pytask/shared.py b/src/_pytask/shared.py index 4747813d7..6059bd1b2 100644 --- a/src/_pytask/shared.py +++ b/src/_pytask/shared.py @@ -10,8 +10,8 @@ import click import networkx as nx from _pytask.console import format_task_id -from _pytask.nodes import FilePathNode -from _pytask.nodes import MetaNode +from _pytask.node_protocols import MetaNode +from _pytask.node_protocols import PPathNode from _pytask.nodes import Task from _pytask.path import find_closest_ancestor from _pytask.path import find_common_ancestor @@ -67,7 +67,7 @@ def reduce_node_name(node: MetaNode, paths: Sequence[str | Path]) -> str: path from one path in ``session.config["paths"]`` to the node. """ - if isinstance(node, (Task, FilePathNode)): + if isinstance(node, (PPathNode, Task)): ancestor = find_closest_ancestor(node.path, paths) if ancestor is None: try: @@ -75,10 +75,7 @@ def reduce_node_name(node: MetaNode, paths: Sequence[str | Path]) -> str: except ValueError: ancestor = node.path.parents[-1] - if isinstance(node, MetaNode): - name = relative_to(node.path, ancestor).as_posix() - else: - raise TypeError(f"Unknown node {node} with type {type(node)!r}.") + name = relative_to(node.path, ancestor).as_posix() return name return node.name diff --git a/src/pytask/__init__.py b/src/pytask/__init__.py index 1b5eca521..d626d4dd3 100644 --- a/src/pytask/__init__.py +++ b/src/pytask/__init__.py @@ -36,8 +36,8 @@ from _pytask.mark_utils import set_marks from _pytask.models import CollectionMetadata from _pytask.models import NodeInfo -from _pytask.nodes import FilePathNode -from _pytask.nodes import MetaNode +from _pytask.node_protocols import MetaNode +from _pytask.nodes import PathNode from _pytask.nodes import Product from _pytask.nodes import PythonNode from _pytask.nodes import Task @@ -84,7 +84,7 @@ "ExecutionReport", "Exit", "ExitCode", - "FilePathNode", + "PathNode", "Mark", "MarkDecorator", "MarkGenerator", diff --git a/tests/test_collect.py b/tests/test_collect.py index db951b15e..3e6c117b4 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -422,3 +422,24 @@ def task_write_text(depends_on, produces): assert "FutureWarning" in result.output assert "Using strings to specify a dependency" in result.output assert "Using strings to specify a product" in result.output + + +@pytest.mark.end_to_end() +def test_setting_name_for_path_node_via_annotation(tmp_path): + source = """ + from pathlib import Path + from typing_extensions import Annotated + from pytask import Product, PathNode + from typing import Any + + def task_example( + path: Annotated[Path, Product, PathNode(name="product")] = Path("out.txt"), + ) -> None: + path.write_text("text") + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + session = main({"paths": [tmp_path]}) + assert session.exit_code == ExitCode.OK + product = session.tasks[0].produces["path"] + assert product.name == "product" diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index 6ab330119..4856db532 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -1,17 +1,17 @@ from __future__ import annotations import os +import pickle import textwrap from pathlib import Path import pytest from _pytask.collect_command import _find_common_ancestor_of_all_nodes from _pytask.collect_command import _print_collected_tasks -from _pytask.nodes import FilePathNode +from _pytask.nodes import PathNode from attrs import define from pytask import cli from pytask import ExitCode -from pytask import MetaNode from pytask import Task @@ -343,7 +343,7 @@ def task_example_2(): @define -class MetaNode(MetaNode): +class Node: path: Path def state(self): @@ -362,8 +362,8 @@ def test_print_collected_tasks_without_nodes(capsys): base_name="function", path=Path("task_path.py"), function=function, - depends_on={0: MetaNode("in.txt")}, - produces={0: MetaNode("out.txt")}, + depends_on={0: Node("in.txt")}, + produces={0: Node("out.txt")}, ) ] } @@ -386,15 +386,9 @@ def test_print_collected_tasks_with_nodes(capsys): path=Path("task_path.py"), function=function, depends_on={ - "depends_on": FilePathNode( - name="in.txt", value=Path("in.txt"), path=Path("in.txt") - ) - }, - produces={ - 0: FilePathNode( - name="out.txt", value=Path("out.txt"), path=Path("out.txt") - ) + "depends_on": PathNode(name="in.txt", value=Path("in.txt")) }, + produces={0: PathNode(name="out.txt", value=Path("out.txt"))}, ) ] } @@ -418,10 +412,10 @@ def test_find_common_ancestor_of_all_nodes(show_nodes, expected_add): path=Path.cwd() / "src" / "task_path.py", function=function, depends_on={ - "depends_on": FilePathNode.from_path(Path.cwd() / "src" / "in.txt") + "depends_on": PathNode.from_path(Path.cwd() / "src" / "in.txt") }, produces={ - 0: FilePathNode.from_path( + 0: PathNode.from_path( Path.cwd().joinpath("..", "bld", "out.txt").resolve() ) }, @@ -518,3 +512,92 @@ def task_example( assert "task_example>" in captured assert "" in result.output assert "Product" in captured + + +def test_node_protocol_for_custom_nodes(runner, tmp_path): + source = """ + from typing_extensions import Annotated + from pytask import Product + from attrs import define + from pathlib import Path + + @define + class CustomNode: + name: str + value: str + + def state(self): + return self.value + + + def task_example( + data = CustomNode("custom", "text"), + out: Annotated[Path, Product] = Path("out.txt"), + ) -> None: + out.write_text(data) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + result = runner.invoke(cli, ["collect", "--nodes", tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert "" in result.output + + +def test_node_protocol_for_custom_nodes_with_paths(runner, tmp_path): + source = """ + from typing_extensions import Annotated + from pytask import Product + from pathlib import Path + from attrs import define + import pickle + + @define + class PickleFile: + name: str + path: Path + + @property + def value(self): + with self.path.open("rb") as f: + out = pickle.load(f) + return out + + def state(self): + return str(self.path.stat().st_mtime) + + + _PATH = Path(__file__).parent.joinpath("in.pkl") + + def task_example( + data = PickleFile(_PATH.as_posix(), _PATH), + out: Annotated[Path, Product] = Path("out.txt"), + ) -> None: + out.write_text(data) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.pkl").write_bytes(pickle.dumps("text")) + + result = runner.invoke(cli, ["collect", "--nodes", tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert "in.pkl" in result.output + + +@pytest.mark.end_to_end() +def test_setting_name_for_python_node_via_annotation(runner, tmp_path): + source = """ + from pathlib import Path + from typing_extensions import Annotated + from pytask import Product, PythonNode + from typing import Any + + def task_example( + input: Annotated[str, PythonNode(name="node-name")] = "text", + path: Annotated[Path, Product] = Path("out.txt"), + ) -> None: + path.write_text(input) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + result = runner.invoke(cli, ["collect", "--nodes", tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert "node-name" in result.output diff --git a/tests/test_console.py b/tests/test_console.py index 92cb4fc99..bd0b5eca6 100644 --- a/tests/test_console.py +++ b/tests/test_console.py @@ -127,14 +127,7 @@ def test_render_to_string(color_system, text, strip_styles, expected): None, Text( _THIS_FILE.as_posix() + "::task_a", - spans=[ - Span(0, len(_THIS_FILE.as_posix()) + 2, "dim"), - Span( - len(_THIS_FILE.as_posix()) + 2, - len(_THIS_FILE.as_posix()) + 2 + 6, - Style(), - ), - ], + spans=[Span(0, len(_THIS_FILE.as_posix()) + 2, "dim")], ), id="format full id", ), @@ -146,7 +139,7 @@ def test_render_to_string(color_system, text, strip_styles, expected): None, Text( "test_console.py::task_a", - spans=[Span(0, 17, "dim"), Span(17, 23, Style())], + spans=[Span(0, 17, "dim")], ), id="format short id", ), @@ -158,7 +151,7 @@ def test_render_to_string(color_system, text, strip_styles, expected): _THIS_FILE.parent, Text( "tests/test_console.py::task_a", - spans=[Span(0, 23, "dim"), Span(23, 29, Style())], + spans=[Span(0, 23, "dim")], ), id="format relative to id", ), diff --git a/tests/test_dag.py b/tests/test_dag.py index 017374eba..09259abc8 100644 --- a/tests/test_dag.py +++ b/tests/test_dag.py @@ -3,7 +3,6 @@ import textwrap from contextlib import ExitStack as does_not_raise # noqa: N813 from pathlib import Path -from typing import Any import networkx as nx import pytest @@ -14,18 +13,14 @@ from attrs import define from pytask import cli from pytask import ExitCode -from pytask import FilePathNode +from pytask import PathNode from pytask import Task @define -class Node(FilePathNode): +class Node(PathNode): """See https://github.com/python-attrs/attrs/issues/293 for property hack.""" - name: str - value: Any - path: Path - def state(self): if "missing" in self.name: raise NodeNotFoundError diff --git a/tests/test_execute.py b/tests/test_execute.py index a86dc1768..21b3909a2 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -530,11 +530,11 @@ def test_error_with_multiple_different_dep_annotations(runner, tmp_path): source = """ from pathlib import Path from typing_extensions import Annotated - from pytask import Product, PythonNode, FilePathNode + from pytask import Product, PythonNode, PathNode from typing import Any def task_example( - dependency: Annotated[Any, PythonNode(), FilePathNode()] = "hello", + dependency: Annotated[Any, PythonNode(), PathNode()] = "hello", path: Annotated[Path, Product] = Path("out.txt") ) -> None: path.write_text(dependency) diff --git a/tests/test_node_protocols.py b/tests/test_node_protocols.py new file mode 100644 index 000000000..4cf1944c0 --- /dev/null +++ b/tests/test_node_protocols.py @@ -0,0 +1,75 @@ +from __future__ import annotations + +import pickle +import textwrap + +from pytask import cli +from pytask import ExitCode + + +def test_node_protocol_for_custom_nodes(runner, tmp_path): + source = """ + from typing_extensions import Annotated + from pytask import Product + from attrs import define + from pathlib import Path + + @define + class CustomNode: + name: str + value: str + + def state(self): + return self.value + + + def task_example( + data = CustomNode("custom", "text"), + out: Annotated[Path, Product] = Path("out.txt"), + ) -> None: + out.write_text(data) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").read_text() == "text" + + +def test_node_protocol_for_custom_nodes_with_paths(runner, tmp_path): + source = """ + from typing_extensions import Annotated + from pytask import Product + from pathlib import Path + from attrs import define + import pickle + + @define + class PickleFile: + name: str + path: Path + + @property + def value(self): + with self.path.open("rb") as f: + out = pickle.load(f) + return out + + def state(self): + return str(self.path.stat().st_mtime) + + + _PATH = Path(__file__).parent.joinpath("in.pkl") + + def task_example( + data = PickleFile(_PATH.as_posix(), _PATH), + out: Annotated[Path, Product] = Path("out.txt"), + ) -> None: + out.write_text(data) + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + tmp_path.joinpath("in.pkl").write_bytes(pickle.dumps("text")) + + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("out.txt").read_text() == "text" diff --git a/tests/test_nodes.py b/tests/test_nodes.py index 9165a2f91..a58d7e501 100644 --- a/tests/test_nodes.py +++ b/tests/test_nodes.py @@ -5,7 +5,7 @@ import pytest from _pytask.shared import reduce_node_name -from pytask import FilePathNode +from pytask import PathNode _ROOT = Path.cwd() @@ -16,14 +16,14 @@ ("node", "paths", "expectation", "expected"), [ pytest.param( - FilePathNode.from_path(_ROOT.joinpath("src/module.py")), + PathNode.from_path(_ROOT.joinpath("src/module.py")), [_ROOT.joinpath("alternative_src")], does_not_raise(), "pytask/src/module.py", - id="Common path found for FilePathNode not in 'paths' and 'paths'", + id="Common path found for PathNode not in 'paths' and 'paths'", ), pytest.param( - FilePathNode.from_path(_ROOT.joinpath("top/src/module.py")), + PathNode.from_path(_ROOT.joinpath("top/src/module.py")), [_ROOT.joinpath("top/src")], does_not_raise(), "src/module.py", From 0c4b0c5251df21a558c6ed42ab14032c2bcc6ec3 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Sun, 27 Aug 2023 18:06:12 +0200 Subject: [PATCH 12/12] Allow tasks to return products. (#404) --- src/_pytask/collect_utils.py | 47 +++++++++++-- src/_pytask/execute.py | 4 +- src/_pytask/models.py | 3 + src/_pytask/node_protocols.py | 37 ++++++++++- src/_pytask/nodes.py | 63 +++++++++++++++--- src/_pytask/task_utils.py | 12 +++- src/_pytask/tree_util.py | 5 ++ tests/test_collect.py | 2 +- tests/test_collect_command.py | 19 ++++-- tests/test_execute.py | 122 ++++++++++++++++++++++++++++++++++ tests/test_node_protocols.py | 24 +++++-- tests/test_task.py | 44 ++++++++++++ tests/test_tree_util.py | 2 +- 13 files changed, 350 insertions(+), 34 deletions(-) diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index a5e6e507c..fa51bfc5f 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -308,9 +308,7 @@ def _find_args_with_node_annotation(func: Callable[..., Any]) -> dict[str, Node] args_with_node_annotation = {} for name, meta in metas.items(): - annot = [ - i for i in meta if not isinstance(i, ProductType) and isinstance(i, Node) - ] + annot = [i for i in meta if not isinstance(i, ProductType)] if len(annot) >= 2: # noqa: PLR2004 raise ValueError( f"Parameter {name!r} has multiple node annotations although only one " @@ -360,6 +358,8 @@ def parse_products_from_task_function( has_produces_decorator = False has_produces_argument = False has_annotation = False + has_return = False + has_task_decorator = False out = {} # Parse products from decorators. @@ -415,8 +415,45 @@ def parse_products_from_task_function( ) out = {parameter_name: collected_products} + if "return" in parameters_with_node_annot: + has_return = True + collected_products = tree_map_with_path( + lambda p, x: _collect_product( + session, + path, + name, + NodeInfo("return", p, x), + is_string_allowed=False, + ), + parameters_with_node_annot["return"], + ) + out = {"return": collected_products} + + task_produces = obj.pytask_meta.produces if hasattr(obj, "pytask_meta") else None + if task_produces: + has_task_decorator = True + collected_products = tree_map_with_path( + lambda p, x: _collect_product( + session, + path, + name, + NodeInfo("return", p, x), + is_string_allowed=False, + ), + task_produces, + ) + out = {"return": collected_products} + if ( - sum((has_produces_decorator, has_produces_argument, has_annotation)) + sum( + ( + has_produces_decorator, + has_produces_argument, + has_annotation, + has_return, + has_task_decorator, + ) + ) >= 2 # noqa: PLR2004 ): raise NodeNotCollectedError(_ERROR_MULTIPLE_PRODUCT_DEFINITIONS) @@ -571,5 +608,5 @@ def _evolve_instance(x: Any, instance_from_annot: Node | None) -> Any: if not instance_from_annot: return x - instance_from_annot.value = x + instance_from_annot.from_annot(x) return instance_from_annot diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index a4d703117..6603d33e2 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -147,11 +147,11 @@ def pytask_execute_task(session: Session, task: Task) -> bool: kwargs = {} for name, value in task.depends_on.items(): - kwargs[name] = tree_map(lambda x: x.value, value) + kwargs[name] = tree_map(lambda x: x.load(), value) for name, value in task.produces.items(): if name in parameters: - kwargs[name] = tree_map(lambda x: x.value, value) + kwargs[name] = tree_map(lambda x: x.load(), value) task.execute(**kwargs) return True diff --git a/src/_pytask/models.py b/src/_pytask/models.py index 9b5dc3368..d0080c3ff 100644 --- a/src/_pytask/models.py +++ b/src/_pytask/models.py @@ -5,6 +5,7 @@ from typing import NamedTuple from typing import TYPE_CHECKING +from _pytask.tree_util import PyTree from attrs import define from attrs import field @@ -24,6 +25,8 @@ class CollectionMetadata: """Contains the markers of the function.""" name: str | None = None """The name of the task function.""" + produces: PyTree[Any] = None + """Definition of products to handle returns.""" class NodeInfo(NamedTuple): diff --git a/src/_pytask/node_protocols.py b/src/_pytask/node_protocols.py index f3062cb4c..f6dc2cf09 100644 --- a/src/_pytask/node_protocols.py +++ b/src/_pytask/node_protocols.py @@ -15,7 +15,13 @@ class MetaNode(Protocol): """The name of node that must be unique.""" @abstractmethod - def state(self) -> Any: + def state(self) -> str | None: + """Return the state of the node. + + The state can be something like a hash or a last modified timestamp. If the node + does not exist, you can also return ``None``. + + """ ... @@ -25,9 +31,36 @@ class Node(MetaNode, Protocol): value: Any + def load(self) -> Any: + """Return the value of the node that will be injected into the task.""" + ... + + def save(self, value: Any) -> Any: + """Save the value that was returned from a task.""" + ... + + def from_annot(self, value: Any) -> Any: + """Complete the node by setting the value from an default argument. + + Use it, if you want to add information on how a node handles an argument while + keeping the type of the value unrelated to pytask. + + .. codeblock: python + + def task_example(value: Annotated[Any, PythonNode(hash=True)], produces): + ... + + + """ + ... + @runtime_checkable class PPathNode(Node, Protocol): - """Nodes with paths.""" + """Nodes with paths. + + Nodes with paths receive special handling when it comes to printing their names. + + """ path: Path diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index b86503109..2d8088fd2 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -6,11 +6,14 @@ from pathlib import Path from typing import Any from typing import Callable +from typing import NoReturn from typing import TYPE_CHECKING from _pytask.node_protocols import MetaNode from _pytask.node_protocols import Node from _pytask.tree_util import PyTree +from _pytask.tree_util import tree_leaves +from _pytask.tree_util import tree_structure from attrs import define from attrs import field @@ -28,6 +31,7 @@ class ProductType: Product = ProductType() +"""ProductType: A singleton to mark products in annotations.""" @define(kw_only=True) @@ -64,6 +68,7 @@ def __attrs_post_init__(self: Task) -> None: self.short_name = self.name def state(self, hash: bool = False) -> str | None: # noqa: A002 + """Return the state of the node.""" if hash and self.path.exists(): return hashlib.sha256(self.path.read_bytes()).hexdigest() if not hash and self.path.exists(): @@ -72,7 +77,22 @@ def state(self, hash: bool = False) -> str | None: # noqa: A002 def execute(self, **kwargs: Any) -> None: """Execute the task.""" - self.function(**kwargs) + out = self.function(**kwargs) + + if "return" in self.produces: + structure_out = tree_structure(out) + structure_return = tree_structure(self.produces["return"]) + if not structure_out == structure_return: + raise ValueError( + "The structure of the function return does not match the structure " + f"of the return annotation.\n\nFunction return: {structure_out}\n\n" + f"Return annotation: {structure_return}" + ) + + for out_, return_ in zip( + tree_leaves(out), tree_leaves(self.produces["return"]) + ): + return_.save(out_) def add_report_section(self, when: str, key: str, content: str) -> None: """Add sections which will be displayed in report like stdout or stderr.""" @@ -86,24 +106,20 @@ class PathNode(Node): name: str = "" """Name of the node which makes it identifiable in the DAG.""" - _value: Path | None = None + value: Path | None = None """Value passed to the decorator which can be requested inside the function.""" @property def path(self) -> Path: return self.value - @property - def value(self) -> Path: - return self._value - - @value.setter - def value(self, value: Path) -> None: + def from_annot(self, value: Path) -> None: + """Set path and if other attributes are not set, set sensible defaults.""" if not isinstance(value, Path): raise TypeError("'value' must be a 'pathlib.Path'.") if not self.name: self.name = value.as_posix() - self._value = value + self.value = value @classmethod @functools.lru_cache @@ -127,6 +143,21 @@ def state(self) -> str | None: return str(self.path.stat().st_mtime) return None + def load(self) -> Path: + """Load the value.""" + return self.value + + def save(self, value: bytes | str) -> None: + """Save strings or bytes to file.""" + if isinstance(value, str): + self.path.write_text(value) + elif isinstance(value, bytes): + self.path.write_bytes(value) + else: + raise TypeError( + f"'PathNode' can only save 'str' and 'bytes', not {type(value)}" + ) + @define(kw_only=True) class PythonNode(Node): @@ -134,11 +165,23 @@ class PythonNode(Node): name: str = "" """Name of the node.""" - value: Any | None = None + value: Any = None """Value of the node.""" hash: bool = False # noqa: A003 """Whether the value should be hashed to determine the state.""" + def load(self) -> Any: + """Load the value.""" + return self.value + + def save(self, value: Any) -> NoReturn: + """Save the value.""" + raise NotImplementedError + + def from_annot(self, value: Any) -> None: + """Set the value from a function annotation.""" + self.value = value + def state(self) -> str | None: """Calculate state of the node. diff --git a/src/_pytask/task_utils.py b/src/_pytask/task_utils.py index cd5a1acef..32acd15de 100644 --- a/src/_pytask/task_utils.py +++ b/src/_pytask/task_utils.py @@ -11,6 +11,7 @@ from _pytask.mark import Mark from _pytask.models import CollectionMetadata from _pytask.shared import find_duplicates +from _pytask.tree_util import PyTree __all__ = ["parse_keyword_arguments_from_signature_defaults"] @@ -31,6 +32,7 @@ def task( *, id: str | None = None, # noqa: A002 kwargs: dict[Any, Any] | None = None, + produces: PyTree[Any] = None, ) -> Callable[..., None]: """Parse inputs of the ``@pytask.mark.task`` decorator. @@ -43,13 +45,15 @@ def task( Parameters ---------- - name : str | None + name The name of the task. - id : str | None + id An id for the task if it is part of a parametrization. - kwargs : dict[Any, Any] | None + kwargs A dictionary containing keyword arguments which are passed to the task when it is executed. + produces + Definition of products to handle returns. """ @@ -77,12 +81,14 @@ def wrapper(func: Callable[..., Any]) -> None: unwrapped.pytask_meta.kwargs = parsed_kwargs unwrapped.pytask_meta.markers.append(Mark("task", (), {})) unwrapped.pytask_meta.id_ = id + unwrapped.pytask_meta.produces = produces else: unwrapped.pytask_meta = CollectionMetadata( name=parsed_name, kwargs=parsed_kwargs, markers=[Mark("task", (), {})], id_=id, + produces=produces, ) COLLECTED_TASKS[path].append(unwrapped) diff --git a/src/_pytask/tree_util.py b/src/_pytask/tree_util.py index beb9a0a61..80edcc71a 100644 --- a/src/_pytask/tree_util.py +++ b/src/_pytask/tree_util.py @@ -9,12 +9,14 @@ from optree import tree_leaves as _optree_tree_leaves from optree import tree_map as _optree_tree_map from optree import tree_map_with_path as _optree_tree_map_with_path +from optree import tree_structure as _optree_tree_structure __all__ = [ "tree_leaves", "tree_map", "tree_map_with_path", + "tree_structure", "PyTree", "TREE_UTIL_LIB_DIRECTORY", ] @@ -29,3 +31,6 @@ tree_map_with_path = functools.partial( _optree_tree_map_with_path, none_is_leaf=True, namespace="pytask" ) +tree_structure = functools.partial( + _optree_tree_structure, none_is_leaf=True, namespace="pytask" +) diff --git a/tests/test_collect.py b/tests/test_collect.py index 3e6c117b4..198328586 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -172,7 +172,7 @@ def test_pytask_collect_node(session, path, node_info, expected): if result is None: assert result is expected else: - assert str(result.value) == str(expected) + assert str(result.load()) == str(expected) @pytest.mark.unit() diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index 4856db532..41e228499 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -529,6 +529,9 @@ class CustomNode: def state(self): return self.value + def load(self): ... + def save(self, value): ... + def from_annot(self, value): ... def task_example( data = CustomNode("custom", "text"), @@ -555,21 +558,27 @@ def test_node_protocol_for_custom_nodes_with_paths(runner, tmp_path): class PickleFile: name: str path: Path + value: Path - @property - def value(self): + def state(self): + return str(self.path.stat().st_mtime) + + def load(self): with self.path.open("rb") as f: out = pickle.load(f) return out - def state(self): - return str(self.path.stat().st_mtime) + def save(self, value): + with self.path.open("wb") as f: + pickle.dump(value, f) + + def from_annot(self, value): ... _PATH = Path(__file__).parent.joinpath("in.pkl") def task_example( - data = PickleFile(_PATH.as_posix(), _PATH), + data = PickleFile(_PATH.as_posix(), _PATH, _PATH), out: Annotated[Path, Product] = Path("out.txt"), ) -> None: out.write_text(data) diff --git a/tests/test_execute.py b/tests/test_execute.py index 21b3909a2..7bddaa416 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +import pickle import re import subprocess import sys @@ -544,3 +545,124 @@ def task_example( result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.COLLECTION_FAILED assert "Parameter 'dependency'" in result.output + + +@pytest.mark.end_to_end() +def test_return_with_path_annotation_as_return(runner, tmp_path): + source = """ + from pathlib import Path + from typing import Any + from typing_extensions import Annotated + from pytask import PathNode + + def task_example() -> Annotated[str, Path("file.txt")]: + return "Hello, World!" + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("file.txt").read_text() == "Hello, World!" + + +@pytest.mark.end_to_end() +def test_return_with_pathnode_annotation_as_return(runner, tmp_path): + source = """ + from pathlib import Path + from typing import Any + from typing_extensions import Annotated + from pytask import PathNode + + node = PathNode.from_path(Path(__file__).parent.joinpath("file.txt")) + + def task_example() -> Annotated[str, node]: + return "Hello, World!" + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("file.txt").read_text() == "Hello, World!" + + +@pytest.mark.end_to_end() +def test_return_with_tuple_pathnode_annotation_as_return(runner, tmp_path): + source = """ + from pathlib import Path + from typing import Any + from typing_extensions import Annotated + from pytask import PathNode + + node1 = PathNode.from_path(Path(__file__).parent.joinpath("file1.txt")) + node2 = PathNode.from_path(Path(__file__).parent.joinpath("file2.txt")) + + def task_example() -> Annotated[str, (node1, node2)]: + return "Hello,", "World!" + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("file1.txt").read_text() == "Hello," + assert tmp_path.joinpath("file2.txt").read_text() == "World!" + + +@pytest.mark.end_to_end() +def test_return_with_custom_type_annotation_as_return(runner, tmp_path): + source = """ + from __future__ import annotations + + from pathlib import Path + import pickle + from typing import Any + from typing_extensions import Annotated + import attrs + + @attrs.define + class PickleNode: + name: str = "" + path: Path | None = None + value: None = None + + def state(self) -> str | None: + if self.path.exists(): + return str(self.path.stat().st_mtime) + return None + + def load(self) -> Any: + return pickle.loads(self.path.read_bytes()) + + def save(self, value: Any) -> None: + self.path.write_bytes(pickle.dumps(value)) + + def from_annot(self, value: Any) -> None: ... + + node = PickleNode("pickled_data", Path(__file__).parent.joinpath("data.pkl")) + + def task_example() -> Annotated[int, node]: + return 1 + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + + data = pickle.loads(tmp_path.joinpath("data.pkl").read_bytes()) # noqa: S301 + assert data == 1 + + +@pytest.mark.end_to_end() +def test_error_when_return_pytree_mismatch(runner, tmp_path): + source = """ + from pathlib import Path + from typing import Any + from typing_extensions import Annotated + from pytask import PathNode + + node1 = PathNode.from_path(Path(__file__).parent.joinpath("file1.txt")) + node2 = PathNode.from_path(Path(__file__).parent.joinpath("file2.txt")) + + def task_example() -> Annotated[str, (node1, node2)]: + return "Hello," + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.FAILED + assert "Function return: PyTreeSpec(*, NoneIsLeaf)" in result.output + assert "Return annotation: PyTreeSpec((*, *), NoneIsLeaf)" in result.output diff --git a/tests/test_node_protocols.py b/tests/test_node_protocols.py index 4cf1944c0..02ce9867d 100644 --- a/tests/test_node_protocols.py +++ b/tests/test_node_protocols.py @@ -22,6 +22,14 @@ class CustomNode: def state(self): return self.value + def load(self): + return self.value + + def save(self, value): + self.value = value + + def from_annot(self, value): ... + def task_example( data = CustomNode("custom", "text"), @@ -48,21 +56,27 @@ def test_node_protocol_for_custom_nodes_with_paths(runner, tmp_path): class PickleFile: name: str path: Path + value: Path + + def state(self): + return str(self.path.stat().st_mtime) - @property - def value(self): + def load(self): with self.path.open("rb") as f: out = pickle.load(f) return out - def state(self): - return str(self.path.stat().st_mtime) + def save(self, value): + with self.path.open("wb") as f: + pickle.dump(value, f) + + def from_annot(self, value): ... _PATH = Path(__file__).parent.joinpath("in.pkl") def task_example( - data = PickleFile(_PATH.as_posix(), _PATH), + data = PickleFile(_PATH.as_posix(), _PATH, _PATH), out: Annotated[Path, Product] = Path("out.txt"), ) -> None: out.write_text(data) diff --git a/tests/test_task.py b/tests/test_task.py index b5d411fa8..7bf9157b1 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -511,3 +511,47 @@ def task_example( assert result.exit_code == ExitCode.OK assert tmp_path.joinpath("out.txt").read_text() == "Hello world!" assert not tmp_path.joinpath("not_used_out.txt").exists() + + +@pytest.mark.end_to_end() +def test_return_with_task_decorator(runner, tmp_path): + source = """ + from pathlib import Path + from typing import Any + from typing_extensions import Annotated + from pytask import PathNode + import pytask + + node = PathNode.from_path(Path(__file__).parent.joinpath("file.txt")) + + @pytask.mark.task(produces=node) + def task_example(): + return "Hello, World!" + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("file.txt").read_text() == "Hello, World!" + + +@pytest.mark.end_to_end() +def test_return_with_tuple_and_task_decorator(runner, tmp_path): + source = """ + from pathlib import Path + from typing import Any + from typing_extensions import Annotated + from pytask import PathNode + import pytask + + node1 = PathNode.from_path(Path(__file__).parent.joinpath("file1.txt")) + node2 = PathNode.from_path(Path(__file__).parent.joinpath("file2.txt")) + + @pytask.mark.task(produces=(node1, node2)) + def task_example(): + return "Hello,", "World!" + """ + tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.OK + assert tmp_path.joinpath("file1.txt").read_text() == "Hello," + assert tmp_path.joinpath("file2.txt").read_text() == "World!" diff --git a/tests/test_tree_util.py b/tests/test_tree_util.py index ed43014ab..34341e9bd 100644 --- a/tests/test_tree_util.py +++ b/tests/test_tree_util.py @@ -43,7 +43,7 @@ def task_example(): assert session.exit_code == exit_code - products = tree_map(lambda x: x.value, getattr(session.tasks[0], decorator_name)) + products = tree_map(lambda x: x.load(), getattr(session.tasks[0], decorator_name)) expected = { 0: tmp_path / "out.txt", 1: {0: tmp_path / "tuple_out.txt"},