pr feedback

api/src/opentrons/protocol_runner/__init__.py

@@ -1,24 +1,24 @@
 """Protocol run control and management.
 
-The main export of this module is the ProtocolRunner class. See
+The main export of this module is the AbstractRunner class. See
 protocol_runner.py for more details.
 """
 from .protocol_runner import (
- ProtocolRunner,
+ AbstractRunner,
  ProtocolRunResult,
  create_protocol_runner,
  JsonRunner,
  PythonAndLegacyRunner,
- MaintenanceRunner,
+ LiveRunner,
 )
 from .create_simulating_runner import create_simulating_runner
 
 __all__ = [
- "ProtocolRunner",
+ "AbstractRunner",
  "ProtocolRunResult",
  "create_simulating_runner",
  "create_protocol_runner",
  "JsonRunner",
  "PythonAndLegacyRunner",
- "MaintenanceRunner",
+ "LiveRunner",
 ]

api/src/opentrons/protocol_runner/create_simulating_runner.py

@@ -1,4 +1,4 @@
-"""Simulating ProtocolRunner factory."""
+"""Simulating AbstractRunner factory."""
 
 from opentrons.config import feature_flags
 from opentrons.hardware_control import API as OT2API, HardwareControlAPI
@@ -11,13 +11,13 @@
 from opentrons_shared_data.robot.dev_types import RobotType
 
 from .legacy_wrappers import LegacySimulatingContextCreator
-from .protocol_runner import create_protocol_runner, ProtocolRunner
+from .protocol_runner import create_protocol_runner, AbstractRunner
 
 
 async def create_simulating_runner(
  robot_type: RobotType, protocol_config: ProtocolConfig
-) -> ProtocolRunner:
- """Create a ProtocolRunner wired to a simulating HardwareControlAPI.
+) -> AbstractRunner:
+ """Create a AbstractRunner wired to a simulating HardwareControlAPI.
 
  Example:
  ```python
@@ -27,15 +27,15 @@ async def create_simulating_runner(
  from opentrons.protocol_runner import (
  ProtocolType,
  ProtocolFile,
- ProtocolRunner,
+ AbstractRunner,
  create_simulating_runner,
  )
 
  protocol = ProtocolFile(
  protocol_type=ProtocolType.PYTHON,
  files=[Path("/path/to/protocol.py")],
  )
- runner: ProtocolRunner = await create_simulating_runner()
+ runner: AbstractRunner = await create_simulating_runner()
  commands: List[Command] = await runner.run(protocol)
  ```
  """

api/src/opentrons/protocol_runner/protocol_runner.py

@@ -2,7 +2,7 @@
 import asyncio
 from typing import List, NamedTuple, Optional, Union
 
-from typing_extensions import Protocol as TypingProtocol
+from abc import ABC, abstractmethod
 
 import anyio
 
@@ -38,48 +38,54 @@ class ProtocolRunResult(NamedTuple):
  state_summary: StateSummary
 
 
-class ProtocolRunner(TypingProtocol):
+class AbstractRunner(ABC):
  """An interface to manage and control a protocol run.
 
- The ProtocolRunner is primarily responsible for feeding a ProtocolEngine
+ The AbstractRunner is primarily responsible for feeding a ProtocolEngine
  with commands and control signals. These commands and signals are
  generated by protocol files, hardware signals, or externally via
  the HTTP robot-server.
 
