From 6877e36dfa33ce2d24aacee62754317c5ae2189a Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 11:54:52 +0100 Subject: [PATCH 01/28] Add StepsFuture implementation --- traits_futures/background_steps.py | 387 +++++++++++++++++++++++++++++ 1 file changed, 387 insertions(+) create mode 100644 traits_futures/background_steps.py diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py new file mode 100644 index 00000000..99da653d --- /dev/null +++ b/traits_futures/background_steps.py @@ -0,0 +1,387 @@ +# Enthought product code +# +# (C) Copyright 2013-2021 Enthought, Inc., Austin, TX +# All rights reserved. +# +# This file and its contents are confidential information and NOT open source. +# Distribution is prohibited. + +""" +Background task for an interruptible progress-reporting callable. + +This module defines a Traits Futures task specification and a corresponding +future for tasks that execute in the background and report progress information +to the foreground. The points at which progress is reported also represent +points at which the task can be interrupted. + +""" + +from abc import ABC + +from traits.api import ( + Callable, + Dict, + HasStrictTraits, + Instance, + Int, + Str, + Tuple, +) + +from traits_futures.api import ( + BaseFuture, + CANCELLED, + COMPLETED, + FAILED, + ITaskSpecification, +) + + +class TaskCancelled(Exception): + """The user has cancelled this background task.""" + + +class IStepsReporter(ABC): + """Communicate progress information about a background job.""" + + def start(self, message=None, steps=-1): + """Start reporting progress. + + Parameters + ---------- + message : str, optional + A description for the job at the start. + steps : int, optional + The number of steps this job will perform. By default, + this job has no known number of steps. Any UI + representation will just show that work is being done + without making any claims about quantitative progress. + + Raises + ------ + TaskCancelled + If the user has called ``cancel()`` before this. + """ + + def step(self, message=None, step=None, steps=None): + """Emit a step event. + + Parameters + ---------- + message : str, optional + A description of what is currently being done, replacing the + current message. + step : int, optional + The step number. If omitted, an internal count will be + incremented by one. + steps : int, optional + The new total number of steps, if changed. + + Raises + ------ + TaskCancelled + If the user has called ``cancel()`` before this. + """ + + +# Message types for messages sent from the background to the future. + +#: Message sent on a start or step operation. Argument is a tuple +#: (step, steps, message), with: +#: step: int or None - number of completed steps so far +#: steps: int or None - total number of steps, if known +#: message: str or None - message to display for this step +STEP = "step" + + +@IStepsReporter.register +class StepsReporter(HasStrictTraits): + """ + Object used by the background task to report progress information. + + A :class:`StepsReporter` instance is passed to the background task, + and its ``start`` and ``step`` methods can be used by that + background task to report progress. + + """ + + def start(self, message=None, steps=-1): + """Start reporting progress. + + Parameters + ---------- + message : str, optional + A description for the task at the start. + steps : int, optional + The number of steps this task will perform. By default, + this task has no known number of steps. Any UI + representation will just show that work is being done + without making any claims about quantitative progress. + + Raises + ------ + TaskCancelled + If the user has called ``cancel()`` before this. + """ + self._check_cancel() + self._send(STEP, (0, steps, message)) + + def step(self, message=None, step=None, steps=None): + """Emit a step event. + + Parameters + ---------- + message : str, optional + A description of what is currently being done, replacing the + current message. + step : int, optional + The step number. If omitted, an internal count will be + incremented by one. + steps : int, optional + The new total number of steps, if changed. + + Raises + ------ + TaskCancelled + If the user has called ``cancel()`` before this. + """ + self._check_cancel() + self._send(STEP, (step, steps, message)) + + # Private traits ########################################################## + + #: Hook to send messages to the foreground. In normal use, this is provided + #: by the Traits Futures machinery. + _send = Callable() + + #: Callable to check whether the task has been cancelled, provided + #: by the Traits Futures machinery. + _cancelled = Callable() + + # Private methods ######################################################### + + def _check_cancel(self): + """Check if the task has been cancelled. + + Raises + ------ + TaskCancelled + If the task has been cancelled. + """ + if self._cancelled(): + raise TaskCancelled("Cancellation requested via the future") + + +class StepsBackgroundTask: + """ + Wrapper around the actual callable to be run. + + This wrapper handles capturing exceptions and sending the final status of + the task on completion. + """ + + def __init__(self, callable, args, kwargs): + self.callable = callable + self.args = args + self.kwargs = kwargs + + def __call__(self, send, cancelled): + progress = StepsReporter(_send=send, _cancelled=cancelled) + try: + result = self.callable( + *self.args, **self.kwargs, progress=progress + ) + except TaskCancelled: + return None + else: + return result + + +class StepsFuture(BaseFuture): + """ + Object representing the front-end handle to a background call. + """ + + #: Total number of steps. + #: A value of -1 implies that the number of steps is unknown. + steps = Int(-1) + + #: The most recently completed step. + step = Int(0) + + #: Most recently received message from either the background task + #: or from cancellation. + message = Str() + + @property + def error(self): + """ + The exception raised by the background task, if any. + + This attribute is only available if the state of this future is + ``FAILED``. If the future has not reached the ``FAILED`` state, any + attempt to access this attribute will raise an ``AttributeError.`` + + Returns + ------- + exception: BaseException + The exception object raised by the background task. + + Raises + ------ + AttributeError + If the task is still executing, or was cancelled, or completed + without raising an exception. + """ + if self.state != FAILED: + raise AttributeError( + "No exception information available. Task has " + "not yet completed, or was cancelled, or completed " + "without an exception. " + "Task state is {}".format(self.state) + ) + return self._error + + def outcome(self): + """ + Either returns the result of the background task, or + re-raises the exception that the background task raised. + + Raises + ------ + RuntimeError + If the background task has not yet completed. + """ + if self.state == COMPLETED: + return self.result + elif self.state == FAILED: + raise self.error + elif self.state == CANCELLED: + raise TaskCancelled("Task was cancelled") + else: + raise RuntimeError( + "Task has not completed - no outcome is available." + ) + + # Private traits ########################################################## + + #: Any exception raised by the background task. + _error = Instance(BaseException, allow_none=True) + + # Private methods ######################################################### + + def _process_step(self, step_event): + """ + Process a STEP message from the background task. + """ + step, steps, message = step_event + if message is not None: + self.message = message + if steps is not None: + self.steps = steps + if step is None: + self.step += 1 + else: + self.step = step + + def _process_error(self, error): + """ + Process an ERROR message from the background task. + """ + self._error = error + + +@ITaskSpecification.register +class BackgroundSteps(HasStrictTraits): + """ + Object representing the background call to be executed. + + The *callable* function will be called with an additional + argument, passed via the name "progress". The object passed + can then be used to submit progress information to the foreground. + + Parameters + ---------- + callable : callable + The callable to be executed in the background. + args : tuple + Positional arguments to be passed to *callable*. + kwargs : mapping + Keyword arguments to be passed to *callable*. + """ + + #: The callable for the task. + callable = Callable() + + #: Positional arguments to be passed to the callable. + args = Tuple() + + #: Named arguments to be passed to the callable, excluding the "progress" + #: named argument. The "progress" argument will be supplied through the + #: execution machinery. + kwargs = Dict(Str()) + + # --- ITaskSpecification implementation ----------------------------------- + + def future(self): + """ + Return a Future for the background task. + + Returns + ------- + future : ProgressFuture + Future object that can be used to monitor the status of the + background task. + """ + return StepsFuture() + + def background_task(self): + """ + Return a background callable for this task specification. + + Returns + ------- + collections.abc.Callable + Callable accepting arguments ``send`` and ``cancelled``. The + callable can use ``send`` to send messages and ``cancelled`` to + check whether cancellation has been requested. + """ + return StepsBackgroundTask( + callable=self.callable, + args=self.args, + kwargs=self.kwargs.copy(), + ) + + +def submit_steps(executor, callable, *args, **kwargs): + """ + Convenience function to submit a background progress task to an executor. + + Parameters + ---------- + executor : TraitsExecutor + Executor to submit the task to. + callable : collections.abc.Callable + Callable to execute in the background. This should accept a + "progress" keyword argument, in addition to any other positional + and named arguments it needs. When the callable is invoked, an + instance of "StepsReporter" will be supplied via that "progress" + argument. + *args + Positional arguments to pass to the callable. + **kwargs + Named arguments to pass to the callable, excluding the "progress" + argument. That argument will be passed separately. + + Returns + ------- + future : StepsFuture + Object representing the state of the background call. + """ + return executor.submit( + BackgroundSteps( + callable=callable, + args=args, + kwargs=kwargs, + ) + ) From 6df53d359a19d82da879a6cadc8a50913fe60695 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 14:35:49 +0100 Subject: [PATCH 02/28] Add to traits_futures.api, rename the exception --- traits_futures/api.py | 11 +++++++ traits_futures/background_steps.py | 47 +++++++----------------------- traits_futures/tests/test_api.py | 3 ++ 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/traits_futures/api.py b/traits_futures/api.py index c3adde6e..555c951b 100644 --- a/traits_futures/api.py +++ b/traits_futures/api.py @@ -29,6 +29,8 @@ - :func:`~.submit_iteration` - :func:`~.submit_progress` - :exc:`~.ProgressCancelled` +- :func:`~.submit_steps` +- :exc:`~.StepsCancelled` Types of futures ---------------- @@ -37,6 +39,7 @@ - :class:`~.CallFuture` - :class:`~.IterationFuture` - :class:`~.ProgressFuture` +- :class:`~.StepsFuture` Future states ------------- @@ -89,6 +92,11 @@ ProgressFuture, submit_progress, ) +from traits_futures.background_steps import ( + StepsCancelled, + StepsFuture, + submit_steps, +) from traits_futures.base_future import BaseFuture from traits_futures.ets_event_loop import ETSEventLoop from traits_futures.executor_states import ( @@ -121,6 +129,8 @@ "IterationFuture", "ProgressFuture", "ProgressCancelled", + "StepsFuture", + "StepsCancelled", # Future states "FutureState", "CANCELLED", @@ -140,6 +150,7 @@ "submit_call", "submit_iteration", "submit_progress", + "submit_steps", # Support for creating new task types "BaseFuture", "ITaskSpecification", diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 99da653d..6f155d2b 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -28,16 +28,12 @@ Tuple, ) -from traits_futures.api import ( - BaseFuture, - CANCELLED, - COMPLETED, - FAILED, - ITaskSpecification, -) +from traits_futures.base_future import BaseFuture +from traits_futures.future_states import FAILED +from traits_futures.i_task_specification import ITaskSpecification -class TaskCancelled(Exception): +class StepsCancelled(Exception): """The user has cancelled this background task.""" @@ -59,7 +55,7 @@ def start(self, message=None, steps=-1): Raises ------ - TaskCancelled + StepsCancelled If the user has called ``cancel()`` before this. """ @@ -79,7 +75,7 @@ def step(self, message=None, step=None, steps=None): Raises ------ - TaskCancelled + StepsCancelled If the user has called ``cancel()`` before this. """ @@ -120,7 +116,7 @@ def start(self, message=None, steps=-1): Raises ------ - TaskCancelled + StepsCancelled If the user has called ``cancel()`` before this. """ self._check_cancel() @@ -142,7 +138,7 @@ def step(self, message=None, step=None, steps=None): Raises ------ - TaskCancelled + StepsCancelled If the user has called ``cancel()`` before this. """ self._check_cancel() @@ -165,11 +161,11 @@ def _check_cancel(self): Raises ------ - TaskCancelled + StepsCancelled If the task has been cancelled. """ if self._cancelled(): - raise TaskCancelled("Cancellation requested via the future") + raise StepsCancelled("Cancellation requested via the future") class StepsBackgroundTask: @@ -191,7 +187,7 @@ def __call__(self, send, cancelled): result = self.callable( *self.args, **self.kwargs, progress=progress ) - except TaskCancelled: + except StepsCancelled: return None else: return result @@ -242,27 +238,6 @@ def error(self): ) return self._error - def outcome(self): - """ - Either returns the result of the background task, or - re-raises the exception that the background task raised. - - Raises - ------ - RuntimeError - If the background task has not yet completed. - """ - if self.state == COMPLETED: - return self.result - elif self.state == FAILED: - raise self.error - elif self.state == CANCELLED: - raise TaskCancelled("Task was cancelled") - else: - raise RuntimeError( - "Task has not completed - no outcome is available." - ) - # Private traits ########################################################## #: Any exception raised by the background task. diff --git a/traits_futures/tests/test_api.py b/traits_futures/tests/test_api.py index a649d0b2..1f494f68 100644 --- a/traits_futures/tests/test_api.py +++ b/traits_futures/tests/test_api.py @@ -35,11 +35,14 @@ def test_imports(self): ProgressCancelled, ProgressFuture, RUNNING, + StepsCancelled, + StepsFuture, STOPPED, STOPPING, submit_call, submit_iteration, submit_progress, + submit_steps, TraitsExecutor, WAITING, ) From 2f4dd45f3b7f19a9592ec5a2aa623d945c892eff Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 14:35:55 +0100 Subject: [PATCH 03/28] Add example scripts --- .../examples/background_progress_dialog.py | 140 ++++++++++++++++++ .../source/guide/examples/progress_dialogs.py | 83 +++++++++++ 2 files changed, 223 insertions(+) create mode 100644 docs/source/guide/examples/background_progress_dialog.py create mode 100644 docs/source/guide/examples/progress_dialogs.py diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py new file mode 100644 index 00000000..e4f76a1b --- /dev/null +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -0,0 +1,140 @@ +# Enthought product code +# +# (C) Copyright 2012-2021 Enthought, Inc., Austin, TX +# All rights reserved. +# +# This file and its contents are confidential information and NOT open source. +# Distribution is prohibited. + +""" Qt implementation of a Pyface Dialog that listens to +a ProgressFuture. +""" + +from pyface.api import Dialog +from pyface.qt import QtCore, QtGui +from traits.api import Any, Bool, Instance, Int, on_trait_change +from traits_futures.api import EXECUTING, StepsFuture + + +# XXX Fix behaviour on dialog close button. Should match pressing the +# "cancelling" button. (What do users want?) +# Similarly for doing a Ctrl-C + + +class ProgressFutureDialog(Dialog): + """Show a cancellable progress dialog listening to a progress manager.""" + + #: Text to show for cancellation label. + cancel_label = "Cancel" + + #: Text to show for the ok label. (Not used here.) + ok_label = "" + + #: Whether to show a 'Cancel' button or not. + cancellable = Bool(True) + + #: The maximum number of steps. + maximum = Int(0) + + #: Whether to show the percentage complete or not. + show_percent = Bool(True) + + #: The traited ``Future`` representing the state of the background call. + progress_future = Instance(StepsFuture) + + def cancel(self): + """Cancel the job. + + Users of the dialog should call this instead of reaching + down to the progress manager since this method will prevent + the job from starting if it has not already. + """ + self.progress_future.cancel() + self._cancel_button_control.setEnabled(False) + + # Private implementation ################################################## + + _progress_bar = Any() + _message_control = Any() + _cancel_button_control = Any() + + def _create_contents(self, parent): + layout = QtGui.QVBoxLayout() + + if not self.resizeable: + layout.setSizeConstraint(QtGui.QLayout.SetFixedSize) + + layout.addWidget(self._create_message(parent, layout)) + layout.addWidget(self._create_gauge(parent, layout)) + if self.cancellable: + layout.addWidget(self._create_buttons(parent)) + + parent.setLayout(layout) + + def _create_buttons(self, parent): + buttons = QtGui.QDialogButtonBox() + + # 'Cancel' button. + btn = buttons.addButton( + self.cancel_label, QtGui.QDialogButtonBox.RejectRole + ) + btn.setDefault(True) + # The QueuedConnection here appears to be necessary to avoid a + # Qt/macOS bug where the dialog widgets are only partially updated + # after a button click. + # xref: https://github.com/enthought/traitsui/issues/1308 + # xref: https://bugreports.qt.io/browse/QTBUG-68067 + buttons.rejected.connect(self.cancel, type=QtCore.Qt.QueuedConnection) + self._cancel_button_control = btn + return buttons + + def _create_message(self, dialog, layout): + self._message_control = QtGui.QLabel( + self.progress_future.message, dialog + ) + self._message_control.setAlignment( + QtCore.Qt.AlignTop | QtCore.Qt.AlignLeft + ) + return self._message_control + + def _create_gauge(self, dialog, layout): + self._progress_bar = QtGui.QProgressBar(dialog) + self._progress_bar.setRange(0, self.maximum) + self._progress_bar.setValue(self.progress_future.step) + if self.show_percent: + self._progress_bar.setFormat("%p%") + else: + self._progress_bar.setFormat("%v") + return self._progress_bar + + @on_trait_change("closing") + def _destroy_traits_on_dialog_closing(self): + self._message_control = None + self._progress_bar = None + self._cancel_button_control = None + + @on_trait_change("maximum") + def _update_max_on_progress_bar(self, maximum): + if self._progress_bar is not None: + self._progress_bar.setRange(0, maximum) + + @on_trait_change("progress_future:[message,state]") + def _update_message(self, future, name, new): + if future.state == EXECUTING and future.message != "": + message = f"{future.state}: {future.message}" + else: + message = f"{future.state}" + self._message_control.setText(message) + + @on_trait_change("progress_future:steps") + def _update_maximum(self, steps): + self.maximum = max(steps, 0) + + @on_trait_change("progress_future:step") + def _update_value(self, step): + if self._progress_bar is not None: + self._progress_bar.setValue(step) + + @on_trait_change("progress_future:done") + def _on_end(self, event): + self.close() diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py new file mode 100644 index 00000000..e3a30769 --- /dev/null +++ b/docs/source/guide/examples/progress_dialogs.py @@ -0,0 +1,83 @@ +""" +Demo script for modal progress dialog. +""" + +# import pyface.qt.QtCore + +# import atexit +import concurrent.futures +import time + +from traits.api import Any, Button, HasStrictTraits, Instance, List, Range +from traits_futures.api import TraitsExecutor, submit_steps +from traitsui.api import Item, View + +from background_progress_dialog import ProgressFutureDialog + + +def count(target, *, sleep=1.0, progress): + progress.start("counting", steps=target) + for i in range(target): + progress.step(f"step {i} of {target}", step=i) + time.sleep(sleep) + + +class MyView(HasStrictTraits): + + calculate = Button() + + nonmodal = Button() + + target = Range(0, 100, 10) + + executor = Instance(TraitsExecutor) + + def _executor_default(self): + return TraitsExecutor() # worker_pool=self.cf_executor) + + dialogs = List(ProgressFutureDialog) + + cf_executor = Any() + + def _calculate_fired(self): + target = self.target + future = submit_steps(self.executor, count, target=target) + dialog = ProgressFutureDialog( + progress_future=future, + style="modal", # this is the default + title=f"Counting to {target}", + ) + dialog.open() + + def _nonmodal_fired(self): + target = self.target + future = submit_steps(self.executor, count, target) + dialog = ProgressFutureDialog( + progress_future=future, + style="nonmodal", # this is the default + title=f"Counting to {target}", + ) + dialog.open() + # Keep a reference to open dialogs, else they'll + # mysteriously disappear. A better solution might be to + # parent them appropriately. + self.dialogs.append(dialog) + + view = View( + Item("calculate", label="Count modal"), + Item("nonmodal", label="Count nonmodal"), + Item("target"), + ) + + +def main(): + with concurrent.futures.ThreadPoolExecutor() as executor: + view = MyView(cf_executor=executor) + view.configure_traits() + + +if __name__ == "__main__": + print("Starting main") + main() + print("Left main") + print("Shutting down interpreter") From 04540d43fbcac35547c341b7a7c97968fcc3f7e4 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 14:44:24 +0100 Subject: [PATCH 04/28] Fix copyright headers --- .../guide/examples/background_progress_dialog.py | 12 +++++++----- docs/source/guide/examples/progress_dialogs.py | 10 ++++++++++ traits_futures/background_steps.py | 12 +++++++----- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py index e4f76a1b..58cc942e 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -1,10 +1,12 @@ -# Enthought product code -# -# (C) Copyright 2012-2021 Enthought, Inc., Austin, TX +# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX # All rights reserved. # -# This file and its contents are confidential information and NOT open source. -# Distribution is prohibited. +# This software is provided without warranty under the terms of the BSD +# license included in LICENSE.txt and may be redistributed only under +# the conditions described in the aforementioned license. The license +# is also available online at http://www.enthought.com/licenses/BSD.txt +# +# Thanks for using Enthought open source! """ Qt implementation of a Pyface Dialog that listens to a ProgressFuture. diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py index e3a30769..e26d001b 100644 --- a/docs/source/guide/examples/progress_dialogs.py +++ b/docs/source/guide/examples/progress_dialogs.py @@ -1,3 +1,13 @@ +# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX +# All rights reserved. +# +# This software is provided without warranty under the terms of the BSD +# license included in LICENSE.txt and may be redistributed only under +# the conditions described in the aforementioned license. The license +# is also available online at http://www.enthought.com/licenses/BSD.txt +# +# Thanks for using Enthought open source! + """ Demo script for modal progress dialog. """ diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 6f155d2b..aa67f8b7 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -1,10 +1,12 @@ -# Enthought product code -# -# (C) Copyright 2013-2021 Enthought, Inc., Austin, TX +# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX # All rights reserved. # -# This file and its contents are confidential information and NOT open source. -# Distribution is prohibited. +# This software is provided without warranty under the terms of the BSD +# license included in LICENSE.txt and may be redistributed only under +# the conditions described in the aforementioned license. The license +# is also available online at http://www.enthought.com/licenses/BSD.txt +# +# Thanks for using Enthought open source! """ Background task for an interruptible progress-reporting callable. From 54c718b78b3fadffd88a990a6798ae39c392dc26 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 14:45:40 +0100 Subject: [PATCH 05/28] Remove debugging code --- docs/source/guide/examples/progress_dialogs.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py index e26d001b..9529cfb3 100644 --- a/docs/source/guide/examples/progress_dialogs.py +++ b/docs/source/guide/examples/progress_dialogs.py @@ -12,9 +12,6 @@ Demo script for modal progress dialog. """ -# import pyface.qt.QtCore - -# import atexit import concurrent.futures import time @@ -87,7 +84,4 @@ def main(): if __name__ == "__main__": - print("Starting main") main() - print("Left main") - print("Shutting down interpreter") From 29a3fd7664e13c51db1a1a5f9f5fccc52eb94d00 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 14:48:03 +0100 Subject: [PATCH 06/28] Import sort order fixes --- docs/source/guide/examples/background_progress_dialog.py | 1 - docs/source/guide/examples/progress_dialogs.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py index 58cc942e..2e4c5408 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -17,7 +17,6 @@ from traits.api import Any, Bool, Instance, Int, on_trait_change from traits_futures.api import EXECUTING, StepsFuture - # XXX Fix behaviour on dialog close button. Should match pressing the # "cancelling" button. (What do users want?) # Similarly for doing a Ctrl-C diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py index 9529cfb3..32d7e23b 100644 --- a/docs/source/guide/examples/progress_dialogs.py +++ b/docs/source/guide/examples/progress_dialogs.py @@ -16,7 +16,7 @@ import time from traits.api import Any, Button, HasStrictTraits, Instance, List, Range -from traits_futures.api import TraitsExecutor, submit_steps +from traits_futures.api import submit_steps, TraitsExecutor from traitsui.api import Item, View from background_progress_dialog import ProgressFutureDialog From d3980046914edd03d921c8a94f47b223bd2ed408 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 15:04:03 +0100 Subject: [PATCH 07/28] Fix doc build(?) --- traits_futures/background_steps.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index aa67f8b7..0cc45207 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -86,9 +86,9 @@ def step(self, message=None, step=None, steps=None): #: Message sent on a start or step operation. Argument is a tuple #: (step, steps, message), with: -#: step: int or None - number of completed steps so far -#: steps: int or None - total number of steps, if known -#: message: str or None - message to display for this step +#: * step: int or None - number of completed steps so far +#: * steps: int or None - total number of steps, if known +#: * message: str or None - message to display for this step STEP = "step" From 0b81f2fae92aeb84bd44a749e9e03ecf10c4157a Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 15:10:54 +0100 Subject: [PATCH 08/28] Add IStepsReporter to the public API --- traits_futures/api.py | 3 +++ traits_futures/background_steps.py | 17 ++++++++++------- traits_futures/tests/test_api.py | 1 + 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/traits_futures/api.py b/traits_futures/api.py index 555c951b..4138a832 100644 --- a/traits_futures/api.py +++ b/traits_futures/api.py @@ -30,6 +30,7 @@ - :func:`~.submit_progress` - :exc:`~.ProgressCancelled` - :func:`~.submit_steps` +- :class:`~.IStepsReporter` - :exc:`~.StepsCancelled` Types of futures @@ -93,6 +94,7 @@ submit_progress, ) from traits_futures.background_steps import ( + IStepsReporter, StepsCancelled, StepsFuture, submit_steps, @@ -129,6 +131,7 @@ "IterationFuture", "ProgressFuture", "ProgressCancelled", + "IStepsReporter", "StepsFuture", "StepsCancelled", # Future states diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 0cc45207..3bd5a1ec 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -9,12 +9,12 @@ # Thanks for using Enthought open source! """ -Background task for an interruptible progress-reporting callable. +Support for an interruptible progress-reporting callable. -This module defines a Traits Futures task specification and a corresponding -future for tasks that execute in the background and report progress information -to the foreground. The points at which progress is reported also represent -points at which the task can be interrupted. +This module defines a task specification and a corresponding future for tasks +that execute in the background and report progress information to the +foreground. The points at which progress is reported also represent points at +which the task can be interrupted. """ @@ -36,11 +36,13 @@ class StepsCancelled(Exception): - """The user has cancelled this background task.""" + """ + Exception raised interally when the user cancels the background task. + """ class IStepsReporter(ABC): - """Communicate progress information about a background job.""" + """Interface for step-reporting object passed to the background job.""" def start(self, message=None, steps=-1): """Start reporting progress. @@ -86,6 +88,7 @@ def step(self, message=None, step=None, steps=None): #: Message sent on a start or step operation. Argument is a tuple #: (step, steps, message), with: +#: #: * step: int or None - number of completed steps so far #: * steps: int or None - total number of steps, if known #: * message: str or None - message to display for this step diff --git a/traits_futures/tests/test_api.py b/traits_futures/tests/test_api.py index 1f494f68..98f44dd7 100644 --- a/traits_futures/tests/test_api.py +++ b/traits_futures/tests/test_api.py @@ -28,6 +28,7 @@ def test_imports(self): IEventLoop, IFuture, IParallelContext, + IStepsReporter, ITaskSpecification, IterationFuture, MultiprocessingContext, From 5edc3ba2b81b260aba46c97eda0a19e73d76fe82 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 15:12:21 +0100 Subject: [PATCH 09/28] Adapt for changes to 'send' introduced in enthought/traits-futures#364 --- traits_futures/background_steps.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 3bd5a1ec..df82aa97 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -125,7 +125,7 @@ def start(self, message=None, steps=-1): If the user has called ``cancel()`` before this. """ self._check_cancel() - self._send(STEP, (0, steps, message)) + self._send((STEP, (0, steps, message))) def step(self, message=None, step=None, steps=None): """Emit a step event. @@ -147,7 +147,7 @@ def step(self, message=None, step=None, steps=None): If the user has called ``cancel()`` before this. """ self._check_cancel() - self._send(STEP, (step, steps, message)) + self._send((STEP, (step, steps, message))) # Private traits ########################################################## From ee046860dc3ad5c5cd088ea3833cbca336c7c465 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 15:23:02 +0100 Subject: [PATCH 10/28] Add some test machinery --- .../tests/background_steps_tests.py | 54 +++++++++++++++++++ traits_futures/tests/test_background_steps.py | 37 +++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 traits_futures/tests/background_steps_tests.py create mode 100644 traits_futures/tests/test_background_steps.py diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py new file mode 100644 index 00000000..1d7d5423 --- /dev/null +++ b/traits_futures/tests/background_steps_tests.py @@ -0,0 +1,54 @@ +# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX +# All rights reserved. +# +# This software is provided without warranty under the terms of the BSD +# license included in LICENSE.txt and may be redistributed only under +# the conditions described in the aforementioned license. The license +# is also available online at http://www.enthought.com/licenses/BSD.txt +# +# Thanks for using Enthought open source! + + +from traits_futures.api import IStepsReporter, submit_steps + + +def check_steps_reporter_interface(progress): + """ + Check that 'progress' has been declared to support the right interface. + """ + if not isinstance(progress, IStepsReporter): + raise RuntimeError() + else: + return True + + +class BackgroundStepsTests: + def test_progress_implements_i_steps_reporter(self): + future = submit_steps(self.executor, check_steps_reporter_interface) + self.wait_until_done(future) + + self.assertResult(future, True) + self.assertNoException(future) + + # Helper functions + + def halt_executor(self): + """ + Wait for the executor to stop. + """ + executor = self.executor + executor.stop() + self.run_until(executor, "stopped", lambda executor: executor.stopped) + del self.executor + + def wait_until_done(self, future): + self.run_until(future, "done", lambda future: future.done) + + # Assertions + + def assertResult(self, future, expected_result): + self.assertEqual(future.result, expected_result) + + def assertNoException(self, future): + with self.assertRaises(AttributeError): + future.exception diff --git a/traits_futures/tests/test_background_steps.py b/traits_futures/tests/test_background_steps.py new file mode 100644 index 00000000..d2d7e74d --- /dev/null +++ b/traits_futures/tests/test_background_steps.py @@ -0,0 +1,37 @@ +# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX +# All rights reserved. +# +# This software is provided without warranty under the terms of the BSD +# license included in LICENSE.txt and may be redistributed only under +# the conditions described in the aforementioned license. The license +# is also available online at http://www.enthought.com/licenses/BSD.txt +# +# Thanks for using Enthought open source! + +""" +Tests for the background steps task. +""" + + +import unittest + +from traits_futures.api import MultithreadingContext, TraitsExecutor +from traits_futures.testing.gui_test_assistant import GuiTestAssistant +from traits_futures.tests.background_steps_tests import BackgroundStepsTests + + +class TestBackgroundSteps( + GuiTestAssistant, BackgroundStepsTests, unittest.TestCase +): + def setUp(self): + GuiTestAssistant.setUp(self) + self._context = MultithreadingContext() + self.executor = TraitsExecutor( + context=self._context, + event_loop=self._event_loop, + ) + + def tearDown(self): + self.halt_executor() + self._context.close() + GuiTestAssistant.tearDown(self) From eec72cdae55156557efac7fec1e86db61338cf5f Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 15:27:23 +0100 Subject: [PATCH 11/28] Add tests for StepsFuture --- traits_futures/tests/test_background_steps.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/traits_futures/tests/test_background_steps.py b/traits_futures/tests/test_background_steps.py index d2d7e74d..9e3a6ea9 100644 --- a/traits_futures/tests/test_background_steps.py +++ b/traits_futures/tests/test_background_steps.py @@ -15,9 +15,14 @@ import unittest -from traits_futures.api import MultithreadingContext, TraitsExecutor +from traits_futures.api import ( + MultithreadingContext, + StepsFuture, + TraitsExecutor, +) from traits_futures.testing.gui_test_assistant import GuiTestAssistant from traits_futures.tests.background_steps_tests import BackgroundStepsTests +from traits_futures.tests.common_future_tests import CommonFutureTests class TestBackgroundSteps( @@ -35,3 +40,8 @@ def tearDown(self): self.halt_executor() self._context.close() GuiTestAssistant.tearDown(self) + + +class TestStepsFuture(CommonFutureTests, unittest.TestCase): + def setUp(self): + self.future_class = StepsFuture From 66a74491563f02eedef8693f2274a609cd17b1dc Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 5 Jul 2021 16:07:45 +0100 Subject: [PATCH 12/28] Stash --- traits_futures/background_steps.py | 62 +++---------------- .../tests/background_steps_tests.py | 33 +++++++++- 2 files changed, 42 insertions(+), 53 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index df82aa97..07e26907 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -18,20 +18,13 @@ """ +# XXX Consider renaming the 'progress' argument to 'reporter'. + from abc import ABC -from traits.api import ( - Callable, - Dict, - HasStrictTraits, - Instance, - Int, - Str, - Tuple, -) +from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple from traits_futures.base_future import BaseFuture -from traits_futures.future_states import FAILED from traits_futures.i_task_specification import ITaskSpecification @@ -44,7 +37,9 @@ class StepsCancelled(Exception): class IStepsReporter(ABC): """Interface for step-reporting object passed to the background job.""" - def start(self, message=None, steps=-1): + # XXX steps should have a default of None, not -1. The -1 should be + # used only in the dialog. + def start(self, message=None, steps=None): """Start reporting progress. Parameters @@ -106,7 +101,7 @@ class StepsReporter(HasStrictTraits): """ - def start(self, message=None, steps=-1): + def start(self, message=None, steps=None): """Start reporting progress. Parameters @@ -147,6 +142,9 @@ def step(self, message=None, step=None, steps=None): If the user has called ``cancel()`` before this. """ self._check_cancel() + if steps is None: + steps = -1 + self._send((STEP, (step, steps, message))) # Private traits ########################################################## @@ -214,40 +212,6 @@ class StepsFuture(BaseFuture): #: or from cancellation. message = Str() - @property - def error(self): - """ - The exception raised by the background task, if any. - - This attribute is only available if the state of this future is - ``FAILED``. If the future has not reached the ``FAILED`` state, any - attempt to access this attribute will raise an ``AttributeError.`` - - Returns - ------- - exception: BaseException - The exception object raised by the background task. - - Raises - ------ - AttributeError - If the task is still executing, or was cancelled, or completed - without raising an exception. - """ - if self.state != FAILED: - raise AttributeError( - "No exception information available. Task has " - "not yet completed, or was cancelled, or completed " - "without an exception. " - "Task state is {}".format(self.state) - ) - return self._error - - # Private traits ########################################################## - - #: Any exception raised by the background task. - _error = Instance(BaseException, allow_none=True) - # Private methods ######################################################### def _process_step(self, step_event): @@ -264,12 +228,6 @@ def _process_step(self, step_event): else: self.step = step - def _process_error(self, error): - """ - Process an ERROR message from the background task. - """ - self._error = error - @ITaskSpecification.register class BackgroundSteps(HasStrictTraits): diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index 1d7d5423..c72cd354 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -9,7 +9,9 @@ # Thanks for using Enthought open source! -from traits_futures.api import IStepsReporter, submit_steps +from traits.api import HasStrictTraits, Instance, List, on_trait_change, Str + +from traits_futures.api import IStepsReporter, StepsFuture, submit_steps def check_steps_reporter_interface(progress): @@ -22,6 +24,20 @@ def check_steps_reporter_interface(progress): return True +class StepsListener(HasStrictTraits): + """ + Listener recording state changes for a StepsFuture. + """ + + messages = List(Str) + + future = Instance(StepsFuture) + + @on_trait_change("future:message") + def _update_messages(self, message): + self.messages.append(message) + + class BackgroundStepsTests: def test_progress_implements_i_steps_reporter(self): future = submit_steps(self.executor, check_steps_reporter_interface) @@ -30,6 +46,21 @@ def test_progress_implements_i_steps_reporter(self): self.assertResult(future, True) self.assertNoException(future) + def test_simple_messages(self): + def send_messages(self, progress): + progress.start("Uploading files") + progress.step("Uploaded file 1") + progress.step("Uploaded file 2") + + future = submit_steps(self.executor, send_messages) + listener = StepsListener(future=future) + self.wait_until_done(future) + + self.assertEqual( + listener.messages, + ["Uploading files", "Uploaded file 1", "Uploaded file 2"], + ) + # Helper functions def halt_executor(self): From 7f4c60c62e7445331bf6903ad128edffbd833045 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Tue, 6 Jul 2021 11:31:44 +0100 Subject: [PATCH 13/28] Stash --- traits_futures/background_steps.py | 17 ++-- .../tests/background_steps_tests.py | 77 ++++++++++++------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 07e26907..f21b2ad1 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -19,10 +19,11 @@ """ # XXX Consider renaming the 'progress' argument to 'reporter'. +# XXX manually test the dialog! Does it work with the default future state? from abc import ABC -from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple +from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple, Union from traits_futures.base_future import BaseFuture from traits_futures.i_task_specification import ITaskSpecification @@ -37,8 +38,6 @@ class StepsCancelled(Exception): class IStepsReporter(ABC): """Interface for step-reporting object passed to the background job.""" - # XXX steps should have a default of None, not -1. The -1 should be - # used only in the dialog. def start(self, message=None, steps=None): """Start reporting progress. @@ -201,16 +200,14 @@ class StepsFuture(BaseFuture): Object representing the front-end handle to a background call. """ - #: Total number of steps. - #: A value of -1 implies that the number of steps is unknown. - steps = Int(-1) + #: Total number of steps, if known. None if not known. + steps = Union(None, Int()) - #: The most recently completed step. + #: The most recently completed step, if any. step = Int(0) - #: Most recently received message from either the background task - #: or from cancellation. - message = Str() + #: Most recently received message from the background task. + message = Union(None, Str()) # Private methods ######################################################### diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index c72cd354..4202c0ce 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -18,10 +18,7 @@ def check_steps_reporter_interface(progress): """ Check that 'progress' has been declared to support the right interface. """ - if not isinstance(progress, IStepsReporter): - raise RuntimeError() - else: - return True + return isinstance(progress, IStepsReporter) class StepsListener(HasStrictTraits): @@ -29,37 +26,69 @@ class StepsListener(HasStrictTraits): Listener recording state changes for a StepsFuture. """ - messages = List(Str) + state_changes = List() future = Instance(StepsFuture) - @on_trait_change("future:message") + @on_trait_change("future:message,future:step,future:steps") def _update_messages(self, message): self.messages.append(message) +# Consider (a) storing the state on the StepsReporter, so that it can be +# accessed directly by the writer of the function, and then (b) there +# can be a single simple message type which sends the state (message, step, steps) +# to the foreground. We can add helper functions to do things like increment +# the step and set a new message all at the same time. + +# State is: +# - step: number of completed steps +# - steps: total number of steps +# - message: description of step currently in progress + +# XXX Simplest use-case: no calls to progress at all; just a long-running task. +# XXX Next simplest: progress.step("uploading file 1"), progress.step("uploading file 2") +# XXX Next simplest: progress.steps = 2; progress.step("..."), progress.step("...") +# XXX Advanced: progress.sync() / progress.update() / .... after manual changes. + +# XXX Does the steps reporter actually _need_ to be a `HasStrictTraits` class? + class BackgroundStepsTests: def test_progress_implements_i_steps_reporter(self): future = submit_steps(self.executor, check_steps_reporter_interface) - self.wait_until_done(future) + result = self.wait_for_result(future) + self.assertTrue(result) + + def test_initial_values(self): + def do_nothing(progress): + return 23 + + future = submit_steps(self.executor, do_nothing) - self.assertResult(future, True) - self.assertNoException(future) + self.assertIsNone(future.message) + self.assertIsNone(future.steps) + self.assertEqual(future.step, 0) + + result = self.wait_for_result(future) + self.assertEqual(result, 23) def test_simple_messages(self): - def send_messages(self, progress): - progress.start("Uploading files") - progress.step("Uploaded file 1") - progress.step("Uploaded file 2") + def send_messages(progress): + progress.step("Uploading file 1") + progress.step("Uploading file 2") future = submit_steps(self.executor, send_messages) listener = StepsListener(future=future) - self.wait_until_done(future) + self.wait_for_result(future) + + expected_changes = [ + dict(message="Uploading file 1", step=0), + dict(message="Uploading file 2", step=1), + ] + + actual_changes = listener.all_state_changes - self.assertEqual( - listener.messages, - ["Uploading files", "Uploaded file 1", "Uploaded file 2"], - ) + self.assertEqual(expected_changes, actual_changes) # Helper functions @@ -72,14 +101,6 @@ def halt_executor(self): self.run_until(executor, "stopped", lambda executor: executor.stopped) del self.executor - def wait_until_done(self, future): + def wait_for_result(self, future): self.run_until(future, "done", lambda future: future.done) - - # Assertions - - def assertResult(self, future, expected_result): - self.assertEqual(future.result, expected_result) - - def assertNoException(self, future): - with self.assertRaises(AttributeError): - future.exception + return future.result From 63e5a6737918a58853580743f1d7ec1fd59c6f7c Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Wed, 11 Aug 2021 10:56:03 +0100 Subject: [PATCH 14/28] Merge main branch --- traits_futures/api.py | 5 +- traits_futures/background_steps.py | 51 ++++++++++--------- .../tests/background_steps_tests.py | 19 +++---- traits_futures/tests/test_api.py | 1 - traits_futures/tests/test_background_steps.py | 8 +-- 5 files changed, 42 insertions(+), 42 deletions(-) diff --git a/traits_futures/api.py b/traits_futures/api.py index 4fea3d2f..753a5542 100644 --- a/traits_futures/api.py +++ b/traits_futures/api.py @@ -88,10 +88,7 @@ IterationFuture, submit_iteration, ) -from traits_futures.background_progress import ( - ProgressFuture, - submit_progress, -) +from traits_futures.background_progress import ProgressFuture, submit_progress from traits_futures.background_steps import ( IStepsReporter, StepsFuture, diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index f21b2ad1..17371583 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -25,16 +25,10 @@ from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple, Union -from traits_futures.base_future import BaseFuture +from traits_futures.base_future import BaseFuture, BaseTask, TaskCancelled from traits_futures.i_task_specification import ITaskSpecification -class StepsCancelled(Exception): - """ - Exception raised interally when the user cancels the background task. - """ - - class IStepsReporter(ABC): """Interface for step-reporting object passed to the background job.""" @@ -53,7 +47,7 @@ def start(self, message=None, steps=None): Raises ------ - StepsCancelled + TaskCancelled If the user has called ``cancel()`` before this. """ @@ -73,7 +67,7 @@ def step(self, message=None, step=None, steps=None): Raises ------ - StepsCancelled + TaskCancelled If the user has called ``cancel()`` before this. """ @@ -115,11 +109,11 @@ def start(self, message=None, steps=None): Raises ------ - StepsCancelled + TaskCancelled If the user has called ``cancel()`` before this. """ self._check_cancel() - self._send((STEP, (0, steps, message))) + self._send(STEP, (0, steps, message)) def step(self, message=None, step=None, steps=None): """Emit a step event. @@ -137,14 +131,14 @@ def step(self, message=None, step=None, steps=None): Raises ------ - StepsCancelled + TaskCancelled If the user has called ``cancel()`` before this. """ self._check_cancel() if steps is None: steps = -1 - self._send((STEP, (step, steps, message))) + self._send(STEP, (step, steps, message)) # Private traits ########################################################## @@ -163,14 +157,14 @@ def _check_cancel(self): Raises ------ - StepsCancelled + TaskCancelled If the task has been cancelled. """ if self._cancelled(): - raise StepsCancelled("Cancellation requested via the future") + raise TaskCancelled("Cancellation requested via the future") -class StepsBackgroundTask: +class StepsTask(BaseTask): """ Wrapper around the actual callable to be run. @@ -183,13 +177,15 @@ def __init__(self, callable, args, kwargs): self.args = args self.kwargs = kwargs - def __call__(self, send, cancelled): - progress = StepsReporter(_send=send, _cancelled=cancelled) + def run(self): + progress = StepsReporter(_send=self.send, _cancelled=self.cancelled) try: result = self.callable( - *self.args, **self.kwargs, progress=progress + *self.args, + **self.kwargs, + progress=progress, ) - except StepsCancelled: + except TaskCancelled: return None else: return result @@ -258,10 +254,17 @@ class BackgroundSteps(HasStrictTraits): # --- ITaskSpecification implementation ----------------------------------- - def future(self): + def future(self, cancel): """ Return a Future for the background task. + Parameters + ---------- + cancel + Zero-argument callable, returning no useful result. The returned + future's ``cancel`` method should call this to request cancellation + of the associated background task. + Returns ------- future : ProgressFuture @@ -270,7 +273,7 @@ def future(self): """ return StepsFuture() - def background_task(self): + def task(self): """ Return a background callable for this task specification. @@ -281,10 +284,10 @@ def background_task(self): callable can use ``send`` to send messages and ``cancelled`` to check whether cancellation has been requested. """ - return StepsBackgroundTask( + return StepsTask( callable=self.callable, args=self.args, - kwargs=self.kwargs.copy(), + kwargs=self.kwargs, ) diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index 4202c0ce..aa4e617a 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -9,7 +9,7 @@ # Thanks for using Enthought open source! -from traits.api import HasStrictTraits, Instance, List, on_trait_change, Str +from traits.api import HasStrictTraits, Instance, List, observe from traits_futures.api import IStepsReporter, StepsFuture, submit_steps @@ -23,16 +23,16 @@ def check_steps_reporter_interface(progress): class StepsListener(HasStrictTraits): """ - Listener recording state changes for a StepsFuture. + Listener recording all state changes for a StepsFuture. """ - state_changes = List() + messages = List() future = Instance(StepsFuture) - @on_trait_change("future:message,future:step,future:steps") - def _update_messages(self, message): - self.messages.append(message) + @observe("future:message,future:step,future:steps") + def _update_messages(self, event): + self.messages.append((event.name, event.new)) # Consider (a) storing the state on the StepsReporter, so that it can be @@ -53,6 +53,7 @@ def _update_messages(self, message): # XXX Does the steps reporter actually _need_ to be a `HasStrictTraits` class? + class BackgroundStepsTests: def test_progress_implements_i_steps_reporter(self): future = submit_steps(self.executor, check_steps_reporter_interface) @@ -81,14 +82,14 @@ def send_messages(progress): listener = StepsListener(future=future) self.wait_for_result(future) - expected_changes = [ + expected_messages = [ dict(message="Uploading file 1", step=0), dict(message="Uploading file 2", step=1), ] - actual_changes = listener.all_state_changes + actual_messages = listener.messages - self.assertEqual(expected_changes, actual_changes) + # self.assertEqual(expected_messages, actual_messages) # Helper functions diff --git a/traits_futures/tests/test_api.py b/traits_futures/tests/test_api.py index 6cac89cc..91794ac8 100644 --- a/traits_futures/tests/test_api.py +++ b/traits_futures/tests/test_api.py @@ -36,7 +36,6 @@ def test_imports(self): MultithreadingContext, ProgressFuture, RUNNING, - StepsCancelled, StepsFuture, STOPPED, STOPPING, diff --git a/traits_futures/tests/test_background_steps.py b/traits_futures/tests/test_background_steps.py index 9e3a6ea9..be8ac148 100644 --- a/traits_futures/tests/test_background_steps.py +++ b/traits_futures/tests/test_background_steps.py @@ -20,16 +20,16 @@ StepsFuture, TraitsExecutor, ) -from traits_futures.testing.gui_test_assistant import GuiTestAssistant +from traits_futures.testing.test_assistant import TestAssistant from traits_futures.tests.background_steps_tests import BackgroundStepsTests from traits_futures.tests.common_future_tests import CommonFutureTests class TestBackgroundSteps( - GuiTestAssistant, BackgroundStepsTests, unittest.TestCase + TestAssistant, BackgroundStepsTests, unittest.TestCase ): def setUp(self): - GuiTestAssistant.setUp(self) + TestAssistant.setUp(self) self._context = MultithreadingContext() self.executor = TraitsExecutor( context=self._context, @@ -39,7 +39,7 @@ def setUp(self): def tearDown(self): self.halt_executor() self._context.close() - GuiTestAssistant.tearDown(self) + TestAssistant.tearDown(self) class TestStepsFuture(CommonFutureTests, unittest.TestCase): From 18681c3d7069294a27326e765c6e1a4bb0201567 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Wed, 11 Aug 2021 11:00:19 +0100 Subject: [PATCH 15/28] flake8 appeasement --- .../tests/background_steps_tests.py | 29 ++++++++++--------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index aa4e617a..5ceef2ba 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -36,10 +36,10 @@ def _update_messages(self, event): # Consider (a) storing the state on the StepsReporter, so that it can be -# accessed directly by the writer of the function, and then (b) there -# can be a single simple message type which sends the state (message, step, steps) -# to the foreground. We can add helper functions to do things like increment -# the step and set a new message all at the same time. +# accessed directly by the writer of the function, and then (b) there can be a +# single simple message type which sends the state (message, step, steps) to +# the foreground. We can add helper functions to do things like increment the +# step and set a new message all at the same time. # State is: # - step: number of completed steps @@ -47,9 +47,12 @@ def _update_messages(self, event): # - message: description of step currently in progress # XXX Simplest use-case: no calls to progress at all; just a long-running task. -# XXX Next simplest: progress.step("uploading file 1"), progress.step("uploading file 2") -# XXX Next simplest: progress.steps = 2; progress.step("..."), progress.step("...") -# XXX Advanced: progress.sync() / progress.update() / .... after manual changes. +# XXX Next simplest: progress.step("uploading file 1"), +# progress.step("uploading file 2") +# XXX Next simplest: progress.steps = 2; progress.step("..."), +# progress.step("...") +# XXX Advanced: progress.sync() / progress.update() / .... after manual +# changes. # XXX Does the steps reporter actually _need_ to be a `HasStrictTraits` class? @@ -79,15 +82,15 @@ def send_messages(progress): progress.step("Uploading file 2") future = submit_steps(self.executor, send_messages) - listener = StepsListener(future=future) + # listener = StepsListener(future=future) self.wait_for_result(future) - expected_messages = [ - dict(message="Uploading file 1", step=0), - dict(message="Uploading file 2", step=1), - ] + # expected_messages = [ + # dict(message="Uploading file 1", step=0), + # dict(message="Uploading file 2", step=1), + # ] - actual_messages = listener.messages + # actual_messages = listener.messages # self.assertEqual(expected_messages, actual_messages) From 2b38a1c1b0f7b94c1ee571a16b82008a0988d45a Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Thu, 12 Aug 2021 09:14:14 +0100 Subject: [PATCH 16/28] Rework internal state --- traits_futures/background_steps.py | 67 ++++++++++--------- .../tests/background_steps_tests.py | 45 ++++++++++--- 2 files changed, 73 insertions(+), 39 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 17371583..e2b56e30 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -94,18 +94,18 @@ class StepsReporter(HasStrictTraits): """ - def start(self, message=None, steps=None): - """Start reporting progress. + def step(self, message, *, step_size=1): + """ + Start a new step. Parameters ---------- - message : str, optional - A description for the task at the start. - steps : int, optional - The number of steps this task will perform. By default, - this task has no known number of steps. Any UI - representation will just show that work is being done - without making any claims about quantitative progress. + message : str + A description of what is currently being done, replacing the + current message. + step_size : int + The size of this step in whatever units have been chosen. + Defaults to 1. Raises ------ @@ -113,35 +113,42 @@ def start(self, message=None, steps=None): If the user has called ``cancel()`` before this. """ self._check_cancel() - self._send(STEP, (0, steps, message)) - def step(self, message=None, step=None, steps=None): - """Emit a step event. + # Close the previous step, if any. + if self._step_size is not None: + self._step += self._step_size + self._step_size = None - Parameters - ---------- - message : str, optional - A description of what is currently being done, replacing the - current message. - step : int, optional - The step number. If omitted, an internal count will be - incremented by one. - steps : int, optional - The new total number of steps, if changed. + self._message = message + self._step_size = step_size + self._send(STEP, (self._step, self._steps, self._message)) - Raises - ------ - TaskCancelled - If the user has called ``cancel()`` before this. - """ + def complete(self, message="Complete"): self._check_cancel() - if steps is None: - steps = -1 - self._send(STEP, (step, steps, message)) + # Close the previous step, if any. + if self._step_size is not None: + self._step += self._step_size + self._step_size = None + + self._message = message + self._send(STEP, (self._step, self._steps, self._message)) # Private traits ########################################################## + #: Total number of steps, if known. None if not known. + _steps = Union(None, Int()) + + #: Number of steps completed. + _step = Int(0) + + #: Size of the step currently in progress, or None if there's no current + #: step (because we haven't started, or have finished). + _step_size = Union(None, Int()) + + #: Description of the step currently in progress, or None. + _message = Union(None, Str()) + #: Hook to send messages to the foreground. In normal use, this is provided #: by the Traits Futures machinery. _send = Callable() diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index 5ceef2ba..81657b79 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -30,6 +30,13 @@ class StepsListener(HasStrictTraits): future = Instance(StepsFuture) + @observe("future") + def _capture_initial_state(self, event): + future = event.new + self.messages.append(("message", future.message)) + self.messages.append(("step", future.step)) + self.messages.append(("steps", future.steps)) + @observe("future:message,future:step,future:steps") def _update_messages(self, event): self.messages.append((event.name, event.new)) @@ -80,19 +87,39 @@ def test_simple_messages(self): def send_messages(progress): progress.step("Uploading file 1") progress.step("Uploading file 2") + progress.complete() future = submit_steps(self.executor, send_messages) - # listener = StepsListener(future=future) + listener = StepsListener(future=future) self.wait_for_result(future) - # expected_messages = [ - # dict(message="Uploading file 1", step=0), - # dict(message="Uploading file 2", step=1), - # ] - - # actual_messages = listener.messages - - # self.assertEqual(expected_messages, actual_messages) + expected_messages = [ + # Initial values + dict(message=None, steps=None, step=0), + # Updates on start of first step + dict(message="Uploading file 1"), + # Updates on start of second step + dict(message="Uploading file 2", step=1), + # Updates on completion. + dict(message="Complete", step=2), + ] + self.check_messages(listener.messages, expected_messages) + + def check_messages(self, actual_messages, expected_messages): + + actual_messages = actual_messages.copy() + + # Expected messages should match actual messages, in chunks + for message_set in expected_messages: + message_count = len(message_set) + self.assertCountEqual( + actual_messages[:message_count], + list(message_set.items()), + ) + actual_messages = actual_messages[message_count:] + + # Check we got everything. + self.assertFalse(actual_messages) # Helper functions From 9d0ca4107bc9569fa3e8e64dc3341374a2f435fa Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Thu, 12 Aug 2021 09:18:07 +0100 Subject: [PATCH 17/28] Cleanup --- traits_futures/background_steps.py | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index e2b56e30..a5145643 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -233,19 +233,6 @@ def _process_step(self, step_event): class BackgroundSteps(HasStrictTraits): """ Object representing the background call to be executed. - - The *callable* function will be called with an additional - argument, passed via the name "progress". The object passed - can then be used to submit progress information to the foreground. - - Parameters - ---------- - callable : callable - The callable to be executed in the background. - args : tuple - Positional arguments to be passed to *callable*. - kwargs : mapping - Keyword arguments to be passed to *callable*. """ #: The callable for the task. @@ -278,7 +265,7 @@ def future(self, cancel): Future object that can be used to monitor the status of the background task. """ - return StepsFuture() + return StepsFuture(_cancel=cancel) def task(self): """ @@ -286,7 +273,7 @@ def task(self): Returns ------- - collections.abc.Callable + task : StepsTask Callable accepting arguments ``send`` and ``cancelled``. The callable can use ``send`` to send messages and ``cancelled`` to check whether cancellation has been requested. @@ -321,8 +308,11 @@ def submit_steps(executor, callable, *args, **kwargs): Returns ------- future : StepsFuture - Object representing the state of the background call. + Object representing the state of the background task. """ + if "progress" in kwargs: + raise TypeError("progress may not be passed as a named argument") + return executor.submit( BackgroundSteps( callable=callable, From 92a24f966d59b7d77343cd2a99e1849fb48ff819 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Thu, 12 Aug 2021 09:56:30 +0100 Subject: [PATCH 18/28] Fix the dialog example --- .../examples/background_progress_dialog.py | 47 ++++++++++++++----- .../source/guide/examples/progress_dialogs.py | 5 +- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py index 2e4c5408..61f77f2d 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -8,18 +8,32 @@ # # Thanks for using Enthought open source! -""" Qt implementation of a Pyface Dialog that listens to -a ProgressFuture. +""" Qt implementation of a Pyface Dialog that listens to a StepsFuture. """ from pyface.api import Dialog from pyface.qt import QtCore, QtGui -from traits.api import Any, Bool, Instance, Int, on_trait_change +from traits.api import ( + Any, + Bool, + Instance, + Int, + observe, + on_trait_change, + Property, + Str, +) from traits_futures.api import EXECUTING, StepsFuture # XXX Fix behaviour on dialog close button. Should match pressing the # "cancelling" button. (What do users want?) -# Similarly for doing a Ctrl-C +# Similarly for doing a Ctrl-C. + +# XXX Rename "ProgressFutureDialog" to "StepsFutureDialog" + +# XXX Rename "progress_future" trait to just "future". + +# XXX Convert everything to use observe instead of on_trait_change. class ProgressFutureDialog(Dialog): @@ -43,6 +57,9 @@ class ProgressFutureDialog(Dialog): #: The traited ``Future`` representing the state of the background call. progress_future = Instance(StepsFuture) + #: The message to display + message = Property(Str, observe="progress_future:[state,message]") + def cancel(self): """Cancel the job. @@ -90,9 +107,7 @@ def _create_buttons(self, parent): return buttons def _create_message(self, dialog, layout): - self._message_control = QtGui.QLabel( - self.progress_future.message, dialog - ) + self._message_control = QtGui.QLabel(self.message, dialog) self._message_control.setAlignment( QtCore.Qt.AlignTop | QtCore.Qt.AlignLeft ) @@ -119,13 +134,19 @@ def _update_max_on_progress_bar(self, maximum): if self._progress_bar is not None: self._progress_bar.setRange(0, maximum) - @on_trait_change("progress_future:[message,state]") - def _update_message(self, future, name, new): - if future.state == EXECUTING and future.message != "": - message = f"{future.state}: {future.message}" + @observe("message") + def _update_message_in_message_control(self, event): + self._message_control.setText(event.new) + + def _get_message(self): + """ + Property getter for the 'message' trait. + """ + future = self.progress_future + if future.state == EXECUTING and future.message is not None: + return f"{future.state}: {future.message}" else: - message = f"{future.state}" - self._message_control.setText(message) + return f"{future.state}" @on_trait_change("progress_future:steps") def _update_maximum(self, steps): diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py index 32d7e23b..b5355132 100644 --- a/docs/source/guide/examples/progress_dialogs.py +++ b/docs/source/guide/examples/progress_dialogs.py @@ -12,6 +12,8 @@ Demo script for modal progress dialog. """ +# XXX Rename to steps_dialogs. + import concurrent.futures import time @@ -23,9 +25,8 @@ def count(target, *, sleep=1.0, progress): - progress.start("counting", steps=target) for i in range(target): - progress.step(f"step {i} of {target}", step=i) + progress.step(f"processing item {i+1} of {target}") time.sleep(sleep) From 2a2429b0e517f301f42ac4dac18c19743c31d89c Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Thu, 12 Aug 2021 10:06:23 +0100 Subject: [PATCH 19/28] Add 'start' back --- traits_futures/background_steps.py | 26 ++++--------- .../tests/background_steps_tests.py | 37 ++++++++++++++++++- 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index a5145643..cc171fac 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -32,25 +32,6 @@ class IStepsReporter(ABC): """Interface for step-reporting object passed to the background job.""" - def start(self, message=None, steps=None): - """Start reporting progress. - - Parameters - ---------- - message : str, optional - A description for the job at the start. - steps : int, optional - The number of steps this job will perform. By default, - this job has no known number of steps. Any UI - representation will just show that work is being done - without making any claims about quantitative progress. - - Raises - ------ - TaskCancelled - If the user has called ``cancel()`` before this. - """ - def step(self, message=None, step=None, steps=None): """Emit a step event. @@ -94,6 +75,13 @@ class StepsReporter(HasStrictTraits): """ + def start(self, *, steps): + """ + Set the number of steps. + """ + self._steps = steps + self._send(STEP, (self._step, self._steps, self._message)) + def step(self, message, *, step_size=1): """ Start a new step. diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index 81657b79..c27283d4 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -11,7 +11,12 @@ from traits.api import HasStrictTraits, Instance, List, observe -from traits_futures.api import IStepsReporter, StepsFuture, submit_steps +from traits_futures.api import ( + COMPLETED, + IStepsReporter, + StepsFuture, + submit_steps, +) def check_steps_reporter_interface(progress): @@ -105,6 +110,31 @@ def send_messages(progress): ] self.check_messages(listener.messages, expected_messages) + def test_set_total(self): + def send_messages(progress): + progress.start(steps=2) + progress.step("Uploading file 1") + progress.step("Uploading file 2") + progress.complete() + + future = submit_steps(self.executor, send_messages) + listener = StepsListener(future=future) + self.wait_for_result(future) + + expected_messages = [ + # Initial values + dict(message=None, steps=None, step=0), + # Updates on setting 'steps' to 2. + dict(steps=2), + # Updates on start of first step + dict(message="Uploading file 1"), + # Updates on start of second step + dict(message="Uploading file 2", step=1), + # Updates on completion. + dict(message="Complete", step=2), + ] + self.check_messages(listener.messages, expected_messages) + def check_messages(self, actual_messages, expected_messages): actual_messages = actual_messages.copy() @@ -134,4 +164,7 @@ def halt_executor(self): def wait_for_result(self, future): self.run_until(future, "done", lambda future: future.done) - return future.result + if future.state == COMPLETED: + return future.result + else: + self.fail(f"Future did not return a result: {future.exception}") From 5eadcfab6816e12ec247901db4297114f8f1ce18 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Thu, 12 Aug 2021 10:06:35 +0100 Subject: [PATCH 20/28] Fix up progress dialogs example to work as before --- docs/source/guide/examples/progress_dialogs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py index b5355132..d0d033ad 100644 --- a/docs/source/guide/examples/progress_dialogs.py +++ b/docs/source/guide/examples/progress_dialogs.py @@ -25,6 +25,7 @@ def count(target, *, sleep=1.0, progress): + progress.start(steps=target) for i in range(target): progress.step(f"processing item {i+1} of {target}") time.sleep(sleep) From ffd8f444afd744b6cb46e7b254b97c63c8f14e1b Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Thu, 12 Aug 2021 11:50:41 +0100 Subject: [PATCH 21/28] Add note about errors seen using the non-modal dialog --- .../examples/background_progress_dialog.py | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py index 61f77f2d..004e48c3 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -19,7 +19,6 @@ Instance, Int, observe, - on_trait_change, Property, Str, ) @@ -33,8 +32,19 @@ # XXX Rename "progress_future" trait to just "future". -# XXX Convert everything to use observe instead of on_trait_change. - +# XXX Diagnose and fix errors seen when launching a non-modal dialog and then +# clicking its close button: +""" +Exception occurred in traits notification handler for event object: TraitChangeEvent(object=, name='message', old=, new='executing: processing item 10 of 10') +Traceback (most recent call last): + File "/Users/mdickinson/.venvs/traits-futures/lib/python3.9/site-packages/traits/observation/_trait_event_notifier.py", line 122, in __call__ + self.dispatcher(handler, event) + File "/Users/mdickinson/.venvs/traits-futures/lib/python3.9/site-packages/traits/observation/observe.py", line 26, in dispatch_same + handler(event) + File "/Users/mdickinson/Enthought/Projects/traits-futures/docs/source/guide/examples/background_progress_dialog.py", line 137, in _update_message_in_message_control + self._message_control.setText(event.new) +AttributeError: 'NoneType' object has no attribute 'setText' +""" class ProgressFutureDialog(Dialog): """Show a cancellable progress dialog listening to a progress manager.""" @@ -123,14 +133,15 @@ def _create_gauge(self, dialog, layout): self._progress_bar.setFormat("%v") return self._progress_bar - @on_trait_change("closing") - def _destroy_traits_on_dialog_closing(self): + @observe("closing") + def _destroy_traits_on_dialog_closing(self, event): self._message_control = None self._progress_bar = None self._cancel_button_control = None - @on_trait_change("maximum") - def _update_max_on_progress_bar(self, maximum): + @observe("maximum") + def _update_max_on_progress_bar(self, event): + maximum = event.new if self._progress_bar is not None: self._progress_bar.setRange(0, maximum) @@ -148,15 +159,17 @@ def _get_message(self): else: return f"{future.state}" - @on_trait_change("progress_future:steps") - def _update_maximum(self, steps): + @observe("progress_future:steps") + def _update_maximum(self, event): + steps = event.new self.maximum = max(steps, 0) - @on_trait_change("progress_future:step") - def _update_value(self, step): + @observe("progress_future:step") + def _update_value(self, event): + step = event.new if self._progress_bar is not None: self._progress_bar.setValue(step) - @on_trait_change("progress_future:done") + @observe("progress_future:done") def _on_end(self, event): self.close() From 6b3577440ff593dca979ce3e0fff5164af2b3412 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Fri, 13 Aug 2021 09:52:19 +0100 Subject: [PATCH 22/28] Some renaming and cleanup --- .../examples/background_progress_dialog.py | 1 + .../source/guide/examples/progress_dialogs.py | 16 ++-- docs/source/guide/examples/steps_dialogs.py | 87 +++++++++++++++++++ traits_futures/background_steps.py | 56 +++++++----- .../tests/background_steps_tests.py | 76 ++++++++++------ 5 files changed, 180 insertions(+), 56 deletions(-) create mode 100644 docs/source/guide/examples/steps_dialogs.py diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py index 004e48c3..183d19b9 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -46,6 +46,7 @@ AttributeError: 'NoneType' object has no attribute 'setText' """ + class ProgressFutureDialog(Dialog): """Show a cancellable progress dialog listening to a progress manager.""" diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py index d0d033ad..cb46a5e5 100644 --- a/docs/source/guide/examples/progress_dialogs.py +++ b/docs/source/guide/examples/progress_dialogs.py @@ -24,10 +24,10 @@ from background_progress_dialog import ProgressFutureDialog -def count(target, *, sleep=1.0, progress): - progress.start(steps=target) +def count(target, *, sleep=1.0, reporter): + reporter.start("Starting", steps=target) for i in range(target): - progress.step(f"processing item {i+1} of {target}") + reporter.step(f"processing item {i+1} of {target}") time.sleep(sleep) @@ -49,22 +49,20 @@ def _executor_default(self): cf_executor = Any() def _calculate_fired(self): - target = self.target - future = submit_steps(self.executor, count, target=target) + future = submit_steps(self.executor, count, target=self.target) dialog = ProgressFutureDialog( progress_future=future, style="modal", # this is the default - title=f"Counting to {target}", + title=f"Counting to {self.target}", ) dialog.open() def _nonmodal_fired(self): - target = self.target - future = submit_steps(self.executor, count, target) + future = submit_steps(self.executor, count, self.target) dialog = ProgressFutureDialog( progress_future=future, style="nonmodal", # this is the default - title=f"Counting to {target}", + title=f"Counting to {self.target}", ) dialog.open() # Keep a reference to open dialogs, else they'll diff --git a/docs/source/guide/examples/steps_dialogs.py b/docs/source/guide/examples/steps_dialogs.py new file mode 100644 index 00000000..b9731f24 --- /dev/null +++ b/docs/source/guide/examples/steps_dialogs.py @@ -0,0 +1,87 @@ +# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX +# All rights reserved. +# +# This software is provided without warranty under the terms of the BSD +# license included in LICENSE.txt and may be redistributed only under +# the conditions described in the aforementioned license. The license +# is also available online at http://www.enthought.com/licenses/BSD.txt +# +# Thanks for using Enthought open source! + +""" +Demo script for modal progress dialog. +""" + +import concurrent.futures +import time + +from traits.api import Any, Button, HasStrictTraits, Instance, List, Range +from traits_futures.api import submit_steps, TraitsExecutor +from traitsui.api import Item, View + +from background_progress_dialog import ProgressFutureDialog + + +def count(target, *, sleep=1.0, reporter): + reporter.start(steps=target) + for i in range(target): + reporter.step(f"processing item {i+1} of {target}") + time.sleep(sleep) + + +class MyView(HasStrictTraits): + + calculate = Button() + + nonmodal = Button() + + target = Range(0, 100, 10) + + executor = Instance(TraitsExecutor) + + def _executor_default(self): + return TraitsExecutor() # worker_pool=self.cf_executor) + + dialogs = List(ProgressFutureDialog) + + cf_executor = Any() + + def _calculate_fired(self): + target = self.target + future = submit_steps(self.executor, count, target=target) + dialog = ProgressFutureDialog( + progress_future=future, + style="modal", # this is the default + title=f"Counting to {target}", + ) + dialog.open() + + def _nonmodal_fired(self): + target = self.target + future = submit_steps(self.executor, count, target) + dialog = ProgressFutureDialog( + progress_future=future, + style="nonmodal", # this is the default + title=f"Counting to {target}", + ) + dialog.open() + # Keep a reference to open dialogs, else they'll + # mysteriously disappear. A better solution might be to + # parent them appropriately. + self.dialogs.append(dialog) + + view = View( + Item("calculate", label="Count modal"), + Item("nonmodal", label="Count nonmodal"), + Item("target"), + ) + + +def main(): + with concurrent.futures.ThreadPoolExecutor() as executor: + view = MyView(cf_executor=executor) + view.configure_traits() + + +if __name__ == "__main__": + main() diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index cc171fac..09187318 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -18,9 +18,6 @@ """ -# XXX Consider renaming the 'progress' argument to 'reporter'. -# XXX manually test the dialog! Does it work with the default future state? - from abc import ABC from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple, Union @@ -28,6 +25,8 @@ from traits_futures.base_future import BaseFuture, BaseTask, TaskCancelled from traits_futures.i_task_specification import ITaskSpecification +# XXX Fix IStepsReporter to match the actual interface of the reporter. + class IStepsReporter(ABC): """Interface for step-reporting object passed to the background job.""" @@ -58,7 +57,7 @@ def step(self, message=None, step=None, steps=None): #: Message sent on a start or step operation. Argument is a tuple #: (step, steps, message), with: #: -#: * step: int or None - number of completed steps so far +#: * step: int - number of completed steps so far #: * steps: int or None - total number of steps, if known #: * message: str or None - message to display for this step STEP = "step" @@ -75,11 +74,19 @@ class StepsReporter(HasStrictTraits): """ - def start(self, *, steps): + def start(self, message=None, *, steps=None): """ Set the number of steps. + + Parameters + ---------- + message : str, optional + Message to set at start time. + steps : int, optional + Number of steps, if known. """ self._steps = steps + self._message = message self._send(STEP, (self._step, self._steps, self._message)) def step(self, message, *, step_size=1): @@ -111,7 +118,7 @@ def step(self, message, *, step_size=1): self._step_size = step_size self._send(STEP, (self._step, self._steps, self._message)) - def complete(self, message="Complete"): + def stop(self, message="Complete"): self._check_cancel() # Close the previous step, if any. @@ -147,6 +154,14 @@ def complete(self, message="Complete"): # Private methods ######################################################### + def _close(self): + if self._step_size is not None: + self._step += self._step_size + self._step_size = None + self._message = None + + self._send(STEP, (self._step, self._steps, self._message)) + def _check_cancel(self): """Check if the task has been cancelled. @@ -173,13 +188,14 @@ def __init__(self, callable, args, kwargs): self.kwargs = kwargs def run(self): - progress = StepsReporter(_send=self.send, _cancelled=self.cancelled) + reporter = StepsReporter(_send=self.send, _cancelled=self.cancelled) try: result = self.callable( *self.args, **self.kwargs, - progress=progress, + reporter=reporter, ) + reporter._close() except TaskCancelled: return None else: @@ -207,14 +223,9 @@ def _process_step(self, step_event): Process a STEP message from the background task. """ step, steps, message = step_event - if message is not None: - self.message = message - if steps is not None: - self.steps = steps - if step is None: - self.step += 1 - else: - self.step = step + self.message = message + self.steps = steps + self.step = step @ITaskSpecification.register @@ -229,8 +240,8 @@ class BackgroundSteps(HasStrictTraits): #: Positional arguments to be passed to the callable. args = Tuple() - #: Named arguments to be passed to the callable, excluding the "progress" - #: named argument. The "progress" argument will be supplied through the + #: Named arguments to be passed to the callable, excluding the "reporter" + #: named argument. The "reporter" argument will be supplied through the #: execution machinery. kwargs = Dict(Str()) @@ -275,7 +286,7 @@ def task(self): def submit_steps(executor, callable, *args, **kwargs): """ - Convenience function to submit a background progress task to an executor. + Convenience function to submit a BackgroundSteps task to an executor. Parameters ---------- @@ -298,8 +309,11 @@ def submit_steps(executor, callable, *args, **kwargs): future : StepsFuture Object representing the state of the background task. """ - if "progress" in kwargs: - raise TypeError("progress may not be passed as a named argument") + if "reporter" in kwargs: + raise TypeError( + "The 'reporter' parameter will be passed automatically; it " + "should not be included in the named parameters." + ) return executor.submit( BackgroundSteps( diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index c27283d4..519d5736 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -19,13 +19,6 @@ ) -def check_steps_reporter_interface(progress): - """ - Check that 'progress' has been declared to support the right interface. - """ - return isinstance(progress, IStepsReporter) - - class StepsListener(HasStrictTraits): """ Listener recording all state changes for a StepsFuture. @@ -47,12 +40,6 @@ def _update_messages(self, event): self.messages.append((event.name, event.new)) -# Consider (a) storing the state on the StepsReporter, so that it can be -# accessed directly by the writer of the function, and then (b) there can be a -# single simple message type which sends the state (message, step, steps) to -# the foreground. We can add helper functions to do things like increment the -# step and set a new message all at the same time. - # State is: # - step: number of completed steps # - steps: total number of steps @@ -70,13 +57,23 @@ def _update_messages(self, event): class BackgroundStepsTests: - def test_progress_implements_i_steps_reporter(self): + def test_reporter_implements_i_steps_reporter(self): + def check_steps_reporter_interface(reporter): + return isinstance(reporter, IStepsReporter) + future = submit_steps(self.executor, check_steps_reporter_interface) result = self.wait_for_result(future) self.assertTrue(result) + def test_reporter_passed_by_name(self): + def reporter_by_name(*, reporter): + return 46 + + future = submit_steps(self.executor, reporter_by_name) + self.assertEqual(self.wait_for_result(future), 46) + def test_initial_values(self): - def do_nothing(progress): + def do_nothing(reporter): return 23 future = submit_steps(self.executor, do_nothing) @@ -89,10 +86,10 @@ def do_nothing(progress): self.assertEqual(result, 23) def test_simple_messages(self): - def send_messages(progress): - progress.step("Uploading file 1") - progress.step("Uploading file 2") - progress.complete() + def send_messages(reporter): + reporter.step("Uploading file 1") + reporter.step("Uploading file 2") + reporter.stop("Finished") future = submit_steps(self.executor, send_messages) listener = StepsListener(future=future) @@ -106,16 +103,16 @@ def send_messages(progress): # Updates on start of second step dict(message="Uploading file 2", step=1), # Updates on completion. - dict(message="Complete", step=2), + dict(message="Finished", step=2), ] self.check_messages(listener.messages, expected_messages) def test_set_total(self): - def send_messages(progress): - progress.start(steps=2) - progress.step("Uploading file 1") - progress.step("Uploading file 2") - progress.complete() + def send_messages(reporter): + reporter.start(steps=2) + reporter.step("Uploading file 1") + reporter.step("Uploading file 2") + reporter.stop("All done") future = submit_steps(self.executor, send_messages) listener = StepsListener(future=future) @@ -131,10 +128,37 @@ def send_messages(progress): # Updates on start of second step dict(message="Uploading file 2", step=1), # Updates on completion. - dict(message="Complete", step=2), + dict(message="All done", step=2), ] self.check_messages(listener.messages, expected_messages) + def test_no_stop(self): + # If the user doesn't call stop, we should still get a final update + # that sets the 'step'. + def send_messages(reporter): + reporter.start(steps=2) + reporter.step("Uploading file 1") + reporter.step("Uploading file 2") + + future = submit_steps(self.executor, send_messages) + listener = StepsListener(future=future) + self.wait_for_result(future) + + expected_messages = [ + # Initial values + dict(message=None, steps=None, step=0), + # Updates on setting 'steps' to 2. + dict(steps=2), + # Updates on start of first step + dict(message="Uploading file 1"), + # Updates on start of second step + dict(message="Uploading file 2", step=1), + # Updates on completion. + dict(message=None, step=2), + ] + + self.check_messages(listener.messages, expected_messages) + def check_messages(self, actual_messages, expected_messages): actual_messages = actual_messages.copy() From 59b90eefddf797221416e40460f09678a1b0c2c9 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Fri, 13 Aug 2021 09:55:14 +0100 Subject: [PATCH 23/28] Remove duplicate file; add missing docstring --- .../source/guide/examples/progress_dialogs.py | 87 ------------------- docs/source/guide/examples/steps_dialogs.py | 12 ++- 2 files changed, 11 insertions(+), 88 deletions(-) delete mode 100644 docs/source/guide/examples/progress_dialogs.py diff --git a/docs/source/guide/examples/progress_dialogs.py b/docs/source/guide/examples/progress_dialogs.py deleted file mode 100644 index cb46a5e5..00000000 --- a/docs/source/guide/examples/progress_dialogs.py +++ /dev/null @@ -1,87 +0,0 @@ -# (C) Copyright 2018-2021 Enthought, Inc., Austin, TX -# All rights reserved. -# -# This software is provided without warranty under the terms of the BSD -# license included in LICENSE.txt and may be redistributed only under -# the conditions described in the aforementioned license. The license -# is also available online at http://www.enthought.com/licenses/BSD.txt -# -# Thanks for using Enthought open source! - -""" -Demo script for modal progress dialog. -""" - -# XXX Rename to steps_dialogs. - -import concurrent.futures -import time - -from traits.api import Any, Button, HasStrictTraits, Instance, List, Range -from traits_futures.api import submit_steps, TraitsExecutor -from traitsui.api import Item, View - -from background_progress_dialog import ProgressFutureDialog - - -def count(target, *, sleep=1.0, reporter): - reporter.start("Starting", steps=target) - for i in range(target): - reporter.step(f"processing item {i+1} of {target}") - time.sleep(sleep) - - -class MyView(HasStrictTraits): - - calculate = Button() - - nonmodal = Button() - - target = Range(0, 100, 10) - - executor = Instance(TraitsExecutor) - - def _executor_default(self): - return TraitsExecutor() # worker_pool=self.cf_executor) - - dialogs = List(ProgressFutureDialog) - - cf_executor = Any() - - def _calculate_fired(self): - future = submit_steps(self.executor, count, target=self.target) - dialog = ProgressFutureDialog( - progress_future=future, - style="modal", # this is the default - title=f"Counting to {self.target}", - ) - dialog.open() - - def _nonmodal_fired(self): - future = submit_steps(self.executor, count, self.target) - dialog = ProgressFutureDialog( - progress_future=future, - style="nonmodal", # this is the default - title=f"Counting to {self.target}", - ) - dialog.open() - # Keep a reference to open dialogs, else they'll - # mysteriously disappear. A better solution might be to - # parent them appropriately. - self.dialogs.append(dialog) - - view = View( - Item("calculate", label="Count modal"), - Item("nonmodal", label="Count nonmodal"), - Item("target"), - ) - - -def main(): - with concurrent.futures.ThreadPoolExecutor() as executor: - view = MyView(cf_executor=executor) - view.configure_traits() - - -if __name__ == "__main__": - main() diff --git a/docs/source/guide/examples/steps_dialogs.py b/docs/source/guide/examples/steps_dialogs.py index b9731f24..3c84c95d 100644 --- a/docs/source/guide/examples/steps_dialogs.py +++ b/docs/source/guide/examples/steps_dialogs.py @@ -23,7 +23,17 @@ def count(target, *, sleep=1.0, reporter): - reporter.start(steps=target) + """ + Parameters + ---------- + target : int + Value to count to. + sleep : float + Time to sleep between each count. + reporter : IStepsReporter + Object used to report progress. + """ + reporter.start("Starting processing", steps=target) for i in range(target): reporter.step(f"processing item {i+1} of {target}") time.sleep(sleep) From 217b0c2be2e7a79a75355c8c59b1fd481d7bc2f7 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Fri, 13 Aug 2021 11:46:17 +0100 Subject: [PATCH 24/28] Stash --- traits_futures/background_steps.py | 61 +++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index 09187318..a079dc5d 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -18,7 +18,7 @@ """ -from abc import ABC +import abc from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple, Union @@ -26,24 +26,42 @@ from traits_futures.i_task_specification import ITaskSpecification # XXX Fix IStepsReporter to match the actual interface of the reporter. +# XXX Add NullReporter, for convenience in testing. -class IStepsReporter(ABC): +class IStepsReporter(abc.ABC): """Interface for step-reporting object passed to the background job.""" - def step(self, message=None, step=None, steps=None): - """Emit a step event. + @abc.abstractmethod + def start(self, message=None, *, steps=None): + """ + Set an initial message and set the total number of steps. Parameters ---------- message : str, optional - A description of what is currently being done, replacing the - current message. - step : int, optional - The step number. If omitted, an internal count will be - incremented by one. + Message to set at start time. steps : int, optional - The new total number of steps, if changed. + Number of steps, if known. + + Raises + ------ + TaskCancelled + If the user has called ``cancel()`` before this. + """ + + @abc.abstractmethod + def step(self, message=None, *, step_size=1): + """ + Start a processing step. + + Parameters + ---------- + message : str, optional + A description of this step. + step_size : int, optional + The size of this step (in whatever units are being used). + Defaults to 1. Raises ------ @@ -51,6 +69,11 @@ def step(self, message=None, step=None, steps=None): If the user has called ``cancel()`` before this. """ + @abc.abstractmethod + def stop(self, message=None, *, ): + """ + """ + # Message types for messages sent from the background to the future. @@ -76,7 +99,7 @@ class StepsReporter(HasStrictTraits): def start(self, message=None, *, steps=None): """ - Set the number of steps. + Set an initial message and set the total number of steps. Parameters ---------- @@ -84,6 +107,11 @@ def start(self, message=None, *, steps=None): Message to set at start time. steps : int, optional Number of steps, if known. + + Raises + ------ + TaskCancelled + If the user has called ``cancel()`` before this. """ self._steps = steps self._message = message @@ -91,15 +119,14 @@ def start(self, message=None, *, steps=None): def step(self, message, *, step_size=1): """ - Start a new step. + Start a processing step. Parameters ---------- - message : str - A description of what is currently being done, replacing the - current message. - step_size : int - The size of this step in whatever units have been chosen. + message : str, optional + A description of this step. + step_size : int, optional + The size of this step (in whatever units are being used). Defaults to 1. Raises From 2d9b437f0645545b2b0a6ffd4aae5a5bdf5f6f04 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Fri, 13 Aug 2021 12:53:24 +0100 Subject: [PATCH 25/28] Make IStepsReporter and StepsReporter match --- .../examples/background_progress_dialog.py | 31 +++-- traits_futures/background_steps.py | 113 +++++++++++------- 2 files changed, 87 insertions(+), 57 deletions(-) diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/background_progress_dialog.py index 183d19b9..908b54ed 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/background_progress_dialog.py @@ -25,8 +25,8 @@ from traits_futures.api import EXECUTING, StepsFuture # XXX Fix behaviour on dialog close button. Should match pressing the -# "cancelling" button. (What do users want?) -# Similarly for doing a Ctrl-C. +# "cancelling" button, and also pressing ESC. (What do users want?) +# XXX What should behaviour be on a Ctrl-C? # XXX Rename "ProgressFutureDialog" to "StepsFutureDialog" @@ -34,17 +34,22 @@ # XXX Diagnose and fix errors seen when launching a non-modal dialog and then # clicking its close button: -""" -Exception occurred in traits notification handler for event object: TraitChangeEvent(object=, name='message', old=, new='executing: processing item 10 of 10') -Traceback (most recent call last): - File "/Users/mdickinson/.venvs/traits-futures/lib/python3.9/site-packages/traits/observation/_trait_event_notifier.py", line 122, in __call__ - self.dispatcher(handler, event) - File "/Users/mdickinson/.venvs/traits-futures/lib/python3.9/site-packages/traits/observation/observe.py", line 26, in dispatch_same - handler(event) - File "/Users/mdickinson/Enthought/Projects/traits-futures/docs/source/guide/examples/background_progress_dialog.py", line 137, in _update_message_in_message_control - self._message_control.setText(event.new) -AttributeError: 'NoneType' object has no attribute 'setText' -""" +# Exception occurred in traits notification handler for event object: +# TraitChangeEvent(object=, name='message', old=, new='executing: +# processing item 10 of 10') +# Traceback (most recent call last): +# File "/Users/mdickinson/.venvs/traits-futures/lib/python3.9/site-packages/ +# traits/observation/_trait_event_notifier.py", line 122, in __call__ +# self.dispatcher(handler, event) +# File "/Users/mdickinson/.venvs/traits-futures/lib/python3.9/site-packages/ +# traits/observation/observe.py", line 26, in dispatch_same +# handler(event) +# File "/Users/mdickinson/Enthought/Projects/traits-futures/docs/source/ +# guide/examples/background_progress_dialog.py", line 137, in +# _update_message_in_message_control +# self._message_control.setText(event.new) +# AttributeError: 'NoneType' object has no attribute 'setText' class ProgressFutureDialog(Dialog): diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index a079dc5d..f7ef9a3c 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -25,8 +25,14 @@ from traits_futures.base_future import BaseFuture, BaseTask, TaskCancelled from traits_futures.i_task_specification import ITaskSpecification -# XXX Fix IStepsReporter to match the actual interface of the reporter. # XXX Add NullReporter, for convenience in testing. +# XXX Add "check" method to just check for cancellation. +# XXX In the StepsFuture, update all traits before sending trait-change +# notifications. (Is this possible?) +# XXX Consider renaming "_step", "_total", etc. +# XXX Consider namedtuple encapsulating progress state. +# XXX Rethink setting of message - should only happen when a message is +# provided? class IStepsReporter(abc.ABC): @@ -51,7 +57,7 @@ def start(self, message=None, *, steps=None): """ @abc.abstractmethod - def step(self, message=None, *, step_size=1): + def step(self, message=None, *, size=1): """ Start a processing step. @@ -59,9 +65,9 @@ def step(self, message=None, *, step_size=1): ---------- message : str, optional A description of this step. - step_size : int, optional - The size of this step (in whatever units are being used). - Defaults to 1. + size : int, optional + The size of this step (in whatever units make sense for the + application at hand). Defaults to 1. Raises ------ @@ -70,8 +76,18 @@ def step(self, message=None, *, step_size=1): """ @abc.abstractmethod - def stop(self, message=None, *, ): + def stop(self, message=None): """ + Report that processing is complete. + + Also updates the total number of steps completed. + + Parameters + ---------- + message : str, optional + Message to display on completion. For a progress dialog that + disappears on completion, this message will never be seen by + the user, but for other views the message may be visible. """ @@ -87,7 +103,7 @@ def stop(self, message=None, *, ): @IStepsReporter.register -class StepsReporter(HasStrictTraits): +class StepsReporter: """ Object used by the background task to report progress information. @@ -97,6 +113,26 @@ class StepsReporter(HasStrictTraits): """ + def __init__(self, send, cancelled): + # Communications support. + self._send = send + self._cancelled = cancelled + + # Set up internal state. + + #: Total number of steps completed. + self._step = 0 + + #: Total number of steps, if known. None if not known. + self._steps = None + + #: Description for the currently-in-progress step, or None. + self._message = None + + #: Size of the currently executing step. None if no step + #: is currently executing. + self._size = None + def start(self, message=None, *, steps=None): """ Set an initial message and set the total number of steps. @@ -117,7 +153,7 @@ def start(self, message=None, *, steps=None): self._message = message self._send(STEP, (self._step, self._steps, self._message)) - def step(self, message, *, step_size=1): + def step(self, message=None, *, size=1): """ Start a processing step. @@ -125,7 +161,7 @@ def step(self, message, *, step_size=1): ---------- message : str, optional A description of this step. - step_size : int, optional + size : int, optional The size of this step (in whatever units are being used). Defaults to 1. @@ -137,54 +173,43 @@ def step(self, message, *, step_size=1): self._check_cancel() # Close the previous step, if any. - if self._step_size is not None: - self._step += self._step_size - self._step_size = None + if self._size is not None: + self._step += self._size + self._size = None self._message = message - self._step_size = step_size + self._size = size self._send(STEP, (self._step, self._steps, self._message)) - def stop(self, message="Complete"): + def stop(self, message=None): + """ + Report that processing is complete. + + Also updates the total number of steps completed. + + Parameters + ---------- + message : str, optional + Message to display on completion. For a progress dialog that + disappears on completion, this message will never be seen by + the user, but for other views the message may be visible. + """ self._check_cancel() # Close the previous step, if any. - if self._step_size is not None: - self._step += self._step_size - self._step_size = None + if self._size is not None: + self._step += self._size + self._size = None self._message = message self._send(STEP, (self._step, self._steps, self._message)) - # Private traits ########################################################## - - #: Total number of steps, if known. None if not known. - _steps = Union(None, Int()) - - #: Number of steps completed. - _step = Int(0) - - #: Size of the step currently in progress, or None if there's no current - #: step (because we haven't started, or have finished). - _step_size = Union(None, Int()) - - #: Description of the step currently in progress, or None. - _message = Union(None, Str()) - - #: Hook to send messages to the foreground. In normal use, this is provided - #: by the Traits Futures machinery. - _send = Callable() - - #: Callable to check whether the task has been cancelled, provided - #: by the Traits Futures machinery. - _cancelled = Callable() - # Private methods ######################################################### def _close(self): - if self._step_size is not None: - self._step += self._step_size - self._step_size = None + if self._size is not None: + self._step += self._size + self._size = None self._message = None self._send(STEP, (self._step, self._steps, self._message)) @@ -215,7 +240,7 @@ def __init__(self, callable, args, kwargs): self.kwargs = kwargs def run(self): - reporter = StepsReporter(_send=self.send, _cancelled=self.cancelled) + reporter = StepsReporter(send=self.send, cancelled=self.cancelled) try: result = self.callable( *self.args, From 44fe6963f04bd214cc26eff64954f7d4859fc926 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 16 Aug 2021 12:54:18 +0100 Subject: [PATCH 26/28] Reworking - allow setting total and message at start time --- ...und_progress_dialog.py => steps_dialog.py} | 57 ++-- docs/source/guide/examples/steps_dialogs.py | 38 ++- traits_futures/background_steps.py | 254 ++++++++++++------ .../tests/background_steps_tests.py | 209 +++++++++++--- 4 files changed, 393 insertions(+), 165 deletions(-) rename docs/source/guide/examples/{background_progress_dialog.py => steps_dialog.py} (84%) diff --git a/docs/source/guide/examples/background_progress_dialog.py b/docs/source/guide/examples/steps_dialog.py similarity index 84% rename from docs/source/guide/examples/background_progress_dialog.py rename to docs/source/guide/examples/steps_dialog.py index 908b54ed..4762f166 100644 --- a/docs/source/guide/examples/background_progress_dialog.py +++ b/docs/source/guide/examples/steps_dialog.py @@ -13,29 +13,17 @@ from pyface.api import Dialog from pyface.qt import QtCore, QtGui -from traits.api import ( - Any, - Bool, - Instance, - Int, - observe, - Property, - Str, -) +from traits.api import Any, Bool, Instance, Int, observe, Property, Str from traits_futures.api import EXECUTING, StepsFuture # XXX Fix behaviour on dialog close button. Should match pressing the # "cancelling" button, and also pressing ESC. (What do users want?) # XXX What should behaviour be on a Ctrl-C? -# XXX Rename "ProgressFutureDialog" to "StepsFutureDialog" - -# XXX Rename "progress_future" trait to just "future". - # XXX Diagnose and fix errors seen when launching a non-modal dialog and then # clicking its close button: # Exception occurred in traits notification handler for event object: -# TraitChangeEvent(object=, name='message', old=, new='executing: # processing item 10 of 10') # Traceback (most recent call last): @@ -52,7 +40,7 @@ # AttributeError: 'NoneType' object has no attribute 'setText' -class ProgressFutureDialog(Dialog): +class StepsDialog(Dialog): """Show a cancellable progress dialog listening to a progress manager.""" #: Text to show for cancellation label. @@ -64,17 +52,17 @@ class ProgressFutureDialog(Dialog): #: Whether to show a 'Cancel' button or not. cancellable = Bool(True) - #: The maximum number of steps. - maximum = Int(0) - #: Whether to show the percentage complete or not. show_percent = Bool(True) #: The traited ``Future`` representing the state of the background call. - progress_future = Instance(StepsFuture) + future = Instance(StepsFuture) + + #: The message to display. + message = Property(Str(), observe="future:[state,message]") - #: The message to display - message = Property(Str, observe="progress_future:[state,message]") + #: The maximum for the dialog. + maximum = Property(Int(), observe="future:total") def cancel(self): """Cancel the job. @@ -83,7 +71,7 @@ def cancel(self): down to the progress manager since this method will prevent the job from starting if it has not already. """ - self.progress_future.cancel() + self.future.cancel() self._cancel_button_control.setEnabled(False) # Private implementation ################################################## @@ -132,7 +120,7 @@ def _create_message(self, dialog, layout): def _create_gauge(self, dialog, layout): self._progress_bar = QtGui.QProgressBar(dialog) self._progress_bar.setRange(0, self.maximum) - self._progress_bar.setValue(self.progress_future.step) + self._progress_bar.setValue(self.future.complete) if self.show_percent: self._progress_bar.setFormat("%p%") else: @@ -159,23 +147,28 @@ def _get_message(self): """ Property getter for the 'message' trait. """ - future = self.progress_future + future = self.future if future.state == EXECUTING and future.message is not None: return f"{future.state}: {future.message}" else: return f"{future.state}" - @observe("progress_future:steps") - def _update_maximum(self, event): - steps = event.new - self.maximum = max(steps, 0) + def _get_maximum(self): + """ + Property getter for the 'maximum' trait. + """ + future = self.future + if future.total is None: + return 0 + else: + return max(future.total, 0) - @observe("progress_future:step") + @observe("future:complete") def _update_value(self, event): - step = event.new + complete = event.new if self._progress_bar is not None: - self._progress_bar.setValue(step) + self._progress_bar.setValue(complete) - @observe("progress_future:done") + @observe("future:done") def _on_end(self, event): self.close() diff --git a/docs/source/guide/examples/steps_dialogs.py b/docs/source/guide/examples/steps_dialogs.py index 3c84c95d..a9f74d6d 100644 --- a/docs/source/guide/examples/steps_dialogs.py +++ b/docs/source/guide/examples/steps_dialogs.py @@ -15,11 +15,19 @@ import concurrent.futures import time -from traits.api import Any, Button, HasStrictTraits, Instance, List, Range +from traits.api import ( + Any, + Button, + HasStrictTraits, + Instance, + List, + observe, + Range, +) from traits_futures.api import submit_steps, TraitsExecutor from traitsui.api import Item, View -from background_progress_dialog import ProgressFutureDialog +from steps_dialog import StepsDialog def count(target, *, sleep=1.0, reporter): @@ -33,7 +41,7 @@ def count(target, *, sleep=1.0, reporter): reporter : IStepsReporter Object used to report progress. """ - reporter.start("Starting processing", steps=target) + reporter.start("Starting processing", total=target) for i in range(target): reporter.step(f"processing item {i+1} of {target}") time.sleep(sleep) @@ -52,25 +60,31 @@ class MyView(HasStrictTraits): def _executor_default(self): return TraitsExecutor() # worker_pool=self.cf_executor) - dialogs = List(ProgressFutureDialog) + dialogs = List(StepsDialog) cf_executor = Any() - def _calculate_fired(self): + @observe("calculate") + def _open_modal_dialog(self, event): target = self.target - future = submit_steps(self.executor, count, target=target) - dialog = ProgressFutureDialog( - progress_future=future, + future = submit_steps( + self.executor, target, "Counting...", count, target=target + ) + dialog = StepsDialog( + future=future, style="modal", # this is the default title=f"Counting to {target}", ) dialog.open() - def _nonmodal_fired(self): + @observe("nonmodal") + def _open_nonmodal_dialog(self, event): target = self.target - future = submit_steps(self.executor, count, target) - dialog = ProgressFutureDialog( - progress_future=future, + future = submit_steps( + self.executor, target, "Counting...", count, target + ) + dialog = StepsDialog( + future=future, style="nonmodal", # this is the default title=f"Counting to {target}", ) diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index f7ef9a3c..e6286e22 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -19,36 +19,49 @@ """ import abc - -from traits.api import Callable, Dict, HasStrictTraits, Int, Str, Tuple, Union +import collections + +from traits.api import ( + Callable, + Dict, + HasStrictTraits, + Instance, + Int, + observe, + Property, + Str, + Tuple, + Union, +) from traits_futures.base_future import BaseFuture, BaseTask, TaskCancelled from traits_futures.i_task_specification import ITaskSpecification -# XXX Add NullReporter, for convenience in testing. -# XXX Add "check" method to just check for cancellation. -# XXX In the StepsFuture, update all traits before sending trait-change -# notifications. (Is this possible?) -# XXX Consider renaming "_step", "_total", etc. -# XXX Consider namedtuple encapsulating progress state. -# XXX Rethink setting of message - should only happen when a message is -# provided? +# XXX Add NullReporter, for convenience in testing progress-reporting tasks +# - later? +# XXX Add "check" method to just check for cancellation - later? + + +#: Tuple encapsulating the entire progress state of the task. +StepsState = collections.namedtuple( + "StepsState", ["total", "complete", "pending", "message"] +) class IStepsReporter(abc.ABC): """Interface for step-reporting object passed to the background job.""" @abc.abstractmethod - def start(self, message=None, *, steps=None): + def start(self, message=None, *, total=None): """ - Set an initial message and set the total number of steps. + Set an initial message and set the progress total. Parameters ---------- message : str, optional Message to set at start time. - steps : int, optional - Number of steps, if known. + total : int, optional + Total progress, in whatever units makes sense for the application. Raises ------ @@ -80,7 +93,7 @@ def stop(self, message=None): """ Report that processing is complete. - Also updates the total number of steps completed. + Also updates the total progress, if any tasks are pending. Parameters ---------- @@ -93,13 +106,9 @@ def stop(self, message=None): # Message types for messages sent from the background to the future. -#: Message sent on a start or step operation. Argument is a tuple -#: (step, steps, message), with: -#: -#: * step: int - number of completed steps so far -#: * steps: int or None - total number of steps, if known -#: * message: str or None - message to display for this step -STEP = "step" +#: Message sent on a start or step operation. Argument is an instance +#: of StepsState. +UPDATE = "update" @IStepsReporter.register @@ -113,45 +122,39 @@ class StepsReporter: """ - def __init__(self, send, cancelled): - # Communications support. + def __init__(self, send, cancelled, initial_steps_state): + #: Callable used to send messages to the linked future. self._send = send - self._cancelled = cancelled - - # Set up internal state. - - #: Total number of steps completed. - self._step = 0 - #: Total number of steps, if known. None if not known. - self._steps = None - - #: Description for the currently-in-progress step, or None. - self._message = None + #: Callable used to check for task cancellation. + self._cancelled = cancelled - #: Size of the currently executing step. None if no step - #: is currently executing. - self._size = None + #: Progress state. + self._steps_state = initial_steps_state - def start(self, message=None, *, steps=None): + def start(self, message=None, *, total=None): """ - Set an initial message and set the total number of steps. + Set an initial message and set the progress total. Parameters ---------- message : str, optional Message to set at start time. - steps : int, optional - Number of steps, if known. + total : int, optional + Total progress, in whatever units makes sense for the application. Raises ------ TaskCancelled If the user has called ``cancel()`` before this. """ - self._steps = steps - self._message = message - self._send(STEP, (self._step, self._steps, self._message)) + self._check_cancel() + + self._steps_state = self._steps_state._replace( + total=self._steps_state.total if total is None else total, + message=self._steps_state.message if message is None else message, + ) + self._report_state() def step(self, message=None, *, size=1): """ @@ -162,8 +165,8 @@ def step(self, message=None, *, size=1): message : str, optional A description of this step. size : int, optional - The size of this step (in whatever units are being used). - Defaults to 1. + The size of this step (in whatever units make sense for the + application at hand). Defaults to 1. Raises ------ @@ -172,20 +175,19 @@ def step(self, message=None, *, size=1): """ self._check_cancel() - # Close the previous step, if any. - if self._size is not None: - self._step += self._size - self._size = None + self._steps_state = self._steps_state._replace( + complete=self._steps_state.complete + self._steps_state.pending, + pending=size, + message=self._steps_state.message if message is None else message, + ) - self._message = message - self._size = size - self._send(STEP, (self._step, self._steps, self._message)) + self._report_state() def stop(self, message=None): """ Report that processing is complete. - Also updates the total number of steps completed. + Also updates the total progress, if any tasks are pending. Parameters ---------- @@ -196,23 +198,29 @@ def stop(self, message=None): """ self._check_cancel() - # Close the previous step, if any. - if self._size is not None: - self._step += self._size - self._size = None - - self._message = message - self._send(STEP, (self._step, self._steps, self._message)) + self._steps_state = self._steps_state._replace( + complete=self._steps_state.complete + self._steps_state.pending, + pending=0, + message=self._steps_state.message if message is None else message, + ) + self._report_state() - # Private methods ######################################################### + # Private methods and properties ########################################## def _close(self): - if self._size is not None: - self._step += self._size - self._size = None - self._message = None + if self._steps_state.pending: + self._steps_state = StepsState( + total=self._steps_state.total, + complete=self._steps_state.complete + + self._steps_state.pending, + pending=0, + message=self._steps_state.message, + ) + + self._report_state() - self._send(STEP, (self._step, self._steps, self._message)) + def _report_state(self): + self._send(UPDATE, self._steps_state) def _check_cancel(self): """Check if the task has been cancelled. @@ -234,13 +242,18 @@ class StepsTask(BaseTask): the task on completion. """ - def __init__(self, callable, args, kwargs): + def __init__(self, initial_steps_state, callable, args, kwargs): self.callable = callable self.args = args self.kwargs = kwargs + self.initial_steps_state = initial_steps_state def run(self): - reporter = StepsReporter(send=self.send, cancelled=self.cancelled) + reporter = StepsReporter( + send=self.send, + cancelled=self.cancelled, + initial_steps_state=self.initial_steps_state, + ) try: result = self.callable( *self.args, @@ -256,36 +269,73 @@ def run(self): class StepsFuture(BaseFuture): """ - Object representing the front-end handle to a background call. + Object representing the front-end handle to a background steps task. """ - #: Total number of steps, if known. None if not known. - steps = Union(None, Int()) + #: Most recently received message from the background task. + message = Property(Union(None, Str())) - #: The most recently completed step, if any. - step = Int(0) + #: Total work, in whatever units make sense for the application. + total = Property(Union(None, Int())) - #: Most recently received message from the background task. - message = Union(None, Str()) + #: Units of work completed so far. + complete = Property(Int()) + + # Private traits ########################################################## + + #: The progress state of the background task. + _steps_state = Instance(StepsState, allow_none=False) # Private methods ######################################################### - def _process_step(self, step_event): + def _process_update(self, steps_state): """ - Process a STEP message from the background task. + Process an UPDATE message from the background task. """ - step, steps, message = step_event - self.message = message - self.steps = steps - self.step = step + self._steps_state = steps_state + + def _get_message(self): + return self._steps_state.message + + def _get_total(self): + return self._steps_state.total + + def _get_complete(self): + return self._steps_state.complete + + @observe("_steps_state") + def _update_state_traits(self, event): + if event.old is None: + return + + old_state, new_state = event.old, event.new + + if old_state.message != new_state.message: + self.trait_property_changed( + "message", old_state.message, new_state.message + ) + if old_state.total != new_state.total: + self.trait_property_changed( + "total", old_state.total, new_state.total + ) + if old_state.complete != new_state.complete: + self.trait_property_changed( + "complete", old_state.complete, new_state.complete + ) @ITaskSpecification.register class BackgroundSteps(HasStrictTraits): """ - Object representing the background call to be executed. + Object representing the background task to be executed. """ + #: Total units of work for the task, if known. None if not known. + total = Union(None, Int()) + + #: Initial message. + message = Union(None, Str()) + #: The callable for the task. callable = Callable() @@ -316,7 +366,10 @@ def future(self, cancel): Future object that can be used to monitor the status of the background task. """ - return StepsFuture(_cancel=cancel) + return StepsFuture( + _cancel=cancel, + _steps_state=self._initial_steps_state, + ) def task(self): """ @@ -330,30 +383,51 @@ def task(self): check whether cancellation has been requested. """ return StepsTask( + initial_steps_state=self._initial_steps_state, callable=self.callable, args=self.args, kwargs=self.kwargs, ) + # Private methods ######################################################### + + @property + def _initial_steps_state(self): + return StepsState( + total=self.total, + complete=0, + pending=0, + message=self.message, + ) -def submit_steps(executor, callable, *args, **kwargs): + +def submit_steps(executor, total, message, callable, *args, **kwargs): """ Convenience function to submit a BackgroundSteps task to an executor. + Note: the 'executor', 'total', 'message', and 'callable' parameters should + be treated as positional only, and should always be passed by position + instead of by name. Future versions of the library may enforce this + restriction. + Parameters ---------- executor : TraitsExecutor Executor to submit the task to. + total : int or None + Total units of work for this task, if known. + message : str or None + Initial message. callable : collections.abc.Callable Callable to execute in the background. This should accept a - "progress" keyword argument, in addition to any other positional + "reporter" keyword argument, in addition to any other positional and named arguments it needs. When the callable is invoked, an - instance of "StepsReporter" will be supplied via that "progress" + instance of "StepsReporter" will be supplied via that "reporter" argument. *args Positional arguments to pass to the callable. **kwargs - Named arguments to pass to the callable, excluding the "progress" + Named arguments to pass to the callable, excluding the "reporter" argument. That argument will be passed separately. Returns @@ -369,6 +443,8 @@ def submit_steps(executor, callable, *args, **kwargs): return executor.submit( BackgroundSteps( + total=total, + message=message, callable=callable, args=args, kwargs=kwargs, diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index 519d5736..19b94936 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -12,11 +12,19 @@ from traits.api import HasStrictTraits, Instance, List, observe from traits_futures.api import ( + CANCELLED, COMPLETED, + FAILED, IStepsReporter, StepsFuture, submit_steps, ) +from traits_futures.exception_handling import _qualified_type_name + +#: Maximum timeout for blocking calls, in seconds. A successful test should +#: never hit this timeout - it's there to prevent a failing test from hanging +#: forever and blocking the rest of the test suite. +SAFETY_TIMEOUT = 5.0 class StepsListener(HasStrictTraits): @@ -32,23 +40,23 @@ class StepsListener(HasStrictTraits): def _capture_initial_state(self, event): future = event.new self.messages.append(("message", future.message)) - self.messages.append(("step", future.step)) - self.messages.append(("steps", future.steps)) + self.messages.append(("total", future.total)) + self.messages.append(("complete", future.complete)) - @observe("future:message,future:step,future:steps") + @observe("future:message,future:total,future:complete") def _update_messages(self, event): self.messages.append((event.name, event.new)) # State is: -# - step: number of completed steps -# - steps: total number of steps +# - total: total work, in whatever units make sense +# - complete: units of work complete # - message: description of step currently in progress # XXX Simplest use-case: no calls to progress at all; just a long-running task. # XXX Next simplest: progress.step("uploading file 1"), # progress.step("uploading file 2") -# XXX Next simplest: progress.steps = 2; progress.step("..."), +# XXX Next simplest: progress.total = 2; progress.step("..."), # progress.step("...") # XXX Advanced: progress.sync() / progress.update() / .... after manual # changes. @@ -61,26 +69,58 @@ def test_reporter_implements_i_steps_reporter(self): def check_steps_reporter_interface(reporter): return isinstance(reporter, IStepsReporter) - future = submit_steps(self.executor, check_steps_reporter_interface) + future = submit_steps( + self.executor, None, None, check_steps_reporter_interface + ) result = self.wait_for_result(future) self.assertTrue(result) - def test_reporter_passed_by_name(self): + def test_reporter_is_passed_by_name(self): def reporter_by_name(*, reporter): return 46 - future = submit_steps(self.executor, reporter_by_name) + future = submit_steps(self.executor, None, None, reporter_by_name) self.assertEqual(self.wait_for_result(future), 46) + def test_result(self): + def return_a_value(reporter): + return 45 + + future = submit_steps(self.executor, None, None, return_a_value) + self.assertEqual(self.wait_for_result(future), 45) + + def test_error(self): + def raise_an_error(reporter): + 1 / 0 + + future = submit_steps(self.executor, None, None, raise_an_error) + self.assertEqual( + self.wait_for_exception(future), + _qualified_type_name(ZeroDivisionError), + ) + + def test_state_changes_no_reports(self): + def return_a_value(reporter): + return 45 + + future = submit_steps(self.executor, None, None, return_a_value) + listener = StepsListener(future=future) + self.wait_for_result(future) + + expected_messages = [ + dict(message=None, total=None, complete=0), + ] + self.check_messages(listener.messages, expected_messages) + def test_initial_values(self): def do_nothing(reporter): return 23 - future = submit_steps(self.executor, do_nothing) + future = submit_steps(self.executor, None, None, do_nothing) self.assertIsNone(future.message) - self.assertIsNone(future.steps) - self.assertEqual(future.step, 0) + self.assertIsNone(future.total) + self.assertEqual(future.complete, 0) result = self.wait_for_result(future) self.assertEqual(result, 23) @@ -91,44 +131,66 @@ def send_messages(reporter): reporter.step("Uploading file 2") reporter.stop("Finished") - future = submit_steps(self.executor, send_messages) + future = submit_steps(self.executor, None, None, send_messages) listener = StepsListener(future=future) self.wait_for_result(future) expected_messages = [ # Initial values - dict(message=None, steps=None, step=0), + dict(message=None, total=None, complete=0), # Updates on start of first step dict(message="Uploading file 1"), # Updates on start of second step - dict(message="Uploading file 2", step=1), + dict(message="Uploading file 2", complete=1), # Updates on completion. - dict(message="Finished", step=2), + dict(message="Finished", complete=2), ] self.check_messages(listener.messages, expected_messages) def test_set_total(self): def send_messages(reporter): - reporter.start(steps=2) + reporter.start(total=2) reporter.step("Uploading file 1") reporter.step("Uploading file 2") reporter.stop("All done") - future = submit_steps(self.executor, send_messages) + future = submit_steps(self.executor, None, None, send_messages) listener = StepsListener(future=future) self.wait_for_result(future) expected_messages = [ # Initial values - dict(message=None, steps=None, step=0), - # Updates on setting 'steps' to 2. - dict(steps=2), + dict(message=None, total=None, complete=0), + # Updates on setting 'total' to 2. + dict(total=2), # Updates on start of first step dict(message="Uploading file 1"), # Updates on start of second step - dict(message="Uploading file 2", step=1), + dict(message="Uploading file 2", complete=1), # Updates on completion. - dict(message="All done", step=2), + dict(message="All done", complete=2), + ] + self.check_messages(listener.messages, expected_messages) + + def test_irregular_step_sizes(self): + def send_messages(reporter): + reporter.start("Uploading files...", total=10) + reporter.step("Uploading file 1", size=2) + reporter.step("Uploading file 2", size=5) + reporter.step("Uploading file 3", size=3) + reporter.stop("Uploads complete") + + future = submit_steps(self.executor, None, None, send_messages) + listener = StepsListener(future=future) + self.wait_for_result(future) + + expected_messages = [ + dict(message=None, total=None, complete=0), + dict(message="Uploading files...", total=10), + dict(message="Uploading file 1"), + dict(message="Uploading file 2", complete=2), + dict(message="Uploading file 3", complete=7), + dict(message="Uploads complete", complete=10), ] self.check_messages(listener.messages, expected_messages) @@ -136,25 +198,92 @@ def test_no_stop(self): # If the user doesn't call stop, we should still get a final update # that sets the 'step'. def send_messages(reporter): - reporter.start(steps=2) + reporter.start(total=2) reporter.step("Uploading file 1") reporter.step("Uploading file 2") - future = submit_steps(self.executor, send_messages) + future = submit_steps(self.executor, None, None, send_messages) listener = StepsListener(future=future) self.wait_for_result(future) expected_messages = [ # Initial values - dict(message=None, steps=None, step=0), - # Updates on setting 'steps' to 2. - dict(steps=2), + dict(message=None, total=None, complete=0), + # Updates on setting 'total' to 2. + dict(total=2), # Updates on start of first step dict(message="Uploading file 1"), # Updates on start of second step - dict(message="Uploading file 2", step=1), + dict(message="Uploading file 2", complete=1), # Updates on completion. - dict(message=None, step=2), + dict(complete=2), + ] + + self.check_messages(listener.messages, expected_messages) + + # XXX test cancellation on start, and cancellation on stop + + def test_cancellation_on_step(self): + checkpoint = self._context.event() + barrier = self._context.event() + detector = self._context.event() + + def send_messages(checkpoint, barrier, reporter, detector): + reporter.start(total=2) + reporter.step("Uploading file 1") + checkpoint.set() + # Test will cancel at this point. + barrier.wait(timeout=SAFETY_TIMEOUT) + # Call to 'step' should abandon execution. + reporter.step("Uploading file 2") + detector.set() + + future = submit_steps( + self.executor, + None, + None, + send_messages, + checkpoint=checkpoint, + barrier=barrier, + detector=detector, + ) + listener = StepsListener(future=future) + + checkpoint.wait(timeout=SAFETY_TIMEOUT) + future.cancel() + barrier.set() + + self.wait_for_cancelled(future) + + self.assertFalse(detector.is_set()) + + expected_messages = [ + # Initial values + dict(message=None, total=None, complete=0), + ] + self.check_messages(listener.messages, expected_messages) + + def test_initial_total(self): + # Exercise the case where we set the total and message up front. + def send_messages(reporter): + reporter.step("Uploading file 1") + reporter.step("Uploading file 2") + + future = submit_steps( + self.executor, 2, "Uploading files", send_messages + ) + listener = StepsListener(future=future) + self.wait_for_result(future) + + expected_messages = [ + # Initial values + dict(message="Uploading files", total=2, complete=0), + # Updates on start of first step + dict(message="Uploading file 1"), + # Updates on start of second step + dict(message="Uploading file 2", complete=1), + # Updates on completion. + dict(complete=2), ] self.check_messages(listener.messages, expected_messages) @@ -190,5 +319,21 @@ def wait_for_result(self, future): self.run_until(future, "done", lambda future: future.done) if future.state == COMPLETED: return future.result - else: - self.fail(f"Future did not return a result: {future.exception}") + elif future.state == FAILED: + exc_type, exc_value, exc_traceback = future.exception + self.fail( + f"Task failed with exception of type: {exc_type}\n" + f"{exc_traceback}" + ) + elif future.state == CANCELLED: + self.fail("Task did not return a result because it was cancelled.") + + def wait_for_exception(self, future): + self.run_until(future, "done", lambda future: future.done) + exc_type, exc_value, exc_traceback = future.exception + return exc_type + + def wait_for_cancelled(self, future): + self.run_until(future, "done", lambda future: future.done) + if future.state != CANCELLED: + raise RuntimeError("Future was not cancelled") From 2b6196a76cfaf917a075ebdbbd3250fc2bef6ccb Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Mon, 16 Aug 2021 17:26:36 +0100 Subject: [PATCH 27/28] Cleanup and updates --- docs/source/guide/examples/steps_dialogs.py | 14 +- traits_futures/background_steps.py | 290 +++++++------ .../tests/background_steps_tests.py | 406 +++++++++--------- 3 files changed, 378 insertions(+), 332 deletions(-) diff --git a/docs/source/guide/examples/steps_dialogs.py b/docs/source/guide/examples/steps_dialogs.py index a9f74d6d..8910c6d5 100644 --- a/docs/source/guide/examples/steps_dialogs.py +++ b/docs/source/guide/examples/steps_dialogs.py @@ -12,6 +12,11 @@ Demo script for modal progress dialog. """ +# XXX To do list (including for StepsFutures) +# * Add NullReporter implementation of IStepsReporter, for convenience in +# testing progress-reporting tasks. +# * Add IStepsReporter "check" method that simply checks for cancellation. + import concurrent.futures import time @@ -41,7 +46,6 @@ def count(target, *, sleep=1.0, reporter): reporter : IStepsReporter Object used to report progress. """ - reporter.start("Starting processing", total=target) for i in range(target): reporter.step(f"processing item {i+1} of {target}") time.sleep(sleep) @@ -68,7 +72,11 @@ def _executor_default(self): def _open_modal_dialog(self, event): target = self.target future = submit_steps( - self.executor, target, "Counting...", count, target=target + self.executor, + target, + "Starting processing...", + count, + target=target, ) dialog = StepsDialog( future=future, @@ -81,7 +89,7 @@ def _open_modal_dialog(self, event): def _open_nonmodal_dialog(self, event): target = self.target future = submit_steps( - self.executor, target, "Counting...", count, target + self.executor, target, "Starting processing...", count, target ) dialog = StepsDialog( future=future, diff --git a/traits_futures/background_steps.py b/traits_futures/background_steps.py index e6286e22..b2776940 100644 --- a/traits_futures/background_steps.py +++ b/traits_futures/background_steps.py @@ -15,7 +15,6 @@ that execute in the background and report progress information to the foreground. The points at which progress is reported also represent points at which the task can be interrupted. - """ import abc @@ -31,56 +30,27 @@ Property, Str, Tuple, - Union, ) from traits_futures.base_future import BaseFuture, BaseTask, TaskCancelled from traits_futures.i_task_specification import ITaskSpecification -# XXX Add NullReporter, for convenience in testing progress-reporting tasks -# - later? -# XXX Add "check" method to just check for cancellation - later? - - -#: Tuple encapsulating the entire progress state of the task. -StepsState = collections.namedtuple( - "StepsState", ["total", "complete", "pending", "message"] -) - class IStepsReporter(abc.ABC): """Interface for step-reporting object passed to the background job.""" @abc.abstractmethod - def start(self, message=None, *, total=None): - """ - Set an initial message and set the progress total. - - Parameters - ---------- - message : str, optional - Message to set at start time. - total : int, optional - Total progress, in whatever units makes sense for the application. - - Raises - ------ - TaskCancelled - If the user has called ``cancel()`` before this. - """ - - @abc.abstractmethod - def step(self, message=None, *, size=1): + def step(self, message, *, size=1): """ Start a processing step. Parameters ---------- - message : str, optional + message : str A description of this step. size : int, optional - The size of this step (in whatever units make sense for the - application at hand). Defaults to 1. + The size of this step, in whatever units make sense for the + application at hand. Defaults to 1. Raises ------ @@ -89,7 +59,7 @@ def step(self, message=None, *, size=1): """ @abc.abstractmethod - def stop(self, message=None): + def stop(self, message): """ Report that processing is complete. @@ -97,72 +67,127 @@ def stop(self, message=None): Parameters ---------- - message : str, optional + message : str Message to display on completion. For a progress dialog that disappears on completion, this message will never be seen by the user, but for other views the message may be visible. """ -# Message types for messages sent from the background to the future. - -#: Message sent on a start or step operation. Argument is an instance -#: of StepsState. -UPDATE = "update" - - -@IStepsReporter.register -class StepsReporter: +class StepsState( + collections.namedtuple( + "StepsState", ["total", "complete", "pending", "message"] + ) +): """ - Object used by the background task to report progress information. + Tuple subclass encapsulating progress state of the task. - A :class:`StepsReporter` instance is passed to the background task, - and its ``start`` and ``step`` methods can be used by that - background task to report progress. + Objects of this type capture the progress state of an in-progress task. + Attributes + ---------- + total : int + Total number of units of work for the task. + complete : int + Total units of work completed. + pending : int + Size of the step currently in progress, or 0 if there's no + in-progress step. + message : str + Description of the current step, or a more general message + if processing has completed or has yet to start. """ - def __init__(self, send, cancelled, initial_steps_state): - #: Callable used to send messages to the linked future. - self._send = send + @classmethod + def initial(cls, total, message): + """ + Initial state, given total work and initial message. - #: Callable used to check for task cancellation. - self._cancelled = cancelled + Parameters + ---------- + total : int + Total units of work. + message : str + Message to use for the initial state. - #: Progress state. - self._steps_state = initial_steps_state + Returns + ------- + StepsState + """ + return cls(total=total, complete=0, pending=0, message=message) - def start(self, message=None, *, total=None): + def set_message(self, message): """ - Set an initial message and set the progress total. + Return a copy of this state with an updated message. Parameters ---------- - message : str, optional - Message to set at start time. - total : int, optional - Total progress, in whatever units makes sense for the application. + message : str + Message to use for the new state. - Raises - ------ - TaskCancelled - If the user has called ``cancel()`` before this. + Returns + ------- + StepsState """ - self._check_cancel() + return self._replace(message=message) + + def set_step(self, size): + """ + Return a copy of this state updated for the next processing step. + + Parameters + ---------- + size : int + Number of units of work represented by the next step. - self._steps_state = self._steps_state._replace( - total=self._steps_state.total if total is None else total, - message=self._steps_state.message if message is None else message, + Returns + ------- + StepsState + """ + return self._replace( + complete=self.complete + self.pending, + pending=size, ) - self._report_state() - def step(self, message=None, *, size=1): + +#: Message type for the message sent on each state update. The argument is an +#: instance of StepsState. +UPDATE = "update" + + +@IStepsReporter.register +class StepsReporter: + """ + Object used by the background task to report progress information. + + A :class:`StepsReporter` instance is passed to the background task, and its + ``step`` and ``stop`` methods can be used by that background task to report + progress. + + Parameters + ---------- + send : callable + Callable provided by the Traits Futures machinery, and used to send + messages to the linked future. + cancelled : callable + Callable provided by the Traits Futures machinery, and used to check + for cancellation requests. + state : StepsState + Initial state of the reporter. + """ + + def __init__(self, send, cancelled, state): + self._send = send + self._cancelled = cancelled + self._state = state + + def step(self, message, *, size=1): """ Start a processing step. Parameters ---------- - message : str, optional + message : str A description of this step. size : int, optional The size of this step (in whatever units make sense for the @@ -174,16 +199,10 @@ def step(self, message=None, *, size=1): If the user has called ``cancel()`` before this. """ self._check_cancel() + self._state = self._state.set_step(size).set_message(message) + self._send(UPDATE, self._state) - self._steps_state = self._steps_state._replace( - complete=self._steps_state.complete + self._steps_state.pending, - pending=size, - message=self._steps_state.message if message is None else message, - ) - - self._report_state() - - def stop(self, message=None): + def stop(self, message): """ Report that processing is complete. @@ -191,37 +210,17 @@ def stop(self, message=None): Parameters ---------- - message : str, optional + message : str Message to display on completion. For a progress dialog that disappears on completion, this message will never be seen by the user, but for other views the message may be visible. """ self._check_cancel() - - self._steps_state = self._steps_state._replace( - complete=self._steps_state.complete + self._steps_state.pending, - pending=0, - message=self._steps_state.message if message is None else message, - ) - self._report_state() + self._state = self._state.set_step(0).set_message(message) + self._send(UPDATE, self._state) # Private methods and properties ########################################## - def _close(self): - if self._steps_state.pending: - self._steps_state = StepsState( - total=self._steps_state.total, - complete=self._steps_state.complete - + self._steps_state.pending, - pending=0, - message=self._steps_state.message, - ) - - self._report_state() - - def _report_state(self): - self._send(UPDATE, self._steps_state) - def _check_cancel(self): """Check if the task has been cancelled. @@ -240,19 +239,40 @@ class StepsTask(BaseTask): This wrapper handles capturing exceptions and sending the final status of the task on completion. + + Parameters + ---------- + initial_state : StepsState + Initial state of the progress. + callable + User-supplied function to be called. + args : tuple + Positional arguments to be passed to ``callable``. + kwargs : dict + Named arguments to be passed to ``callable``, not including the + ``reporter`` argument. """ - def __init__(self, initial_steps_state, callable, args, kwargs): + def __init__(self, initial_state, callable, args, kwargs): self.callable = callable self.args = args self.kwargs = kwargs - self.initial_steps_state = initial_steps_state + self.initial_state = initial_state def run(self): + """ + Run the body of the steps task. + + Returns + ------- + object + May return any object. That object will be delivered to the + future's ``result`` attribute. + """ reporter = StepsReporter( send=self.send, cancelled=self.cancelled, - initial_steps_state=self.initial_steps_state, + state=self.initial_state, ) try: result = self.callable( @@ -260,7 +280,6 @@ def run(self): **self.kwargs, reporter=reporter, ) - reporter._close() except TaskCancelled: return None else: @@ -273,10 +292,10 @@ class StepsFuture(BaseFuture): """ #: Most recently received message from the background task. - message = Property(Union(None, Str())) + message = Property(Str()) #: Total work, in whatever units make sense for the application. - total = Property(Union(None, Int())) + total = Property(Int()) #: Units of work completed so far. complete = Property(Int()) @@ -284,26 +303,29 @@ class StepsFuture(BaseFuture): # Private traits ########################################################## #: The progress state of the background task. - _steps_state = Instance(StepsState, allow_none=False) + _progress_state = Instance(StepsState, allow_none=False) # Private methods ######################################################### - def _process_update(self, steps_state): + def _process_update(self, progress_state): """ Process an UPDATE message from the background task. """ - self._steps_state = steps_state + self._progress_state = progress_state def _get_message(self): - return self._steps_state.message + """Traits property getter for the 'message' trait.""" + return self._progress_state.message def _get_total(self): - return self._steps_state.total + """Traits property getter for the 'total' trait.""" + return self._progress_state.total def _get_complete(self): - return self._steps_state.complete + """Traits property getter for the 'complete' property.""" + return self._progress_state.complete - @observe("_steps_state") + @observe("_progress_state") def _update_state_traits(self, event): if event.old is None: return @@ -331,10 +353,10 @@ class BackgroundSteps(HasStrictTraits): """ #: Total units of work for the task, if known. None if not known. - total = Union(None, Int()) + total = Int() #: Initial message. - message = Union(None, Str()) + message = Str() #: The callable for the task. callable = Callable() @@ -368,7 +390,7 @@ def future(self, cancel): """ return StepsFuture( _cancel=cancel, - _steps_state=self._initial_steps_state, + _progress_state=self._initial_state, ) def task(self): @@ -383,7 +405,7 @@ def task(self): check whether cancellation has been requested. """ return StepsTask( - initial_steps_state=self._initial_steps_state, + initial_state=self._initial_state, callable=self.callable, args=self.args, kwargs=self.kwargs, @@ -392,13 +414,8 @@ def task(self): # Private methods ######################################################### @property - def _initial_steps_state(self): - return StepsState( - total=self.total, - complete=0, - pending=0, - message=self.message, - ) + def _initial_state(self): + return StepsState.initial(total=self.total, message=self.message) def submit_steps(executor, total, message, callable, *args, **kwargs): @@ -406,18 +423,19 @@ def submit_steps(executor, total, message, callable, *args, **kwargs): Convenience function to submit a BackgroundSteps task to an executor. Note: the 'executor', 'total', 'message', and 'callable' parameters should - be treated as positional only, and should always be passed by position - instead of by name. Future versions of the library may enforce this - restriction. + always be passed by position instead of by name. Future versions of the + library may enforce this restriction. Parameters ---------- executor : TraitsExecutor Executor to submit the task to. - total : int or None - Total units of work for this task, if known. - message : str or None - Initial message. + total : int + Total units of work for this task, in whatever units are appropriate + for the task in hand. + message : str + Description of the task. This will be used until the first step + message is received from the background task. callable : collections.abc.Callable Callable to execute in the background. This should accept a "reporter" keyword argument, in addition to any other positional diff --git a/traits_futures/tests/background_steps_tests.py b/traits_futures/tests/background_steps_tests.py index 19b94936..d6ad425c 100644 --- a/traits_futures/tests/background_steps_tests.py +++ b/traits_futures/tests/background_steps_tests.py @@ -9,7 +9,16 @@ # Thanks for using Enthought open source! -from traits.api import HasStrictTraits, Instance, List, observe +from traits.api import ( + HasStrictTraits, + Instance, + Int, + List, + observe, + Str, + Tuple, + Union, +) from traits_futures.api import ( CANCELLED, @@ -19,7 +28,6 @@ StepsFuture, submit_steps, ) -from traits_futures.exception_handling import _qualified_type_name #: Maximum timeout for blocking calls, in seconds. A successful test should #: never hit this timeout - it's there to prevent a failing test from hanging @@ -27,41 +35,33 @@ SAFETY_TIMEOUT = 5.0 +#: Trait type for the progress state: total, complete, message. +ProgressState = Tuple(Union(None, Int()), Int(), Union(None, Str())) + + class StepsListener(HasStrictTraits): """ Listener recording all state changes for a StepsFuture. """ - messages = List() - + #: The future we're listening to. future = Instance(StepsFuture) - @observe("future") - def _capture_initial_state(self, event): - future = event.new - self.messages.append(("message", future.message)) - self.messages.append(("total", future.total)) - self.messages.append(("complete", future.complete)) - - @observe("future:message,future:total,future:complete") - def _update_messages(self, event): - self.messages.append((event.name, event.new)) - + #: The progress state. + state = Union(None, ProgressState) -# State is: -# - total: total work, in whatever units make sense -# - complete: units of work complete -# - message: description of step currently in progress + #: All recorded states, including the initial state. + states = List(ProgressState) -# XXX Simplest use-case: no calls to progress at all; just a long-running task. -# XXX Next simplest: progress.step("uploading file 1"), -# progress.step("uploading file 2") -# XXX Next simplest: progress.total = 2; progress.step("..."), -# progress.step("...") -# XXX Advanced: progress.sync() / progress.update() / .... after manual -# changes. + @observe("future.[total,complete,message]") + def _record_state(self, event): + future = event.new if event.name == "future" else event.object + self.state = future.total, future.complete, future.message -# XXX Does the steps reporter actually _need_ to be a `HasStrictTraits` class? + @observe("state") + def _append_new_state(self, event): + """Record the new state whenever it changes.""" + self.states.append(event.new) class BackgroundStepsTests: @@ -70,60 +70,51 @@ def check_steps_reporter_interface(reporter): return isinstance(reporter, IStepsReporter) future = submit_steps( - self.executor, None, None, check_steps_reporter_interface + self.executor, 0, "Checking", check_steps_reporter_interface ) - result = self.wait_for_result(future) - self.assertTrue(result) + self.assertTaskEventuallyCompletes(future, True) def test_reporter_is_passed_by_name(self): def reporter_by_name(*, reporter): return 46 - future = submit_steps(self.executor, None, None, reporter_by_name) - self.assertEqual(self.wait_for_result(future), 46) + future = submit_steps( + self.executor, 0, "Doing nothing", reporter_by_name + ) + self.assertTaskEventuallyCompletes(future, 46) def test_result(self): def return_a_value(reporter): return 45 - future = submit_steps(self.executor, None, None, return_a_value) - self.assertEqual(self.wait_for_result(future), 45) + future = submit_steps( + self.executor, 0, "Returning a value", return_a_value + ) + self.assertTaskEventuallyCompletes(future, 45) def test_error(self): def raise_an_error(reporter): 1 / 0 - future = submit_steps(self.executor, None, None, raise_an_error) - self.assertEqual( - self.wait_for_exception(future), - _qualified_type_name(ZeroDivisionError), - ) + future = submit_steps(self.executor, 0, "Raising", raise_an_error) + self.assertTaskEventuallyFails(future, ZeroDivisionError) def test_state_changes_no_reports(self): def return_a_value(reporter): return 45 - future = submit_steps(self.executor, None, None, return_a_value) + future = submit_steps( + self.executor, 0, "Doing nothing", return_a_value + ) listener = StepsListener(future=future) - self.wait_for_result(future) - - expected_messages = [ - dict(message=None, total=None, complete=0), - ] - self.check_messages(listener.messages, expected_messages) - - def test_initial_values(self): - def do_nothing(reporter): - return 23 + self.assertTaskEventuallyCompletes(future, 45) - future = submit_steps(self.executor, None, None, do_nothing) - - self.assertIsNone(future.message) - self.assertIsNone(future.total) - self.assertEqual(future.complete, 0) - - result = self.wait_for_result(future) - self.assertEqual(result, 23) + self.assertEqual( + listener.states, + [ + (0, 0, "Doing nothing"), + ], + ) def test_simple_messages(self): def send_messages(reporter): @@ -131,178 +122,188 @@ def send_messages(reporter): reporter.step("Uploading file 2") reporter.stop("Finished") - future = submit_steps(self.executor, None, None, send_messages) - listener = StepsListener(future=future) - self.wait_for_result(future) - - expected_messages = [ - # Initial values - dict(message=None, total=None, complete=0), - # Updates on start of first step - dict(message="Uploading file 1"), - # Updates on start of second step - dict(message="Uploading file 2", complete=1), - # Updates on completion. - dict(message="Finished", complete=2), - ] - self.check_messages(listener.messages, expected_messages) - - def test_set_total(self): - def send_messages(reporter): - reporter.start(total=2) - reporter.step("Uploading file 1") - reporter.step("Uploading file 2") - reporter.stop("All done") - - future = submit_steps(self.executor, None, None, send_messages) + future = submit_steps( + self.executor, 2, "Uploading files", send_messages + ) listener = StepsListener(future=future) - self.wait_for_result(future) - - expected_messages = [ - # Initial values - dict(message=None, total=None, complete=0), - # Updates on setting 'total' to 2. - dict(total=2), - # Updates on start of first step - dict(message="Uploading file 1"), - # Updates on start of second step - dict(message="Uploading file 2", complete=1), - # Updates on completion. - dict(message="All done", complete=2), - ] - self.check_messages(listener.messages, expected_messages) + self.assertTaskEventuallyCompletes(future, None) + self.assertEqual( + listener.states, + [ + (2, 0, "Uploading files"), + (2, 0, "Uploading file 1"), + (2, 1, "Uploading file 2"), + (2, 2, "Finished"), + ], + ) def test_irregular_step_sizes(self): def send_messages(reporter): - reporter.start("Uploading files...", total=10) reporter.step("Uploading file 1", size=2) reporter.step("Uploading file 2", size=5) reporter.step("Uploading file 3", size=3) reporter.stop("Uploads complete") - future = submit_steps(self.executor, None, None, send_messages) + future = submit_steps( + self.executor, 10, "Uploading files...", send_messages + ) listener = StepsListener(future=future) - self.wait_for_result(future) - - expected_messages = [ - dict(message=None, total=None, complete=0), - dict(message="Uploading files...", total=10), - dict(message="Uploading file 1"), - dict(message="Uploading file 2", complete=2), - dict(message="Uploading file 3", complete=7), - dict(message="Uploads complete", complete=10), - ] - self.check_messages(listener.messages, expected_messages) + self.assertTaskEventuallyCompletes(future, None) + self.assertEqual( + listener.states, + [ + (10, 0, "Uploading files..."), + (10, 0, "Uploading file 1"), + (10, 2, "Uploading file 2"), + (10, 7, "Uploading file 3"), + (10, 10, "Uploads complete"), + ], + ) def test_no_stop(self): - # If the user doesn't call stop, we should still get a final update - # that sets the 'step'. + # If the user doesn't call stop, we don't get a final update, but + # nothing else should go wrong. def send_messages(reporter): - reporter.start(total=2) reporter.step("Uploading file 1") reporter.step("Uploading file 2") - future = submit_steps(self.executor, None, None, send_messages) + future = submit_steps( + self.executor, 2, "Uploading files", send_messages + ) listener = StepsListener(future=future) - self.wait_for_result(future) - - expected_messages = [ - # Initial values - dict(message=None, total=None, complete=0), - # Updates on setting 'total' to 2. - dict(total=2), - # Updates on start of first step - dict(message="Uploading file 1"), - # Updates on start of second step - dict(message="Uploading file 2", complete=1), - # Updates on completion. - dict(complete=2), - ] - - self.check_messages(listener.messages, expected_messages) - - # XXX test cancellation on start, and cancellation on stop + self.assertTaskEventuallyCompletes(future, None) + self.assertEqual( + listener.states, + [ + (2, 0, "Uploading files"), + (2, 0, "Uploading file 1"), + (2, 1, "Uploading file 2"), + ], + ) def test_cancellation_on_step(self): - checkpoint = self._context.event() barrier = self._context.event() detector = self._context.event() - def send_messages(checkpoint, barrier, reporter, detector): - reporter.start(total=2) + def send_messages(barrier, reporter, detector): reporter.step("Uploading file 1") - checkpoint.set() # Test will cancel at this point. barrier.wait(timeout=SAFETY_TIMEOUT) - # Call to 'step' should abandon execution. reporter.step("Uploading file 2") + # Should never get here. detector.set() + reporter.stop("All files uploaded") future = submit_steps( self.executor, - None, - None, + 2, + "Uploading files", send_messages, - checkpoint=checkpoint, barrier=barrier, detector=detector, ) listener = StepsListener(future=future) - checkpoint.wait(timeout=SAFETY_TIMEOUT) + # Run until we get the first progress message, then cancel and allow + # the background job to proceed. + self.run_until( + listener, + "state", + lambda listener: listener.state[2] == "Uploading file 1", + ) future.cancel() barrier.set() - self.wait_for_cancelled(future) + self.assertTaskEventuallyCancelled(future) + self.assertFalse(detector.is_set()) + + self.assertEqual( + listener.states, + [ + (2, 0, "Uploading files"), + (2, 0, "Uploading file 1"), + ], + ) + def test_cancellation_on_stop(self): + barrier = self._context.event() + detector = self._context.event() + + def send_messages(barrier, reporter, detector): + reporter.step("Uploading file 1") + reporter.step("Uploading file 2") + # Test will cancel at this point. + barrier.wait(timeout=SAFETY_TIMEOUT) + reporter.stop("All files uploaded"), + # Should never get here. + detector.set() + + future = submit_steps( + self.executor, + 2, + "Uploading files", + send_messages, + barrier=barrier, + detector=detector, + ) + listener = StepsListener(future=future) + + # Run until we get the first progress message, then cancel and allow + # the background job to proceed. + self.run_until( + listener, + "state", + lambda listener: listener.state[2] == "Uploading file 2", + ) + future.cancel() + barrier.set() + + self.assertTaskEventuallyCancelled(future) self.assertFalse(detector.is_set()) - expected_messages = [ - # Initial values - dict(message=None, total=None, complete=0), - ] - self.check_messages(listener.messages, expected_messages) + self.assertEqual( + listener.states, + [ + (2, 0, "Uploading files"), + (2, 0, "Uploading file 1"), + (2, 1, "Uploading file 2"), + ], + ) def test_initial_total(self): # Exercise the case where we set the total and message up front. def send_messages(reporter): reporter.step("Uploading file 1") reporter.step("Uploading file 2") + reporter.stop("All uploaded") future = submit_steps( self.executor, 2, "Uploading files", send_messages ) listener = StepsListener(future=future) - self.wait_for_result(future) - - expected_messages = [ - # Initial values - dict(message="Uploading files", total=2, complete=0), - # Updates on start of first step - dict(message="Uploading file 1"), - # Updates on start of second step - dict(message="Uploading file 2", complete=1), - # Updates on completion. - dict(complete=2), - ] - - self.check_messages(listener.messages, expected_messages) - - def check_messages(self, actual_messages, expected_messages): - - actual_messages = actual_messages.copy() - - # Expected messages should match actual messages, in chunks - for message_set in expected_messages: - message_count = len(message_set) - self.assertCountEqual( - actual_messages[:message_count], - list(message_set.items()), - ) - actual_messages = actual_messages[message_count:] + self.assertTaskEventuallyCompletes(future, None) + self.assertEqual( + listener.states, + [ + (2, 0, "Uploading files"), + (2, 0, "Uploading file 1"), + (2, 1, "Uploading file 2"), + (2, 2, "All uploaded"), + ], + ) - # Check we got everything. - self.assertFalse(actual_messages) + def test_reporter_in_kwargs(self): + def some_callable(reporter): + pass + + with self.assertRaises(TypeError): + submit_steps( + self.executor, + 2, + "Uploading files", + some_callable, + reporter=None, + ) # Helper functions @@ -315,25 +316,44 @@ def halt_executor(self): self.run_until(executor, "stopped", lambda executor: executor.stopped) del self.executor - def wait_for_result(self, future): + def assertTaskEventuallyCompletes(self, future, result): + """ + Wait for a task to finish, and check its return value. + + Parameters + ---------- + future : BaseFuture + The future to wait for. + result : object + Value that that task is expected to return. + """ self.run_until(future, "done", lambda future: future.done) - if future.state == COMPLETED: - return future.result - elif future.state == FAILED: - exc_type, exc_value, exc_traceback = future.exception - self.fail( - f"Task failed with exception of type: {exc_type}\n" - f"{exc_traceback}" - ) - elif future.state == CANCELLED: - self.fail("Task did not return a result because it was cancelled.") + self.assertEqual(future.state, COMPLETED) + self.assertEqual(future.result, result) - def wait_for_exception(self, future): + def assertTaskEventuallyFails(self, future, exception_type): + """ + Wait for a task to finish, and verify that it fails. + + Parameters + ---------- + future : BaseFuture + The future to wait for. + exception_type : type + Type of the exception that the task is expected to raise. + """ self.run_until(future, "done", lambda future: future.done) - exc_type, exc_value, exc_traceback = future.exception - return exc_type + self.assertEqual(future.state, FAILED) + self.assertIn(exception_type.__name__, future.exception[0]) - def wait_for_cancelled(self, future): + def assertTaskEventuallyCancelled(self, future): + """ + Wait for a task to finish, and check it reached CANCELLED state. + + Parameters + ---------- + future : BaseFuture + The future to wait for. + """ self.run_until(future, "done", lambda future: future.done) - if future.state != CANCELLED: - raise RuntimeError("Future was not cancelled") + self.assertEqual(future.state, CANCELLED) From 17b73623d8cc791880a18691c0c956f9121526ab Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Fri, 12 Nov 2021 11:04:32 +0000 Subject: [PATCH 28/28] A couple more todo comments --- docs/source/guide/examples/steps_dialog.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/source/guide/examples/steps_dialog.py b/docs/source/guide/examples/steps_dialog.py index 4762f166..42f3de8e 100644 --- a/docs/source/guide/examples/steps_dialog.py +++ b/docs/source/guide/examples/steps_dialog.py @@ -19,6 +19,13 @@ # XXX Fix behaviour on dialog close button. Should match pressing the # "cancelling" button, and also pressing ESC. (What do users want?) # XXX What should behaviour be on a Ctrl-C? +# XXX Two possible desirable behaviours: (1) cancel then close immediately +# (2) prevent closing, close when cancel completes +# (1) is more likely to be important with a modal dialog +# All three of (a) pressing the cancel button, (b) using the dialog close +# button, (c) hitting ESC should have the same behaviour. +# For the preventing closing behaviour, the dialog itself needs some state. + # XXX Diagnose and fix errors seen when launching a non-modal dialog and then # clicking its close button: