Skip to content

Support tuple items in map #17

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.

Sign up for GitHub

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

Closed
jolovicdev wants to merge 1 commit into from
Closed

Support tuple items in map #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 13 additions & 0 deletions src/cashet/_batch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import asyncio
import logging
from collections.abc import Callable, Iterable
from datetime import timedelta
from typing import Any

Expand Down Expand Up @@ -74,6 +75,18 @@ def normalize_tasks(
return normalized


def build_map_tasks(
Copy link
Copy Markdown

@ds-review ds-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Low build_map_tasks lacks docstring

The function should include a docstring explaining its behavior, especially the tuple unpacking logic, to help maintainability.

func: Callable[..., Any],
items: Iterable[Any],
args: tuple[Any, ...],
kwargs: dict[str, Any],
) -> list[Any]:
Copy link
Copy Markdown

@ds-review ds-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Low Imprecise return type in build_map_tasks

The return type is list[Any], which loses information. Consider using list[tuple[Callable[..., Any], tuple[Any, ...], dict[str, Any]]] for better type safety.

Suggested change
) -> list[Any]:
) -> list[tuple[Callable[..., Any], tuple[Any, ...], dict[str, Any]]]:

return [
(func, (*item, *args) if isinstance(item, tuple) else (item, *args), kwargs)
Copy link
Copy Markdown

@ds-review ds-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 High Breaking change: tuple items now unpacked automatically

The new logic in build_map_tasks unpacks any tuple item with *item, treating each element as a separate positional argument. This silently breaks callers who previously used map with tuple items that were meant to be passed as a single argument. The old behavior treated item as a single argument regardless of type. Consider documenting this as a breaking change or adding a parameter to control unpacking.

Copy link
Copy Markdown

@ds-review ds-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 High Namedtuple items incorrectly unpacked

Namedtuples are instances of tuple, so isinstance(item, tuple) is True. Passing a list of namedtuples will unpack each field as separate arguments, which is probably not the intended behavior. Users might expect namedtuples to be treated as single items. Either document this as intentional or change the check to isinstance(item, tuple) and not namedtuple.

Copy link
Copy Markdown

@ds-review ds-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Medium Empty tuple items lead to missing arguments

If an empty tuple () is passed as an item, *item expands to nothing, so the resulting task will have only the extra *args and kwargs, likely causing TypeError for functions that require at least one positional argument. The previous behavior passed the empty tuple as a single argument. This edge case may cause unexpected errors.

for item in items
]


def build_deps(
keys: list[BatchKey],
normalized: list[NormalizedTask],
Expand Down
3 changes: 2 additions & 1 deletion src/cashet/async_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from cashet._batch import (
build_deps,
build_map_tasks,
execute_batch,
normalize_tasks,
topological_sort,
Expand Down Expand Up @@ -277,7 +278,7 @@ async def map(
max_workers: int | None = None,
**kwargs: Any,
) -> list[AsyncResultRef[T]]:
task_list: list[Any] = [(func, (item, *args), kwargs) for item in items]
task_list = build_map_tasks(func, items, args, kwargs)
return await self.submit_many(
task_list,
_cache=_cache,
Expand Down
12 changes: 12 additions & 0 deletions tests/test_map.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ def greet(name: str, greeting: str = "hello") -> str:
return f"{greeting} {name}"


def add_pair(x: int, y: int) -> int:
return x + y


class TestSyncMap:
def test_map_basic(self, client: Client) -> None:
refs = client.map(double, [1, 2, 3])
Expand All @@ -38,6 +42,10 @@ def test_map_with_extra_args(self, client: Client) -> None:
refs = client.map(add, [1, 2, 3], 10)
assert [r.load() for r in refs] == [11, 12, 13]

def test_map_with_tuple_items(self, client: Client) -> None:
refs = client.map(add_pair, [(1, 2), (3, 4)])
Copy link
Copy Markdown

@ds-review ds-review Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 Low Missing edge case tests for tuple unpacking

The new test only covers a tuple of two elements. Consider adding tests for empty tuple, single-element tuple, and mixed non-tuple/tuple items to ensure correctness.

assert [r.load() for r in refs] == [3, 7]

def test_map_with_kwargs(self, client: Client) -> None:
refs = client.map(greet, ["alice", "bob"], greeting="hi")
assert [r.load() for r in refs] == ["hi alice", "hi bob"]
Expand Down Expand Up @@ -74,6 +82,10 @@ async def test_map_with_extra_args(self, async_client: AsyncClient) -> None:
refs = await async_client.map(add, [1, 2, 3], 10)
assert [await r.load() for r in refs] == [11, 12, 13]

async def test_map_with_tuple_items(self, async_client: AsyncClient) -> None:
refs = await async_client.map(add_pair, [(1, 2), (3, 4)])
assert [await r.load() for r in refs] == [3, 7]

async def test_map_caching(self, async_client: AsyncClient) -> None:
call_count = 0

Expand Down
Loading