- A ProtocolRunner controls a single run. Once the run is finished,
- you will need a new ProtocolRunner to do another run.
+ A AbstractRunner controls a single run. Once the run is finished,
+ you will need a new AbstractRunner to do another run.
  """
 
+ @abstractmethod
  def was_started(self) -> bool:
  """Whether the runner has been started.
 
  This value is latched; once it is True, it will never become False.
  """
 
+ @abstractmethod
  async def load(self, protocol_source: ProtocolSource) -> None:
  """Load a ProtocolSource into managed ProtocolEngine.
 
  Calling this method is only necessary if the runner will be used
  to control the run of a file-based protocol.
  """
 
+ @abstractmethod
  def play(self) -> None:
  """Start or resume the run."""
 
+ @abstractmethod
  def pause(self) -> None:
  """Pause the run."""
 
+ @abstractmethod
  async def stop(self) -> None:
  """Stop (cancel) the run."""
 
+ @abstractmethod
  async def run(
  self,
  protocol_source: Optional[ProtocolSource] = None,
  ) -> ProtocolRunResult:
  """Run a given protocol to completion."""
 
 
-class PythonAndLegacyRunner(ProtocolRunner):
+class PythonAndLegacyRunner(AbstractRunner):
  """Protocol runner implementation for Python protocols, and JSON protocols ≤v5."""
 
  def __init__(
@@ -105,18 +111,9 @@ def __init__(
  self._task_queue = task_queue or TaskQueue(cleanup_func=protocol_engine.finish)
 
  def was_started(self) -> bool:
- """Whether the runner has been started.
-
- This value is latched; once it is True, it will never become False.
- """
  return self._protocol_engine.state_view.commands.has_been_played()
 
  async def load(self, protocol_source: ProtocolSource) -> None:
- """Load a ProtocolSource into managed ProtocolEngine.
-
- Calling this method is only necessary if the runner will be used
- to control the run of a file-based protocol.
- """
  labware_definitions = await protocol_reader.extract_labware_definitions(
  protocol_source=protocol_source
  )
@@ -152,15 +149,12 @@ async def load(self, protocol_source: ProtocolSource) -> None:
  )
 
  def play(self) -> None:
- """Start or resume the run."""
  self._protocol_engine.play()
 
  def pause(self) -> None:
- """Pause the run."""
  self._protocol_engine.pause()
 
  async def stop(self) -> None:
- """Stop (cancel) the run."""
  if self.was_started():
  await self._protocol_engine.stop()
  else:
@@ -173,11 +167,10 @@ async def run(
  self,
  protocol_source: Optional[ProtocolSource] = None,
  ) -> ProtocolRunResult:
- """Run a given protocol to completion."""
  # TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
  # currently `protocol_source` arg is only used by tests
  if protocol_source:
- await self.load(protocol_source)
+ await self.load(protocol_source=protocol_source)
 
  await self._hardware_api.home()
  self.play()
@@ -189,7 +182,7 @@ async def run(
  return ProtocolRunResult(commands=commands, state_summary=run_data)
 
 
-class JsonRunner(ProtocolRunner):
+class JsonRunner(AbstractRunner):
  """Protocol runner implementation for json protocols."""
 
  def __init__(
@@ -210,18 +203,9 @@ def __init__(
  self._task_queue = task_queue or TaskQueue(cleanup_func=protocol_engine.finish)
 
  def was_started(self) -> bool:
- """Whether the runner has been started.
-
- This value is latched; once it is True, it will never become False.
- """
  return self._protocol_engine.state_view.commands.has_been_played()
 
  async def load(self, protocol_source: ProtocolSource) -> None:
- """Load a ProtocolSource into managed ProtocolEngine.
-
- Calling this method is only necessary if the runner will be used
- to control the run of a file-based protocol.
- """
  labware_definitions = await protocol_reader.extract_labware_definitions(
  protocol_source=protocol_source
  )
@@ -267,15 +251,12 @@ async def load(self, protocol_source: ProtocolSource) -> None:
  self._task_queue.set_run_func(func=self._protocol_engine.wait_until_complete)
 
  def play(self) -> None:
- """Start or resume the run."""
  self._protocol_engine.play()
 
  def pause(self) -> None:
- """Pause the run."""
  self._protocol_engine.pause()
 
  async def stop(self) -> None:
- """Stop (cancel) the run."""
  if self.was_started():
  await self._protocol_engine.stop()
  else:
@@ -288,7 +269,6 @@ async def run(
  self,
  protocol_source: Optional[ProtocolSource] = None,
  ) -> ProtocolRunResult:
- """Run a given protocol to completion."""
  # TODO(mc, 2022-01-11): move load to runner creation, remove from `run`
  # currently `protocol_source` arg is only used by tests
  if protocol_source:
@@ -304,51 +284,35 @@ async def run(
  return ProtocolRunResult(commands=commands, state_summary=run_data)
 
 
-class MaintenanceRunner(ProtocolRunner):
+class LiveRunner(AbstractRunner):
  """Protocol runner implementation for setup commands."""
 
  def __init__(
  self,
  protocol_engine: ProtocolEngine,
  hardware_api: HardwareControlAPI,
  ) -> None:
- """Initialize the MaintenanceRunner with its dependencies."""
+ """Initialize the LiveRunner with its dependencies."""
  self._protocol_engine = protocol_engine
  # TODO(mc, 2022-01-11): replace task queue with specific implementations
  # of runner interface
  self._hardware_api = hardware_api
 
  def was_started(self) -> bool:
- """Whether the runner has been started.
-
- This value is latched; once it is True, it will never become False.
- """
  return self._protocol_engine.state_view.commands.has_been_played()
 
  async def load(self, protocol_source: ProtocolSource) -> None:
- """Load a ProtocolSource into managed ProtocolEngine.
-
- Calling this method is only necessary if the runner will be used
- to control the run of a file-based protocol.
- """
  raise NotImplementedError(
- "MaintenanceRunner.load() not supported for setup commands. no protocol associated."
+ "LiveRunner.load() not supported for setup commands. no protocol associated."
  )
 
  def play(self) -> None:
- """Start or resume the run."""
- raise NotImplementedError(
- "MaintenanceRunner.play() not supported for setup commands."
- )
+ self._protocol_engine.play()
 
  def pause(self) -> None:
- """Pause the run."""
- raise NotImplementedError(
- "MaintenanceRunner.pause() not supported for setup commands."
- )
+ self._protocol_engine.pause()
 
  async def stop(self) -> None:
- """Stop (cancel) the run."""
  if self.was_started():
  await self._protocol_engine.stop()
  else:
@@ -361,7 +325,6 @@ async def run(
  self,
  protocol_source: Optional[ProtocolSource] = None,
  ) -> ProtocolRunResult:
- """Run a given protocol to completion."""
  if protocol_source:
  self.load(protocol_source)
 
@@ -382,7 +345,7 @@ def create_protocol_runner(
  legacy_file_reader: Optional[LegacyFileReader] = None,
  legacy_context_creator: Optional[LegacyContextCreator] = None,
  legacy_executor: Optional[LegacyExecutor] = None,
-) -> ProtocolRunner:
+) -> AbstractRunner:
  """Create a protocol runner."""
  if protocol_config:
  if (
@@ -406,7 +369,7 @@ def create_protocol_runner(
  legacy_executor=legacy_executor,
  )
 
- return MaintenanceRunner(
+ return LiveRunner(
  protocol_engine=protocol_engine,
  hardware_api=hardware_api,
  )

api/tests/opentrons/protocol_runner/smoke_tests/test_protocol_runner.py

@@ -1,6 +1,6 @@
-"""Smoke tests for the ProtocolRunner and ProtocolEngine classes.
+"""Smoke tests for the AbstractRunner and ProtocolEngine classes.
 
-These tests construct a ProtocolRunner with a real ProtocolEngine
+These tests construct a AbstractRunner with a real ProtocolEngine
 hooked to a simulating HardwareAPI.
 
 Minimal, but valid and complete, protocol files are then loaded from
@@ -43,7 +43,7 @@ async def test_runner_with_python(
  python_protocol_file: Path,
  tempdeck_v1_def: ModuleDefinition,
 ) -> None:
- """It should run a Python protocol on the ProtocolRunner."""
+ """It should run a Python protocol on the AbstractRunner."""
  protocol_reader = ProtocolReader()
  protocol_source = await protocol_reader.read_saved(
  files=[python_protocol_file],
@@ -112,7 +112,7 @@ async def test_runner_with_python(
 
 
 async def test_runner_with_json(json_protocol_file: Path) -> None:
- """It should run a JSON protocol on the ProtocolRunner."""
+ """It should run a JSON protocol on the AbstractRunner."""
  protocol_reader = ProtocolReader()
  protocol_source = await protocol_reader.read_saved(
  files=[json_protocol_file],
@@ -173,7 +173,7 @@ async def test_runner_with_json(json_protocol_file: Path) -> None:
 
 
 async def test_runner_with_legacy_python(legacy_python_protocol_file: Path) -> None:
- """It should run a Python protocol on the ProtocolRunner."""
+ """It should run a Python protocol on the AbstractRunner."""
  protocol_reader = ProtocolReader()
  protocol_source = await protocol_reader.read_saved(
  files=[legacy_python_protocol_file],
@@ -233,7 +233,7 @@ async def test_runner_with_legacy_python(legacy_python_protocol_file: Path) -> N
 
 
 async def test_runner_with_legacy_json(legacy_json_protocol_file: Path) -> None:
- """It should run a Python protocol on the ProtocolRunner."""
+ """It should run a Python protocol on the AbstractRunner."""
  protocol_reader = ProtocolReader()
  protocol_source = await protocol_reader.read_saved(
  files=[legacy_json_protocol_file],

api/tests/opentrons/protocol_runner/test_legacy_command_mapper.py

@@ -1,4 +1,4 @@
-"""Tests for the ProtocolRunner's LegacyContextPlugin."""
+"""Tests for the AbstractRunner's LegacyContextPlugin."""
 import inspect
 from datetime import datetime
 from typing import cast

api/tests/opentrons/protocol_runner/test_legacy_context_plugin.py

@@ -1,4 +1,4 @@
-"""Tests for the ProtocolRunner's LegacyContextPlugin."""
+"""Tests for the AbstractRunner's LegacyContextPlugin."""
 import pytest
 from anyio import to_thread
 from decoy import Decoy, matchers