Skip to content

Commit 3287d82

Browse files
authored
Support functools.partial with lithops (zarr-developers#899)
* Run doctests via pytest * Support use of functools.partial with Lithops. * Use mode "forkserver" with ProcessPoolExecutor if the default mode is "fork." This can be removed once our minimum python version is 3.14, as "fork" will no longer be a default mode.
1 parent 552e5bf commit 3287d82

13 files changed

Lines changed: 215 additions & 104 deletions

File tree

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
1-
<!-- Feel free to remove check-list items aren't relevant to your change -->
1+
# What I did
2+
<!-- Please describe what you did and why -->
3+
4+
Acceptance criteria:
5+
<!-- Feel free to remove check-list items that aren't relevant to your change -->
26

37
- [ ] Closes #xxxx
48
- [ ] Tests added
59
- [ ] Tests passing
10+
- [ ] No test coverage regression
611
- [ ] Full type hint coverage
7-
- [ ] Changes are documented in `docs/releases.rst`
8-
- [ ] New functions/methods are listed in `api.rst`
12+
- [ ] Changes are documented in `docs/releases.md`
13+
- [ ] New functions/methods are listed in an appropriate `*.md` file under `docs/api`
914
- [ ] New functionality has documentation

.github/workflows/minimum-versions.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ concurrency:
1818
cancel-in-progress: true
1919

2020
jobs:
21-
test-upstream:
21+
test-minimum-versions:
2222
name: minimum-versions-build
2323
if: |
2424
always()

docs/releases.md

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,15 @@
1818
By [Tom Nicholas](https://github.com/TomNicholas).
1919
- Raise clearer error when kerchunk references have malformed codec specifications.
2020
([#864](https://github.com/zarr-developers/VirtualiZarr/pull/864)).
21-
- Fixed warnings caused by outdated imports from `obspec_utils` ([#863](https://github.com/zarr-developers/VirtualiZarr/pull/863)).
21+
- Fix warnings caused by outdated imports from `obspec_utils` ([#863](https://github.com/zarr-developers/VirtualiZarr/pull/863)).
2222
By [Tom Nicholas](https://github.com/TomNicholas).
2323
- Allow `ZarrParser` to work from inside a running event loop (e.g. inside a Jupyter Notebook) ([#900](https://github.com/zarr-developers/VirtualiZarr/pull/900))
2424
By [Julius Busecke](https://github.com/jbusecke).
25+
- Fix Lithops executor to allow use of `functools.partial`, and update
26+
`get_executor` function to ensure `ProcessPoolExecutor` uses `"forkserver"`
27+
mode on platforms that default to `"fork"`
28+
([#899](https://github.com/zarr-developers/VirtualiZarr/pull/899)). By [Chuck
29+
Daniels](https://github.com/chuckwondo).
2530

2631
### Documentation
2732

@@ -43,10 +48,10 @@ This release moves the `ObjectStoreRegistry` to a separate package `obspec_utils
4348
([#844](https://github.com/zarr-developers/VirtualiZarr/pull/844)).
4449
By [Max Jones](https://github.com/maxrjones).
4550

46-
- `ObjectStoreRegistry` has moved from `virtualizarr.registry` to `obspec_utils.registry`. The old import path still works but emits a `DeprecationWarning` and will be removed in a future release.
47-
- `ObstoreReader` has been removed from `virtualizarr.utils`. This should not break user's code, as it was not part of the public/documented API. See [obspec_utils](https://obspec-utils.readthedocs.io/en/latest/api/obspec/) for public file handlers.
48-
- Added `obspec_utils>=0.7.0` as a required dependency. This package provides the `ObjectStoreRegistry` that was previously part of VirtualiZarr.
49-
- Minimum required version of `obstore` is now `0.7.0` (previously `0.5.1`). This was the first release to implement obspec protocols.
51+
- `ObjectStoreRegistry` has moved from `virtualizarr.registry` to `obspec_utils.registry`. The old import path still works but emits a `DeprecationWarning` and will be removed in a future release.
52+
- `ObstoreReader` has been removed from `virtualizarr.utils`. This should not break user's code, as it was not part of the public/documented API. See [obspec_utils](https://obspec-utils.readthedocs.io/en/latest/api/obspec/) for public file handlers.
53+
- Added `obspec_utils>=0.7.0` as a required dependency. This package provides the `ObjectStoreRegistry` that was previously part of VirtualiZarr.
54+
- Minimum required version of `obstore` is now `0.7.0` (previously `0.5.1`). This was the first release to implement obspec protocols.
5055

5156
## v2.3.0 (20th January 2026)
5257

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ exclude_lines = [
301301
]
302302

303303
[tool.pytest.ini_options]
304+
addopts = ["--doctest-modules", "--ignore=examples"]
304305
# See https://pytest-asyncio.readthedocs.io/en/latest/concepts.html#asyncio-event-loops
305306
# Explicitly set asyncio_default_fixture_loop_scope to eliminate the following warning:
306307
#

virtualizarr/accessor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ def rename_paths(
249249
... filename = Path(old_local_path).name
250250
... return str(new_s3_bucket_url / filename)
251251
>>>
252-
>>> ds.vz.rename_paths(local_to_s3_url)
252+
>>> ds.vz.rename_paths(local_to_s3_url) # doctest: +SKIP
253253
"""
254254

255255
new_ds = self.ds.copy()

virtualizarr/manifests/array.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ def rename_paths(
266266
... filename = Path(old_local_path).name
267267
... return str(new_s3_bucket_url / filename)
268268
>>>
269-
>>> marr.rename_paths(local_to_s3_url)
269+
>>> marr.rename_paths(local_to_s3_url) # doctest: +SKIP
270270
"""
271271
renamed_manifest = self.manifest.rename_paths(new)
272272
return ManifestArray(metadata=self.metadata, chunkmanifest=renamed_manifest)

virtualizarr/manifests/manifest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ def rename_paths(
459459
... filename = Path(old_local_path).name
460460
... return str(new_s3_bucket_url / filename)
461461
>>>
462-
>>> manifest.rename_paths(local_to_s3_url)
462+
>>> manifest.rename_paths(local_to_s3_url) # doctest: +SKIP
463463
"""
464464
if isinstance(new, str):
465465
renamed_paths = np.full_like(self._paths, fill_value=new)

virtualizarr/parallel.py

Lines changed: 80 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
import inspect
2+
import multiprocessing as mp
23
import warnings
3-
from concurrent.futures import Executor, Future
4-
from typing import Any, Callable, Iterable, Iterator, Literal, TypeVar
4+
from concurrent.futures import Executor, Future, ProcessPoolExecutor
5+
from functools import partial
6+
from typing import (
7+
Any,
8+
Callable,
9+
Generic,
10+
Iterable,
11+
Iterator,
12+
Literal,
13+
ParamSpec,
14+
TypeVar,
15+
)
516

617
__all__ = [
718
"SerialExecutor",
@@ -15,28 +26,38 @@
1526
# TODO lithops should just not require a special wrapper class, see https://github.com/lithops-cloud/lithops/issues/1427
1627

1728

29+
P = ParamSpec("P")
1830
# Type variable for return type
1931
T = TypeVar("T")
2032

2133

2234
def get_executor(
23-
parallel: Literal["dask", "lithops"] | type[Executor] | Literal[False],
24-
) -> type[Executor]:
25-
"""Get an executor that follows the concurrent.futures.Executor ABC API."""
35+
parallel: Literal["dask", "lithops", False] | type[Executor],
36+
) -> Callable[..., Executor]:
37+
"""Get a callable with a return type that follows the concurrent.futures.Executor ABC API."""
2638

2739
if parallel == "dask":
2840
return DaskDelayedExecutor
29-
elif parallel == "lithops":
41+
if parallel == "lithops":
3042
return LithopsEagerFunctionExecutor
31-
elif parallel is False:
43+
if parallel is False:
3244
return SerialExecutor
33-
elif inspect.isclass(parallel) and issubclass(parallel, Executor):
45+
if parallel is ProcessPoolExecutor:
46+
# TODO Once we drop support for python <3.14, we can remove this context
47+
# dance because from 3.14 onward, POSIX defaults to "forkserver" rather
48+
# than "fork".
49+
method = mp.get_context().get_start_method()
50+
context = mp.get_context("forkserver" if method == "fork" else method)
51+
return partial(ProcessPoolExecutor, mp_context=context)
52+
if inspect.isclass(parallel) and issubclass(parallel, Executor):
3453
return parallel
35-
else:
36-
raise ValueError(
37-
f"Unrecognized argument to ``parallel``: {parallel}"
38-
"Please supply either ``'dask'``, ``'lithops'``, ``False``, or a concrete subclass of ``concurrent.futures.Executor``."
39-
)
54+
55+
raise ValueError(
56+
f"Invalid value for `parallel`: {parallel}. Please supply "
57+
"either the string 'dask' or 'lithops', or a concrete subclass of "
58+
"concurrent.futures.Executor. To obtain a serial executor, specify "
59+
"the boolean value `False`."
60+
)
4061

4162

4263
class SerialExecutor(Executor):
@@ -111,18 +132,6 @@ def map(
111132
"""
112133
return map(fn, *iterables)
113134

114-
def shutdown(self, wait: bool = True, *, cancel_futures: bool = False) -> None:
115-
"""
116-
Shutdown the executor.
117-
118-
Parameters
119-
----------
120-
wait
121-
Whether to wait for pending futures (always True for serial executor)
122-
"""
123-
# In a serial executor, shutdown is a no-op
124-
pass
125-
126135

127136
class DaskDelayedExecutor(Executor):
128137
"""
@@ -211,31 +220,48 @@ def map(
211220
# Compute all tasks
212221
return iter(dask.compute(*delayed_tasks))
213222

214-
def shutdown(self, wait: bool = True, *, cancel_futures: bool = False) -> None:
215-
"""
216-
Shutdown the executor
217-
218-
Parameters
219-
----------
220-
wait
221-
Whether to wait for pending futures (always True for serial executor))
222-
"""
223-
# For Dask.delayed, shutdown is essentially a no-op
224-
pass
225-
226223

227224
class LithopsEagerFunctionExecutor(Executor):
228225
"""
229-
Lithops-based function executor which follows the [concurrent.futures.Executor][] API.
226+
Lithops-based function executor that follows the [concurrent.futures.Executor][] API.
230227
231-
Only required because lithops doesn't follow the [concurrent.futures.Executor][] API, see https://github.com/lithops-cloud/lithops/issues/1427.
228+
Only required because lithops doesn't follow the [concurrent.futures.Executor][] API.
229+
See https://github.com/lithops-cloud/lithops/issues/1427.
232230
"""
233231

232+
class compatible_callable(Generic[P, T]):
233+
"""Wraps a callable to make it fully compatible with Lithops.
234+
235+
This wrapper deals with 2 oddities in Lithops:
236+
237+
1. Use of `functools.partial`, which Lithops fails to recognize as being
238+
callable. This is likely due to the builtin `partial` class using
239+
slots, which causes Lithops to not recognize the `__call__` method as
240+
a method. See https://github.com/lithops-cloud/lithops/issues/1428.
241+
2. Use of generic function wrappers that define generic `args` and
242+
`kwargs` parameters. In this case, because of the way Lithops
243+
inspects function signatures to determine how to pass arguments, it
244+
does not properly "spread" arguments as normally expected. Instead,
245+
it collects all positional arguments and associates them with the
246+
keyword argument `"args"`, which is utterly unhelpful.
247+
"""
248+
249+
def __init__(self, f: Callable[P, T]):
250+
self.f = f
251+
252+
def __call__(self, *args: P.args, **kwargs: P.kwargs) -> T:
253+
if not args and "args" in kwargs:
254+
# Lithops collected all positional args into an "args" kwarg,
255+
# so we're undoing that nonsense here.
256+
args = kwargs.pop("args", ()) # type: ignore
257+
258+
return self.f(*args, **kwargs)
259+
234260
def __init__(self, **kwargs) -> None:
235261
import lithops # type: ignore[import-untyped]
236262

237263
# Create Lithops client with optional configuration
238-
self.lithops_client = lithops.FunctionExecutor(**kwargs)
264+
self.lithops_client = lithops.FunctionExecutor(**kwargs).__enter__()
239265

240266
# Track submitted futures
241267
self._futures: list[Future] = []
@@ -263,7 +289,11 @@ def submit(self, fn: Callable[..., T], /, *args: Any, **kwargs: Any) -> Future[T
263289

264290
try:
265291
# Submit to Lithops
266-
lithops_future = self.lithops_client.call_async(fn, *args, **kwargs)
292+
lithops_future = self.lithops_client.call_async(
293+
LithopsEagerFunctionExecutor.compatible_callable(fn),
294+
*args,
295+
**kwargs,
296+
)
267297

268298
# Add a callback to set the result or exception
269299
def _on_done(lithops_result):
@@ -294,7 +324,8 @@ def map(
294324
"""
295325
Apply a function to an iterable using lithops.
296326
297-
Only needed because [lithops.executors.FunctionExecutor.map][lithops.executors.FunctionExecutor.map] returns futures, unlike [concurrent.futures.Executor.map][].
327+
Only needed because [lithops.executors.FunctionExecutor.map][lithops.executors.FunctionExecutor.map]
328+
returns futures, unlike [concurrent.futures.Executor.map][].
298329
299330
Parameters
300331
----------
@@ -309,14 +340,13 @@ def map(
309340
-------
310341
Generator of results
311342
"""
312-
import lithops # type: ignore[import-untyped]
313-
314-
fexec = lithops.FunctionExecutor()
315-
316-
futures = fexec.map(fn, *iterables)
317-
results = fexec.get_result(futures)
343+
fexec = self.lithops_client
344+
futures = fexec.map(
345+
LithopsEagerFunctionExecutor.compatible_callable(fn),
346+
list(zip(*iterables)),
347+
)
318348

319-
return results
349+
return fexec.get_result(futures) # type: ignore
320350

321351
def shutdown(self, wait: bool = True, *, cancel_futures: bool = False) -> None:
322352
"""
@@ -327,5 +357,4 @@ def shutdown(self, wait: bool = True, *, cancel_futures: bool = False) -> None:
327357
wait
328358
Whether to wait for pending futures.
329359
"""
330-
# Should this call lithops .clean() method?
331-
pass
360+
self.lithops_client.__exit__(None, None, None)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import multiprocessing as mp
2+
3+
import pytest
4+
5+
from virtualizarr.parallel import LithopsEagerFunctionExecutor, get_executor
6+
from virtualizarr.tests import requires_lithops
7+
8+
9+
@requires_lithops
10+
def test_lithops_executor_with_multiple_args():
11+
with LithopsEagerFunctionExecutor() as exec:
12+
results = exec.map(lambda x, y: x + y, (1, 2, 3), (4, 5, 6))
13+
14+
assert tuple(results) == (5, 7, 9)
15+
16+
17+
@requires_lithops
18+
def test_lithops_executor_with_partial():
19+
from functools import partial
20+
21+
inc = partial(lambda x, y: x + y, 1)
22+
23+
with LithopsEagerFunctionExecutor() as exec:
24+
results = exec.map(inc, (1, 2, 3))
25+
26+
assert tuple(results) == (2, 3, 4)
27+
28+
29+
@pytest.mark.skipif(
30+
mp.get_start_method() != "fork",
31+
reason="Default multiprocessing start method is not 'fork'",
32+
)
33+
def test_get_executor_process_pool_mode():
34+
from concurrent.futures import ProcessPoolExecutor
35+
36+
executor = get_executor(ProcessPoolExecutor)()
37+
38+
assert isinstance(executor, ProcessPoolExecutor), "Expected a ProcessPoolExecutor"
39+
40+
ctx = executor._mp_context
41+
42+
assert ctx is not None, "Expected executor to have a multiprocessing context"
43+
assert ctx.get_start_method() == "forkserver"

virtualizarr/tests/test_parsers/test_dmrpp.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import platform
12
import textwrap
23
from contextlib import nullcontext
34
from pathlib import Path
@@ -486,6 +487,10 @@ def test_split_groups(hdf5_groups_file, group_path):
486487
assert result_tags == expected_tags
487488

488489

490+
@pytest.mark.xfail(
491+
platform.system() == "Linux",
492+
reason="See https://github.com/zarr-developers/VirtualiZarr/issues/904.",
493+
)
489494
@pytest.mark.parametrize(
490495
"group,warns",
491496
[

0 commit comments

Comments
 (0)