Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of names and signatures of PythonNodes. #482

Merged
merged 8 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.4
hooks:
- id: ruff-format
- id: ruff
args: [--unsafe-fixes]
- id: ruff-format
- repo: https://github.com/dosisod/refurb
rev: v1.22.1
hooks:
Expand Down
1 change: 1 addition & 0 deletions docs/source/changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and
- {pull}`480` removes the check for missing root nodes from the generation of the DAG.
It is delegated to the check during the execution.
- {pull}`481` improves coverage.
- {pull}`482` correctly handles names and signatures of {class}`~pytask.PythonNode`.

## 0.4.1 - 2023-10-11

Expand Down
7 changes: 7 additions & 0 deletions src/_pytask/_hashlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,14 @@ def hash_value(value: Any) -> int | str:
Compute the hash of paths, strings, and bytes with a hash function or otherwise the
hashes are salted.

The hash of None constant https://github.com/python/cpython/pull/99541 starting with
Python 3.12.

"""
if value is None:
return 0xFCA86420
if isinstance(value, (tuple, list)):
value = "".join(str(hash_value(i)) for i in value)
if isinstance(value, Path):
value = str(value)
if isinstance(value, str):
Expand Down
6 changes: 4 additions & 2 deletions src/_pytask/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,9 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PN
node = node_info.value

if isinstance(node, PythonNode):
node.name = create_name_of_python_node(node_info)
node.node_info = node_info
if not node.name:
node.name = create_name_of_python_node(node_info)
return node

if isinstance(node, PPathNode) and not node.path.is_absolute():
Expand Down Expand Up @@ -363,7 +365,7 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PN
return PathNode(name=name, path=node)

node_name = create_name_of_python_node(node_info)
return PythonNode(value=node, name=node_name)
return PythonNode(value=node, name=node_name, node_info=node_info)


def _raise_error_if_casing_of_path_is_wrong(
Expand Down
6 changes: 1 addition & 5 deletions src/_pytask/collect_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,11 +684,7 @@ def _collect_product(

def create_name_of_python_node(node_info: NodeInfo) -> str:
"""Create name of PythonNode."""
prefix = (
node_info.task_path.as_posix() + "::" + node_info.task_name
if node_info.task_path
else node_info.task_name
)
prefix = node_info.task_name if node_info.task_path else node_info.task_name
node_name = prefix + "::" + node_info.arg_name
if node_info.path:
suffix = "-".join(map(str, node_info.path))
Expand Down
13 changes: 12 additions & 1 deletion src/_pytask/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@


if TYPE_CHECKING:
from _pytask.models import NodeInfo
from _pytask.tree_util import PyTree
from _pytask.mark import Mark

Expand Down Expand Up @@ -211,6 +212,8 @@ class PythonNode(PNode):
objects that are hashable like strings and tuples. For dictionaries and other
non-hashable objects, you need to provide a function that can hash these
objects.
node_info
The infos acquired while collecting the node.
signature
The signature of the node.

Expand All @@ -229,11 +232,19 @@ class PythonNode(PNode):
name: str = ""
value: Any | NoDefault = no_default
hash: bool | Callable[[Any], bool] = False # noqa: A003
node_info: NodeInfo | None = None

@property
def signature(self) -> str:
"""The unique signature of the node."""
raw_key = str(hash_value(self.name))
raw_key = (
"".join(
str(hash_value(getattr(self.node_info, name)))
for name in ("arg_name", "path", "task_name", "task_path")
)
if self.node_info
else str(hash_value(self.node_info))
)
return hashlib.sha256(raw_key.encode()).hexdigest()

def load(self, is_product: bool = False) -> Any:
Expand Down
34 changes: 22 additions & 12 deletions tests/__snapshots__/test_collect_command.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,28 @@
Collected tasks:
└── 🐍 <Module test_more_nested_pytree_and_py0/task_module.py>
└── 📝 <Function task_module.py::task_example>
├── 📄 <Product
│ test_more_nested_pytree_and_py0/task_module.py::task_example::return
│ ::0>
├── 📄 <Product
│ test_more_nested_pytree_and_py0/task_module.py::task_example::return
│ ::1-0>
├── 📄 <Product
│ test_more_nested_pytree_and_py0/task_module.py::task_example::return
│ ::1-1>
└── 📄 <Product
test_more_nested_pytree_and_py0/task_module.py::task_example::return
::2>
├── 📄 <Product task_example::return::0>
├── 📄 <Product task_example::return::1-0>
├── 📄 <Product task_example::return::1-1>
└── 📄 <Product task_example::return::2>

────────────────────────────────────────────────────────────────────────────────
'''
# ---
# name: test_more_nested_pytree_and_python_node_as_return_with_names
'''
───────────────────────────── Start pytask session ─────────────────────────────
Platform: <platform> -- Python <version>, pytask <version>, pluggy <version>
Root: <path>
Collected 1 task.

Collected tasks:
└── 🐍 <Module test_more_nested_pytree_and_py1/task_module.py>
└── 📝 <Function task_module.py::task_example>
├── 📄 <Product dict>
├── 📄 <Product int>
├── 📄 <Product tuple1>
└── 📄 <Product tuple2>

────────────────────────────────────────────────────────────────────────────────
'''
Expand Down
27 changes: 27 additions & 0 deletions tests/test_collect_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -619,6 +619,33 @@ def test_more_nested_pytree_and_python_node_as_return(runner, snapshot_cli, tmp_
from pytask import PythonNode
from typing import Dict

nodes = [
PythonNode(),
(PythonNode(), PythonNode()),
PythonNode()
]

def task_example() -> Annotated[Dict[str, str], nodes]:
return [{"first": "a", "second": "b"}, (1, 2), 1]
"""
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
if sys.platform != "win32":
assert result.output == snapshot_cli()


