From 2ff2865b4cf698f1a79c11dfceb347cb8c4a7c9f Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Thu, 7 Sep 2023 09:10:03 +0200 Subject: [PATCH] Remove .from_annot. --- docs/source/changes.md | 1 + src/_pytask/collect_command.py | 11 ++---- src/_pytask/collect_utils.py | 39 +++++---------------- src/_pytask/execute.py | 2 -- src/_pytask/node_protocols.py | 5 ++- src/_pytask/nodes.py | 63 +++------------------------------- src/_pytask/profile.py | 2 -- src/_pytask/typing.py | 29 +--------------- tests/test_collect.py | 2 +- tests/test_collect_command.py | 9 ++--- tests/test_execute.py | 13 ++++--- tests/test_node_protocols.py | 6 ---- 12 files changed, 27 insertions(+), 155 deletions(-) diff --git a/docs/source/changes.md b/docs/source/changes.md index 11ebc8c2..0bc5a062 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -32,6 +32,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`412` adds protocols for tasks. - {pull}`413` removes scripts to generate `.svg`s. - {pull}`414` allow more ruff rules. +- {pull}`416` removes `.from_annot` again. ## 0.3.2 - 2023-06-07 diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index bbbba485..7ebeb56f 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -29,7 +29,6 @@ from _pytask.pluginmanager import get_plugin_manager from _pytask.session import Session from _pytask.tree_util import tree_leaves -from _pytask.typing import no_value from rich.text import Text from rich.tree import Tree @@ -129,14 +128,10 @@ 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, PPathNode) and x.path is not no_value + 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) - if isinstance(x, PPathNode) and x.path is not no_value + x.path for x in tree_leaves(task.produces) if isinstance(x, PPathNode) ) return find_common_ancestor(*all_paths, *paths) @@ -208,7 +203,6 @@ def _print_collected_tasks( sorted_nodes = sorted(nodes, key=lambda x: x.name) for node in sorted_nodes: if isinstance(node, PPathNode): - assert node.path is not no_value if node.path.as_posix() in node.name: reduced_node_name = str( relative_to(node.path, common_ancestor) @@ -228,7 +222,6 @@ def _print_collected_tasks( tree_leaves(task.produces), key=lambda x: getattr(x, "path", x.name) ): if isinstance(node, PPathNode): - assert node.path is not no_value reduced_node_name = str(relative_to(node.path, common_ancestor)) url_style = create_url_style_for_path( node.path, editor_url_scheme diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 0a4767a2..b65805b4 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -1,7 +1,6 @@ """Contains utility functions for :mod:`_pytask.collect`.""" from __future__ import annotations -import functools import itertools import uuid import warnings @@ -275,17 +274,12 @@ def parse_dependencies_from_task_function( if parameter_name == "depends_on": continue - 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_dependency( session, path, name, - NodeInfo(parameter_name, p, partialed_evolve(x)), # noqa: B023 + NodeInfo(parameter_name, p, x), # noqa: B023 ), value, ) @@ -402,22 +396,21 @@ def parse_products_from_task_function( if parameters_with_product_annot: has_annotation = True - for parameter_name in parameters_with_product_annot: - if parameter_name in kwargs: - partialed_evolve = functools.partial( - _evolve_instance, - instance_from_annot=parameters_with_node_annot.get(parameter_name), - ) + for parameter_name in parameters_with_product_annot: + value = kwargs.get(parameter_name) or parameters_with_node_annot.get( + parameter_name + ) + if value: collected_products = tree_map_with_path( lambda p, x: _collect_product( session, path, name, - NodeInfo(parameter_name, p, partialed_evolve(x)), # noqa: B023 + NodeInfo(parameter_name, p, x), # noqa: B023 is_string_allowed=False, ), - kwargs[parameter_name], + value, ) out = {parameter_name: collected_products} @@ -602,19 +595,3 @@ def _collect_product( raise NodeNotCollectedError(msg) 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 - - if not hasattr(instance_from_annot, "from_annot"): - msg = ( - f"The node {instance_from_annot!r} does not define '.from_annot' which is " - f"necessary to complete the node with the value {x!r}." - ) - raise AttributeError(msg) - - instance_from_annot.from_annot(x) # type: ignore[attr-defined] - return instance_from_annot diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index d20074e4..902fe352 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -36,7 +36,6 @@ from _pytask.tree_util import tree_leaves from _pytask.tree_util import tree_map from _pytask.tree_util import tree_structure -from _pytask.typing import no_value from rich.text import Text if TYPE_CHECKING: @@ -136,7 +135,6 @@ def pytask_execute_task_setup(session: Session, task: PTask) -> None: for product in session.dag.successors(task.name): node = session.dag.nodes[product]["node"] if isinstance(node, PPathNode): - assert node.path is not no_value node.path.parent.mkdir(parents=True, exist_ok=True) would_be_executed = has_mark(task, "would_be_executed") diff --git a/src/_pytask/node_protocols.py b/src/_pytask/node_protocols.py index dc014f5a..c2328b7b 100644 --- a/src/_pytask/node_protocols.py +++ b/src/_pytask/node_protocols.py @@ -9,7 +9,6 @@ if TYPE_CHECKING: - from _pytask.typing import NoValue from _pytask.tree_util import PyTree from pathlib import Path from _pytask.mark import Mark @@ -54,7 +53,7 @@ class PPathNode(Node, Protocol): """ - path: Path | NoValue + path: Path @runtime_checkable @@ -82,4 +81,4 @@ class PTaskWithPath(PTask, Protocol): """ - path: Path | NoValue + path: Path diff --git a/src/_pytask/nodes.py b/src/_pytask/nodes.py index ab3a4298..957b7ca6 100644 --- a/src/_pytask/nodes.py +++ b/src/_pytask/nodes.py @@ -3,7 +3,7 @@ import functools import hashlib -from pathlib import Path +from pathlib import Path # noqa: TCH003 from typing import Any from typing import Callable from typing import TYPE_CHECKING @@ -11,8 +11,6 @@ from _pytask.node_protocols import MetaNode from _pytask.node_protocols import Node from _pytask.node_protocols import PPathNode -from _pytask.typing import no_value -from _pytask.typing import NoValue from attrs import define from attrs import field @@ -76,33 +74,11 @@ def execute(self, **kwargs: Any) -> None: class PathNode(PPathNode): """The class for a node which is a path.""" - name: str = "" + name: str """Name of the node which makes it identifiable in the DAG.""" - path: Path | NoValue = no_value + path: Path """The path to the file.""" - def from_annot(self, value: Path) -> None: - """Set path and if other attributes are not set, set sensible defaults. - - Use it, if you want to control the name of the node. - - .. codeblock: python - - def task_example(value: Annotated[Any, PathNode(name="value")]): - ... - - - """ - if self.path is not no_value: - msg = "'path' is already defined." - raise ValueError(msg) - if not isinstance(value, Path): - msg = "'value' must be a 'pathlib.Path'." - raise TypeError(msg) - if not self.name: - self.name = value.as_posix() - self.path = value - @classmethod @functools.lru_cache def from_path(cls, path: Path) -> PathNode: @@ -122,24 +98,16 @@ def state(self) -> str | None: The state is given by the modification timestamp. """ - if self.path is no_value: - return None if self.path.exists(): return str(self.path.stat().st_mtime) return None def load(self) -> Path: """Load the value.""" - if self.path is no_value: - msg = "PathNode has no path defined." - raise ValueError(msg) return self.path def save(self, value: bytes | str) -> None: """Save strings or bytes to file.""" - if self.path is no_value: - msg = "'path' must be a 'pathlib.Path' to store data." - raise TypeError(msg) if isinstance(value, str): self.path.write_text(value) elif isinstance(value, bytes): @@ -155,40 +123,19 @@ class PythonNode(Node): name: str = "" """Name of the node.""" - value: Any | NoValue = no_value + value: Any | None = None """Value of the node.""" hash: bool | Callable[[Any], bool] = False # noqa: A003 """Whether the value should be hashed to determine the state.""" def load(self) -> Any: """Load the value.""" - if self.value is no_value: - msg = "PythonNode has no value defined." - raise ValueError(msg) return self.value def save(self, value: Any) -> None: """Save the value.""" self.value = value - def from_annot(self, value: Any) -> None: - """Set the value from a function annotation. - - 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. For example, the node could - be hashed. - - .. codeblock: python - - def task_example(value: Annotated[Any, PythonNode(hash=True)]): - ... - - """ - if self.value is not no_value: - msg = "'value' is already defined." - raise ValueError(msg) - self.value = value - def state(self) -> str | None: """Calculate state of the node. @@ -204,8 +151,6 @@ def state(self) -> str | None: random integer and it would confuse users. """ - if self.value is no_value: - return None if self.hash: if callable(self.hash): return str(self.hash(self.value)) diff --git a/src/_pytask/profile.py b/src/_pytask/profile.py index e65c8fe0..5fc2f7ce 100644 --- a/src/_pytask/profile.py +++ b/src/_pytask/profile.py @@ -29,7 +29,6 @@ from _pytask.pluginmanager import get_plugin_manager from _pytask.session import Session from _pytask.traceback import render_exc_info -from _pytask.typing import no_value from rich.table import Table from sqlalchemy import Column from sqlalchemy import Float @@ -230,7 +229,6 @@ def pytask_profile_add_info_on_task( node = session.dag.nodes[successor]["node"] if isinstance(node, PPathNode): with suppress(FileNotFoundError): - assert node.path is not no_value sum_bytes += node.path.stat().st_size profile[task.name]["Size of Products"] = _to_human_readable_size( diff --git a/src/_pytask/typing.py b/src/_pytask/typing.py index bcdb8dd0..e568c7ee 100644 --- a/src/_pytask/typing.py +++ b/src/_pytask/typing.py @@ -1,36 +1,9 @@ from __future__ import annotations -from enum import Enum -from typing import Literal - from attr import define -__all__ = ["no_value", "NoValue"] - - -class _NoValue(Enum): - """Sentinel value when no value was provided. - - For nodes, we need a different default value than :obj:`None` to signal that the - user provided no value. - - We make this an Enum - - - because it round-trips through pickle correctly (see GH#pandas/40397) - - because mypy does not understand singletons - - """ - - no_value = "NO_VALUE" - - def __repr__(self) -> str: - return "" - - -# Note: no_value is exported to the public API in pytask -no_value = _NoValue.no_value -NoValue = Literal[_NoValue.no_value] +__all__ = ["Product", "ProductType"] @define(frozen=True) diff --git a/tests/test_collect.py b/tests/test_collect.py index a462bf7d..b0d3fcac 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -433,7 +433,7 @@ def test_setting_name_for_path_node_via_annotation(tmp_path): from typing import Any def task_example( - path: Annotated[Path, Product, PathNode(name="product")] = Path("out.txt"), + path: Annotated[Path, Product, PathNode(path=Path("out.txt"), name="product")], ) -> None: path.write_text("text") """ diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index 1bb9fb34..86a48386 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -529,7 +529,6 @@ def state(self): def load(self): ... def save(self, value): ... - def from_annot(self, value): ... def task_example( data = CustomNode("custom", "text"), @@ -556,7 +555,6 @@ 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) @@ -570,13 +568,10 @@ 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, _PATH), + data = PickleFile(_PATH.as_posix(), _PATH), out: Annotated[Path, Product] = Path("out.txt"), ) -> None: out.write_text(data) @@ -598,7 +593,7 @@ def test_setting_name_for_python_node_via_annotation(runner, tmp_path): from typing import Any def task_example( - input: Annotated[str, PythonNode(name="node-name")] = "text", + input: Annotated[str, PythonNode(name="node-name", value="text")], path: Annotated[Path, Product] = Path("out.txt"), ) -> None: path.write_text(input) diff --git a/tests/test_execute.py b/tests/test_execute.py index c9f5b8d8..0cb388f7 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -470,7 +470,7 @@ def task_example( "definition", [ " = PythonNode(value=data['dependency'], hash=True)", - ": Annotated[Any, PythonNode(hash=True)] = data['dependency']", + ": Annotated[Any, PythonNode(value=data['dependency'], hash=True)]", ], ) def test_task_with_hashed_python_node(runner, tmp_path, definition): @@ -532,8 +532,10 @@ def test_error_with_multiple_different_dep_annotations(runner, tmp_path): from pytask import Product, PythonNode, PathNode from typing import Any + annotation = Annotated[Any, PythonNode(), PathNode(name="a", path=Path("a.txt"))] + def task_example( - dependency: Annotated[Any, PythonNode(), PathNode()] = "hello", + dependency: annotation = "hello", path: Annotated[Path, Product] = Path("out.txt") ) -> None: path.write_text(dependency) @@ -615,9 +617,8 @@ def test_return_with_custom_type_annotation_as_return(runner, tmp_path): @attrs.define class PickleNode: - name: str = "" - path: Path | None = None - value: None = None + name: str + path: Path def state(self) -> str | None: if self.path.exists(): @@ -630,8 +631,6 @@ def load(self) -> Any: 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]: diff --git a/tests/test_node_protocols.py b/tests/test_node_protocols.py index 02ce9867..6fafd14f 100644 --- a/tests/test_node_protocols.py +++ b/tests/test_node_protocols.py @@ -28,9 +28,6 @@ def load(self): def save(self, value): self.value = value - def from_annot(self, value): ... - - def task_example( data = CustomNode("custom", "text"), out: Annotated[Path, Product] = Path("out.txt"), @@ -70,9 +67,6 @@ 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(