feat(app,api): allow rich version specification for python protocols

api/src/opentrons/api/session.py

@@ -9,7 +9,7 @@
 from opentrons.commands import tree, types as command_types
 from opentrons.commands.commands import is_new_loc, listify
 from opentrons.config import feature_flags as ff
-from opentrons.protocols.types import JsonProtocol, PythonProtocol
+from opentrons.protocols.types import JsonProtocol, PythonProtocol, APIVersion
 from opentrons.protocols.parse import parse
 from opentrons.types import Location, Point
 from opentrons.protocol_api import (ProtocolContext,
@@ -277,12 +277,12 @@ def on_command(message):
 
     def refresh(self):
         self._reset()
-        self.api_level = 2 if ff.use_protocol_api_v2() else 1
         # self.metadata is exposed via jrpc
         if isinstance(self._protocol, PythonProtocol):
+            self.api_level = self._protocol.api_level
             self.metadata = self._protocol.metadata
             if ff.use_protocol_api_v2()\
-               and self._protocol.api_level == '1'\
+               and self._protocol.api_level == APIVersion(1, 0)\
                and not ff.enable_back_compat():
                 raise RuntimeError(
                     'This protocol targets Protocol API V1, but the robot is '
@@ -294,6 +294,10 @@ def refresh(self):
 
             log.info(f"Protocol API version: {self._protocol.api_level}")
         else:
+            if ff.use_protocol_api_v2():
+                self.api_level = APIVersion(2, 0)
+            else:
+                self.api_level = APIVersion(1, 0)
             self.metadata = {}
             log.info(f"JSON protocol")
 

api/src/opentrons/execute.py

@@ -18,7 +18,7 @@
 from opentrons import commands
 from opentrons.config import feature_flags as ff
 from opentrons.protocols.parse import parse
-from opentrons.protocols.types import JsonProtocol
+from opentrons.protocols.types import JsonProtocol, APIVersion
 from opentrons.hardware_control import API
 
 _HWCONTROL: Optional[API] = None
@@ -179,7 +179,7 @@ def execute(protocol_file: TextIO,
     contents = protocol_file.read()
     protocol = parse(contents, protocol_file.name)
     if isinstance(protocol, JsonProtocol)\
-            or protocol.api_level == '2'\
+            or protocol.api_level >= APIVersion(2, 0)\
             or (ff.enable_back_compat() and ff.use_protocol_api_v2()):
         context = get_protocol_api(
             bundled_labware=getattr(protocol, 'bundled_labware', None),

api/src/opentrons/protocol_api/execute.py

@@ -9,7 +9,7 @@
 from . import execute_v3, legacy_wrapper
 
 from opentrons import config
-from opentrons.protocols.types import PythonProtocol, Protocol
+from opentrons.protocols.types import PythonProtocol, Protocol, APIVersion
 
 MODULE_LOG = logging.getLogger(__name__)
 
@@ -169,9 +169,9 @@ def run_protocol(protocol: Protocol,
         raise RuntimeError(
             'Will not automatically generate hardware controller')
     if isinstance(protocol, PythonProtocol):
-        if protocol.api_level == '2':
+        if protocol.api_level >= APIVersion(2, 0):
             _run_python(protocol, true_context)
-        elif protocol.api_level == '1':
+        elif protocol.api_level == APIVersion(1, 0):
             _run_python_legacy(protocol, true_context)
         else:
             raise RuntimeError(

api/src/opentrons/protocols/parse.py

@@ -6,16 +6,20 @@
 import itertools
 import json
 import pkgutil
+import re
 from io import BytesIO
 from zipfile import ZipFile
 from typing import Any, Dict, Union
 
 import jsonschema  # type: ignore
 
 from opentrons.config import feature_flags as ff
-from .types import Protocol, PythonProtocol, JsonProtocol, Metadata
+from .types import Protocol, PythonProtocol, JsonProtocol, Metadata, APIVersion
 from .bundle import extract_bundle
 
+# match e.g. "2.0" but not "hi", "2", "2.0.1"
+API_VERSION_RE = re.compile(r'^(\d+)\.(\d+)$')
+
 
 def _parse_json(
         protocol_contents: str, filename: str = None) -> JsonProtocol:
@@ -46,7 +50,7 @@ def _parse_python(
                        filename=ast_filename)
     metadata = extract_metadata(parsed)
     protocol = compile(parsed, filename=ast_filename, mode='exec')
-    version = infer_version(metadata, parsed)
+    version = get_version(metadata, parsed)
 
     result = PythonProtocol(
         text=protocol_contents,
@@ -78,7 +82,7 @@ def _parse_bundle(bundle: ZipFile, filename: str = None) -> PythonProtocol:  # n
         contents.bundled_data,
         contents.bundled_python)
 
-    if result.api_level != '2':
+    if result.api_level < APIVersion(2, 0):
         raise RuntimeError('Bundled protocols must use Protocol API V2, ' +
                            f'got {result.api_level}')
 
@@ -155,7 +159,7 @@ def extract_metadata(parsed: ast.Module) -> Metadata:
     return metadata
 
 
-def infer_version_from_imports(parsed: ast.Module) -> str:
+def infer_version_from_imports(parsed: ast.Module) -> APIVersion:
     # Imports in the form of `import opentrons.robot` will have an entry in
     # parsed.body[i].names[j].name in the form "opentrons.robot". Find those
     # imports and transform them to strip away the 'opentrons.' part.
@@ -182,12 +186,31 @@ def infer_version_from_imports(parsed: ast.Module) -> str:
     v1_markers = set(('robot', 'instruments', 'modules', 'containers'))
     v1evidence = v1_markers.intersection(opentrons_imports)
     if v1evidence:
-        return '1'
+        return APIVersion(1, 0)
     else:
-        return '2'
+        raise RuntimeError('Cannot infer API level')
+
 
+def version_from_metadata(metadata: Metadata) -> APIVersion:
+    """ Build an API version from metadata, if we can.
 
-def infer_version(metadata: Metadata, parsed: ast.Module) -> str:
+    If there is no apiLevel key, raise a KeyError.
+    If the apiLevel value is malformed, raise a ValueError.
+    """
+    if 'apiLevel' not in metadata:
+        raise KeyError('apiLevel')
+    requested_level = str(metadata['apiLevel'])
+    if requested_level == '1':
+        return APIVersion(1, 0)
+    matches = API_VERSION_RE.match(requested_level)
+    if not matches:
+        raise ValueError(
+            f'apiLevel {requested_level} is incorrectly formatted. It should '
+            'major.minor, where both major and minor are numbers.')
+    return APIVersion(major=int(matches.group(1)), minor=int(matches.group(2)))
+
+
+def get_version(metadata: Metadata, parsed: ast.Module) -> APIVersion:
     """
     Infer protocol API version based on a combination of metadata and imports.
 
@@ -203,10 +226,17 @@ def infer_version(metadata: Metadata, parsed: ast.Module) -> str:
     APIv2 protocol (note that 'labware' is not in this list, as there is a
     valid APIv2 import named 'labware').
     """
-    level = str(metadata.get('apiLevel'))
-    if level in ('1', '2'):
-        return level
-    return infer_version_from_imports(parsed)
+    try:
+        return version_from_metadata(metadata)
+    except KeyError:  # No apiLevel key, may be apiv1
+        pass
+    try:
+        return infer_version_from_imports(parsed)
+    except RuntimeError:
+        raise RuntimeError(
+            'If this is not an API v1 protocol, you must specify the target '
+            'api level in the apiLevel key of the metadata. For instance, '
+            'metadata={"apiLevel": "2.0"}')
 
 
 def _get_protocol_schema_version(protocol_json: Dict[Any, Any]) -> int:

api/src/opentrons/protocols/types.py

@@ -3,6 +3,14 @@
 Metadata = Dict[str, Union[str, int]]
 
 
+class APIVersion(NamedTuple):
+    major: int
+    minor: int
+
+    def __str__(self):
+        return f'{self.major}.{self.minor}'
+
+
 class JsonProtocol(NamedTuple):
     text: str
     filename: Optional[str]
@@ -15,7 +23,7 @@ class PythonProtocol(NamedTuple):
     filename: Optional[str]
     contents: Any  # This is the output of compile() which we can't type
     metadata: Metadata
-    api_level: str  # For now, should be '1' or '2'
+    api_level: APIVersion
     # these 'bundled_' attrs should only be included when the protocol is a zip
     bundled_labware: Optional[Dict[str, Dict[str, Any]]]
     bundled_data: Optional[Dict[str, bytes]]

api/src/opentrons/simulate.py

@@ -21,7 +21,7 @@
 from opentrons import protocol_api
 from opentrons.protocols import parse, bundle
 from opentrons.protocols.types import (
-    JsonProtocol, PythonProtocol, BundleContents)
+    JsonProtocol, PythonProtocol, BundleContents, APIVersion)
 from opentrons.protocol_api import execute
 from .util.entrypoint_util import labware_from_paths, datafiles_from_paths
 
@@ -249,7 +249,7 @@ def simulate(protocol_file: TextIO,
                            extra_data=extra_data)
 
     if isinstance(protocol, JsonProtocol)\
-            or protocol.api_level == '2'\
+            or protocol.api_level >= APIVersion(2, 0)\
             or (ff.enable_back_compat() and ff.use_protocol_api_v2()):
         context = get_protocol_api(protocol)
         scraper = CommandScraper(stack_logger, log_level, context.broker)

api/tests/opentrons/api/test_session.py

@@ -88,9 +88,22 @@ async def test_async_notifications(main_router):
     assert res == {'name': 'foo', 'payload': {'bar': 'baz'}}
 
 
-def test_load_protocol_with_error(session_manager):
+@pytest.mark.api2_only
+def test_load_protocol_with_error_v2(session_manager, hardware):
+    with pytest.raises(Exception) as e:
+        session = session_manager.create(
+            name='<blank>', contents='metadata={"apiLevel": "2.0"}; blah')
+        assert session is None
+
+    args, = e.value.args
+    assert args == "name 'blah' is not defined"
+
+
+@pytest.mark.api1_only
+def test_load_protocol_with_error_v1(session_manager, hardware):
     with pytest.raises(Exception) as e:
-        session = session_manager.create(name='<blank>', contents='blah')
+        session = session_manager.create(
+            name='<blank>', contents='metadata={"apiLevel": "1.0"}; blah')
         assert session is None
 
     args, = e.value.args
@@ -231,7 +244,8 @@ async def test_get_instruments_and_containers(labware_setup,
     instruments, containers, modules, interactions = \
         _accumulate([_get_labware(command) for command in commands])
 
-    session = session_manager.create(name='', contents='')
+    session = session_manager.create(
+        name='', contents='from opentrons import instruments')
     # We are calling dedupe directly for testing purposes.
     # Normally it is called from within a session
     session._instruments.extend(_dedupe(instruments))
@@ -357,7 +371,7 @@ async def test_session_create_error(main_router):
     with pytest.raises(SyntaxError):
         main_router.session_manager.create(
             name='<blank>',
-            contents='syntax error ;(')
+            contents='from opentrons import instruments; syntax error ;(')
 
     with pytest.raises(TimeoutError):
         # No state change is expected
@@ -366,27 +380,54 @@ async def test_session_create_error(main_router):
     with pytest.raises(ZeroDivisionError):
         main_router.session_manager.create(
             name='<blank>',
-            contents='1/0')
+            contents='from opentrons import instruments; 1/0')
 
     with pytest.raises(TimeoutError):
         # No state change is expected
         await main_router.wait_until(lambda _: True)
 
 
-async def test_session_metadata(main_router):
+@pytest.mark.api1_only
+async def test_session_metadata_v1(main_router):
     expected = {
         'hello': 'world',
         'what?': 'no'
     }
 
     prot = """
+from opentrons import instruments
 this = 0
 that = 1
 metadata = {
 'what?': 'no',
 'hello': 'world'
 }
 print('wat?')
+"""
+
+    session = main_router.session_manager.create(
+        name='<blank>',
+        contents=prot)
+    assert session.metadata == expected
+
+
+@pytest.mark.api2_only
+async def test_session_metadata_v2(main_router):
+    expected = {
+        'hello': 'world',
+        'what?': 'no',
+        'apiLevel': '2.0'
+    }
+
+    prot = """
+this = 0
+that = 1
+metadata = {
+'what?': 'no',
+'hello': 'world',
+'apiLevel': '2.0'
+}
+print('wat?')
 
 def run(ctx):
     print('hi there')

api/tests/opentrons/conftest.py

@@ -646,7 +646,8 @@ def _get_bundle_protocol_fixture(fixture_name):
             result['bundled_python'] = {}
 
             # NOTE: this is copy-pasted from the .py fixture file
-            result['metadata'] = {'author': 'MISTER FIXTURE'}
+            result['metadata'] = {'author': 'MISTER FIXTURE',
+                                  'apiLevel': '2.0'}
 
             # make binary zipfile
             binary_zipfile = io.BytesIO()

api/tests/opentrons/data/testosaur-gen2-v2.py

@@ -4,7 +4,8 @@
     'protocolName': 'Testosaur',
     'author': 'Opentrons <[email protected]>',
     'description': 'A variant on "Dinosaur" for testing',
-    'source': 'Opentrons Repository'
+    'source': 'Opentrons Repository',
+    'apiLevel': '2.0'
 }
 
 

api/tests/opentrons/data/testosaur_v2.py

@@ -4,7 +4,8 @@
     'protocolName': 'Testosaur',
     'author': 'Opentrons <[email protected]>',
     'description': 'A variant on "Dinosaur" for testing',
-    'source': 'Opentrons Repository'
+    'source': 'Opentrons Repository',
+    'apiLevel': '2.0',
 }