add on_start() and error_count prop to BatchRunner #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances the BatchRunner class by adding callback and error tracking capabilities. It introduces an on_start callback that receives the total item count when a batch begins (useful for initializing progress bars), and an error_count property that tracks errors with thread-safe access and automatic reset between runs. The implementation follows existing patterns in the codebase, with proper thread safety through lock usage and callbacks invoked outside locks to prevent deadlocks.
Key Changes:
- Added
on_startcallback parameter to receive total item count at batch initialization - Introduced thread-safe
error_countproperty with automatic reset on each run - Expanded documentation with usage examples for the new features
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/nbatch/_runner.py | Added on_start callback parameter, error_count property with thread-safe access, and comprehensive documentation including usage examples |
| tests/test_runner.py | Added tests for on_start callback invocation, error_count tracking, and reset behavior across multiple runs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/nbatch/_runner.py
Outdated
| ---------- | ||
| on_start : Callable[[int], None] | None, optional | ||
| Called when batch starts, receives total item count. Use to initialize | ||
| progress bars (e.g., ``on_start=lambda total: progress_bar.setMax(total)``). |
There was a problem hiding this comment.
Inconsistent method name in documentation. Line 47 uses setMax(total) but line 74 uses the correct setMaximum(total). Qt progress bars use setMaximum, not setMax. The example on line 47 should be updated to match line 74.
| progress bars (e.g., ``on_start=lambda total: progress_bar.setMax(total)``). | |
| progress bars (e.g., ``on_start=lambda total: progress_bar.setMaximum(total)``). |
| def test_on_start_callback(self): | ||
| """Test on_start callback is called with total count.""" | ||
| on_start = MagicMock() | ||
|
|
||
| def process(item): | ||
| return item | ||
|
|
||
| runner = BatchRunner(on_start=on_start) | ||
| runner.run(process, [1, 2, 3, 4, 5], threaded=False) | ||
|
|
||
| on_start.assert_called_once_with(5) |
There was a problem hiding this comment.
The on_start callback is only tested in synchronous mode (threaded=False). For consistency with other callback tests (e.g., on_item_complete, on_complete, on_error in TestBatchRunnerNapariThreading), consider adding a test for on_start in threaded mode to ensure it works correctly in both execution paths.
| def test_error_count_tracking(self): | ||
| """Test error_count tracks errors during batch.""" | ||
|
|
||
| def process(item): | ||
| if item in [2, 4]: | ||
| raise ValueError(f'Error on {item}') | ||
| return item | ||
|
|
||
| runner = BatchRunner() | ||
| runner.run(process, [1, 2, 3, 4, 5], threaded=False) | ||
|
|
||
| assert runner.error_count == 2 |
There was a problem hiding this comment.
The error_count property is only tested in synchronous mode (threaded=False). For consistency with other error handling tests (e.g., test_run_napari_threaded_with_errors in TestBatchRunnerNapariThreading), consider adding a test for error_count in threaded mode to ensure the thread-safe counter works correctly across concurrent execution paths.
1 participant
This pull request adds new features and improvements to the
BatchRunnerclass and its usage in widgets, focusing on better progress tracking and error reporting. The main enhancements include support for anon_startcallback to initialize progress bars, tracking the number of errors encountered during batch runs, and improved documentation and tests for these features.Feature additions and API changes:
on_startcallback parameter toBatchRunner, which is called at the start of a batch with the total item count. This allows widgets to initialize progress bars more reliably. (src/nbatch/_runner.py,README.md) [1] [2] [3] [4]error_countproperty toBatchRunnerto track the number of errors in the current or last batch, and reset it at the start of each run. (src/nbatch/_runner.py,README.md) [1] [2] [3] [4] [5] [6]Widget integration improvements:
MyWidgetexample to use the newon_startcallback for progress bar initialization and to display error counts after batch completion. (README.md) [1] [2] [3]Documentation and usability:
src/nbatch/_runner.py,README.md) [1] [2]Testing improvements:
on_startcallback anderror_countproperty, including tests for error tracking and reset behavior across runs. (tests/test_runner.py)These changes make batch processing more robust and user-friendly, especially in GUI contexts where progress and error reporting are important.