@pytest.mark.end_to_end()
def test_more_nested_pytree_and_python_node_as_return_with_names(
runner, snapshot_cli, tmp_path
):
source = """
from pathlib import Path
from typing import Any
from typing_extensions import Annotated
from pytask import PythonNode
from typing import Dict

nodes = [
PythonNode(name="dict"),
(PythonNode(name="tuple1"), PythonNode(name="tuple2")),
Expand Down
35 changes: 34 additions & 1 deletion tests/test_execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def task_example() -> Annotated[Dict[str, str], PythonNode(name="result")]:


@pytest.mark.end_to_end()
def test_more_nested_pytree_and_python_node_as_return(runner, tmp_path):
def test_more_nested_pytree_and_python_node_as_return_with_names(runner, tmp_path):
source = """
from pathlib import Path
from typing import Any
Expand All @@ -758,6 +758,39 @@ def task_example() -> Annotated[Dict[str, str], nodes]:
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 "1 Succeeded" in result.output

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_more_nested_pytree_and_python_node_as_return(runner, tmp_path):
source = """
from pathlib import Path
from typing import Any
from typing_extensions import Annotated
from pytask import PythonNode
from typing import Dict

nodes = [
PythonNode(),
(PythonNode(), PythonNode()),
PythonNode()
]

def task_example() -> Annotated[Dict[str, str], nodes]:
return [{"first": "a", "second": "b"}, (1, 2), 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
assert "1 Succeeded" in result.output

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()
Expand Down
17 changes: 16 additions & 1 deletion tests/test_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from pathlib import Path

import pytest
from pytask import NodeInfo
from pytask import PathNode
from pytask import PickleNode
from pytask import PNode
Expand Down Expand Up @@ -39,7 +40,21 @@ def test_hash_of_python_node(value, hash_, expected):
),
(
PythonNode(name="name", value=None),
"c8265d64828f9e007a9108251883a2b63954c326c678fca23c49a0b08ea7c925",
"a1f217807169de3253d1176ea5c45be20f3db2e2e81ea26c918f6008d2eb715b",
),
(
PythonNode(
name="name",
value=None,
node_info=NodeInfo(
arg_name="arg_name",
path=(0, 1, "dict_key"),
value=None,
task_path=Path("task_example.py"),
task_name="task_example",
),
),
"7284475a87b8f1aa49c40126c5064269f0ba926265b8fe9158a39a882c6a1512",
),
(
Task(base_name="task", path=Path("task.py"), function=None),
Expand Down