I am trying to use the FuturesExecutor and have run into a few problems. I've enumerated them in the order that I came across them. I have opened a PR (#3836) for issues 1, 2, 4, and 5.

Issues 3 and 6 are more nuanced so hopefully someone can take a look at those.

Installation

I've installed the latest ndcctools as follows:

conda create -p $PWD/test-env python=3.11
conda install -y -c conda-forge ndcctools

Issues

1. None return type in FuturesExecutor.submit()

The first example in the docs does not work (https://cctools.readthedocs.io/en/stable/taskvine/#futures).

import ndcctools.taskvine as vine

def my_sum(x, y):
    return x + y

m = vine.FuturesExecutor(manager_name='my_manager')

a = m.submit(my_sum, 3, 4)
b = m.submit(my_sum, 5, 2)
c = m.submit(my_sum, a, b)  # note that the futures a and b are
                            # a passed as any other argument.

print(c.result())

$ python t.py
Traceback (most recent call last):
  File "/home/jgpaul/workspace/taskvine/t.py", line 13, in <module>
    print(c.result())
          ^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'result'

This is because Futures.Executor.submit() is missing a return statement if the two isinstance checks fail:

The second example works correctly because a FuturePythonTask is created manually, but this (and another) of the examples in the docs have a typo and use FutureExecutor instead of FuturesExecutor.

2. Adding a callback to VineFuture fails

This should be _callback_fns:

3. Printing a VineFuture fails

While trying to print the futures returned by FuturesExecutor.submit(), you get the following error:

  File ".../lib/python3.11/concurrent/futures/_base.py", line 345, in __repr__
    with self._condition:
         ^^^^^^^^^^^^^^^
AttributeError: 'VineFuture' object has no attribute '_condition'

This is because VineFuture subclasses concurrent.futures.Future but does not call super().__init__() which initialized some attributes of concurrent.futures.Future used in concurrent.futures.Future.__repr__.

This also means that VineFuture is not compatible with any functions that operate on Future (i.e., wait or as_completed), but that is maybe a more nuanced design decision.

4. VineFuture.result(timeout=None) does not work

The concurrent.futures.Future specification describes that timeout=None says there is no limit to the wait time.

It seems FineFuture uses "wait_forever" instead to signal no wait time limit:

This makes it annoying to use FuturesExecutor in the same code base that may use other concurrent.futures.Executor implementations. It would be easy to translate None -> "wait_forever" within result.

5. Future callback signature is different

concurrent.futures.Future.add_done_callback(fn) says that "fn will be called, with the future as its only argument".

However, FuturesExecutor calls the callback with the task's output:

This is another deviation from the concurrent.futures.Executor spec, and the callback can still access the result from the future if the result is the only thing that is needed.

6. VineFuture callbacks invoked when retrieving result

This is another deviation from the behavior of concurrent.futures.Future. concurrent.futures.Future invokes its callbacks when one of the following happens: a task is cancelled, a result is set, or an exception is set.

VineFuture, instead, invokes the callbacks when VineFuture.result() is called. It caches the result so later calls VineFuture.result() return early and do not invoke the callbacks. However, if bug (5) is fixed and the callback accesses the result of the future, the callback will be invoked twice (or infinitely in some cases).

For example, this prints "callback!" indefinitely until a RecursionError is raised.

import ndcctools.taskvine as vine

def my_sum(x, y):
    return x + y

def callback(future):
    print('callback!')
    future.result()

m = vine.FuturesExecutor(manager_name='my_manager')

a = m.submit(my_sum, 3, 4)
a.add_done_callback(callback)

print(a.result())

I'm not sure what the best way to fix this is, but ideally invoking callbacks would be independent of the client code calling VineFuture.result(). Otherwise, that sort of defeats the value of callbacks.