chore(api): Fix more mypy type errors (#2399)

* chore(api): Require mypy pass for lint pass and fix type errors

api/Makefile

@@ -39,7 +39,7 @@ test:
 
 .PHONY: lint
 lint:
-	-$(python) -m mypy opentrons
+	$(python) -m mypy opentrons
 	$(python) -m pylama opentrons tests
 
 .PHONY: docs

api/opentrons/broker/__init__.py

@@ -1,4 +1,4 @@
 from .broker import subscribe, publish, Notifications
 from . import topics
 
-__all__ = [subscribe, publish, Notifications, topics]
+__all__ = ['subscribe', 'publish', 'Notifications', 'topics']

api/opentrons/commands/commands.py

@@ -325,7 +325,7 @@ def resume():
     )
 
 
-def publish(before, after, command, meta=None):
+def _do_publish(before, after, command, meta=None):
     publish_command = functools.partial(
         broker.publish,
         topic=types.COMMAND)
@@ -337,24 +337,26 @@ def decorated(*args, **kwargs):
             command_args = dict(
                 zip(
                     reversed(inspect.getfullargspec(command).args),
-                    reversed(inspect.getfullargspec(command).defaults or [])))
+                    reversed(inspect.getfullargspec(command).defaults
+                             or [])))
 
             # TODO (artyom, 20170927): we are doing this to be able to use
             # the decorator in Instrument class methods, in which case
             # self is effectively an instrument.
-            # To narrow the scope of this hack, we are checking if the command
-            # is expecting instrument first.
+            # To narrow the scope of this hack, we are checking if the
+            # command is expecting instrument first.
             if 'instrument' in inspect.getfullargspec(command).args:
                 # We are also checking if call arguments have 'self' and
-                # don't have instruments specified, in which case instruments
-                # should take precedence.
+                # don't have instruments specified, in which case
+                # instruments should take precedence.
                 if 'self' in call_args and 'instrument' not in call_args:
                     call_args['instrument'] = call_args['self']
 
             command_args.update({
                 key: call_args[key]
                 for key in
-                set(inspect.getfullargspec(command).args) & call_args.keys()
+                (set(inspect.getfullargspec(command).args)
+                 & call_args.keys())
             })
 
             if meta:
@@ -378,6 +380,21 @@ def decorated(*args, **kwargs):
     return decorator
 
 
+class publish:
+    """ Class that allows namespaced decorators with valid mypy types
+
+    These were previously defined by adding them as attributes to the
+    publish function, which is not currently supported by mypy:
+    https://github.com/python/mypy/issues/2087
+
+    Making a simple class with these as attributes does the same thing
+    but in a way that mypy can actually verify.
+    """
+    before = functools.partial(_do_publish, before=True, after=False)
+    after = functools.partial(_do_publish, before=False, after=True)
+    both = functools.partial(_do_publish, before=True, after=True)
+
+
 def _get_args(f, args, kwargs):
     # Create the initial dictionary with args that have defaults
     res = {}
@@ -394,8 +411,3 @@ def _get_args(f, args, kwargs):
     # Update it with values for named args
     res.update(kwargs)
     return res
-
-
-publish.before = functools.partial(publish, before=True, after=False)
-publish.after = functools.partial(publish, before=False, after=True)
-publish.both = functools.partial(publish, before=True, after=True)

api/opentrons/config/advanced_settings.py

@@ -65,15 +65,15 @@ def __repr__(self):
 def get_adv_setting(_id: str) -> bool:
     _id = _clean_id(_id)
     s = get_all_adv_settings()
-    return s.get(_id).get('value')
+    return s[_id].get('value')
 
 
 def get_all_adv_settings() -> dict:
     """
     :return: a dict of settings keyed by setting ID, where each value is a
         dict with keys "id", "title", "description", and "value"
     """
-    settings_file = get_config_index().get('featureFlagFile')
+    settings_file = get_config_index()['featureFlagFile']
 
     values = _read_settings_file(settings_file)
     for key, value in values.items():
@@ -86,7 +86,7 @@ def get_all_adv_settings() -> dict:
 
 def set_adv_setting(_id: str, value):
     _id = _clean_id(_id)
-    settings_file = get_config_index().get('featureFlagFile')
+    settings_file = get_config_index()['featureFlagFile']
     s = _read_settings_file(settings_file)
     s[_id] = value
     _write_settings_file(s, settings_file)
@@ -128,7 +128,7 @@ def _read_settings_file(settings_file: str) -> dict:
     if any([k in old_keys for k in data.keys()]):
         for v in data.keys():
             if v in old_keys:
-                new_key = settings_by_old_id.get(v).id
+                new_key = settings_by_old_id[v].id
                 data[new_key] = data[v]
                 data.pop(v)
         _write_settings_file(data, settings_file)

api/opentrons/data_storage/database.py

@@ -3,7 +3,7 @@
 # import warnings
 from typing import List
 from opentrons.legacy_api.containers.placeable\
-    import Container, Well, Module
+    import Container, Well, Module, Placeable
 from opentrons.data_storage import database_queries as db_queries
 from opentrons.util import environment
 from opentrons.util.vector import Vector
@@ -72,7 +72,7 @@ def _load_container_object_from_db(db, container_name: str):
         )
 
     if container_name in SUPPORTED_MODULES:
-        container = Module()
+        container: Placeable = Module()
     else:
         container = Container()
 
@@ -214,10 +214,12 @@ def overwrite_container(container: Container) -> bool:
 
 def save_labware_offset(labware: Container, labware_name: str=None) -> bool:
     if labware_name is None:
-        labware_name = labware.get_name()
+        name = labware.get_name()
+    else:
+        name = labware_name
     offset = _calculate_offset(labware)
-    log.debug("Saving offset {} for {}".format(offset, labware_name))
-    return ldef.save_labware_offset(labware_name, offset)
+    log.debug("Saving offset {} for {}".format(offset, name))
+    return ldef.save_labware_offset(name, offset)
 
 
 def delete_container(container_name) -> bool:

api/opentrons/data_storage/old_container_loading.py

@@ -11,12 +11,13 @@
 import os
 import pkg_resources
 from collections import OrderedDict
+from typing import Dict, Any, List
 from opentrons.legacy_api.containers.placeable import Container, Well
 from opentrons.util import environment
 from opentrons.util.vector import Vector
 
-persisted_containers_dict = {}
-containers_file_list = []
+persisted_containers_dict: Dict[str, Any] = {}
+containers_file_list: List[str] = []
 
 containers_dir_path = pkg_resources.resource_filename(
     'opentrons.config',
@@ -174,7 +175,7 @@ def create_container_obj_from_dict(container_data: dict) -> Container:
     origin_offset_z = container_data.get('origin-offset', {}).get('z') or 0
 
     container = Container()
-    locations = container_data.get('locations')
+    locations = container_data['locations']
     container._coordinates = Vector(
         origin_offset_x,
         origin_offset_y,

api/opentrons/data_storage/serializers.py

@@ -33,7 +33,7 @@ def json_to_labware(json_defn: dict) -> Container:
     container = Container()
     container._coordinates = Vector(0, 0, 0)
 
-    wells = container_data.get('wells')
+    wells = container_data['wells']
     for well_name, json_well in wells.items():
         well, coordinates = _json_to_well(json_well)
         container.add(well, well_name, coordinates)

api/opentrons/drivers/__init__.py

@@ -9,9 +9,6 @@
 
 from opentrons.drivers import connection
 
-__all__ = [
-]
-
 VIRTUAL_SMOOTHIE_PORT = 'Virtual Smoothie'
 
 SMOOTHIE_DEFAULTS_DIR = pkg_resources.resource_filename(

api/opentrons/drivers/mag_deck/__init__.py

@@ -1,5 +1,5 @@
 from opentrons.drivers.mag_deck.driver import MagDeck
 
 __all__ = [
-    MagDeck
+    'MagDeck'
 ]

api/opentrons/drivers/mag_deck/driver.py

@@ -142,7 +142,8 @@ def _parse_distance_response(distance_string) -> float:
         raise ParseError(err_msg)
     if 'Z' not in distance_string and 'height' not in distance_string:
         raise ParseError(err_msg)
-    return _parse_number_from_substring(distance_string.strip())
+    return _parse_number_from_substring(  # type: ignore
+        distance_string.strip())          # (preconditions checked above)
 
 
 class MagDeck:

api/opentrons/drivers/temp_deck/__init__.py

@@ -1,5 +1,5 @@
 from opentrons.drivers.temp_deck.driver import TempDeck
 
 __all__ = [
-    TempDeck
+    'TempDeck'
 ]

api/opentrons/hardware_control/__init__.py

@@ -14,14 +14,14 @@
 import functools
 import logging
 import enum
-from typing import Dict
+from typing import Dict, Union
 from opentrons import types
-from . import simulator
+from .simulator import Simulator
 try:
-    from . import controller
+    from .controller import Controller
 except ModuleNotFoundError:
     # implies windows
-    controller = None  # type: ignore
+    Controller = None  # type: ignore
 
 
 mod_log = logging.getLogger(__name__)
@@ -51,6 +51,9 @@ class MustHomeError(RuntimeError):
     pass
 
 
+_Backend = Union[Controller, Simulator]
+
+
 class API:
     """ This API is the primary interface to the hardware controller.
 
@@ -65,7 +68,7 @@ class API:
     CLS_LOG = mod_log.getChild('API')
 
     def __init__(self,
-                 backend: object,
+                 backend: _Backend,
                  config: dict = None,
                  loop: asyncio.AbstractEventLoop = None) -> None:
         """ Initialize an API instance.
@@ -81,7 +84,7 @@ def __init__(self,
         else:
             self._loop = loop
         # {'X': 0.0, 'Y': 0.0, 'Z': 0.0, 'A': 0.0, 'B': 0.0, 'C': 0.0}
-        self._current_position: dict = None
+        self._current_position: Dict[str, float] = {}
 
         self._attached_instruments = {types.Mount.LEFT: None,
                                       types.Mount.RIGHT: None}
@@ -96,10 +99,10 @@ def build_hardware_controller(
         real robot only one true hardware controller may be active at one
         time.
         """
-        if None is controller:
+        if None is Controller:
             raise RuntimeError(
                 'The hardware controller may only be instantiated on a robot')
-        return cls(controller.Controller(config, loop),
+        return cls(Controller(config, loop),
                    config=config, loop=loop)
 
     @classmethod
@@ -113,7 +116,7 @@ def build_hardware_simulator(
         This method may be used both on a real robot and on dev machines.
         Multiple simulating hardware controllers may be active at one time.
         """
-        return cls(simulator.Simulator(attached_instruments, config, loop),
+        return cls(Simulator(attached_instruments, config, loop),
                    config=config, loop=loop)
 
     # Query API
@@ -180,7 +183,7 @@ async def home_z(self):
 
     @_log_call
     async def move_to(
-            self, mount: types.Mount, abs_position: types.Point = None):
+            self, mount: types.Mount, abs_position: types.Point):
         if not self._current_position:
             raise MustHomeError
         z_axis = _Axis.by_mount(mount)
@@ -193,7 +196,7 @@ async def move_to(
         await self._move(target_position)
 
     @_log_call
-    async def move_rel(self, mount: types.Mount, delta: types.Point = None):
+    async def move_rel(self, mount: types.Mount, delta: types.Point):
         if not self._current_position:
             raise MustHomeError
         z_axis = _Axis.by_mount(mount)

api/opentrons/helpers/__init__.py

@@ -1,3 +1,3 @@
 from .helpers import flip_coordinates
 
-__all__ = [flip_coordinates]
+__all__ = ['flip_coordinates']

api/opentrons/legacy_api/api.py

@@ -1,7 +1,8 @@
 from . import robot, instruments as inst, containers as cnt
 from .instruments import pipette_config
 
-robot = robot.Robot()  # typing: ignore This is why we’re moving away from it
+# Ignore the type here because well, this is exactly why this is the legacy_api
+robot = robot.Robot()  # type: ignore
 
 
 def reset():

api/opentrons/protocol_api/__init__.py

@@ -76,7 +76,7 @@ def load_labware_by_name(
         This function returns the created and initialized labware for use
         later in the protocol.
         """
-        labware = load(labware_name)
+        labware = load(labware_name, location)
         self.load_labware(labware, location)
         return labware
 

api/opentrons/server/endpoints/serverlib_fallback.py

@@ -7,6 +7,7 @@
 from time import sleep
 from aiohttp import web
 from threading import Thread
+from typing import Dict, Any
 
 log = logging.getLogger(__name__)
 ignore_file = 'ignore.json'
@@ -162,7 +163,7 @@ async def update_api(request: web.Request) -> web.Response:
             res2 = await install_smoothie_firmware(
                 data['fw'], request.loop)
             reslist.append(res2)
-        res = {
+        res: Dict[str, Any] = {
             'message': [r['message'] for r in reslist],
             'filename': [r['filename'] for r in reslist]
         }

api/tests/opentrons/hardware_control/test_instantiation.py

@@ -4,7 +4,7 @@
 import pytest
 from opentrons import hardware_control as hc
 
-if not hc.controller:
+if not hc.Controller:
     pytest.skip('hardware controller not available (probably windows)',
                 allow_module_level=True)
 

api/tests/opentrons/hardware_control/test_instruments.py

@@ -12,7 +12,7 @@ async def test_cache_instruments(loop):
     assert hw_api._attached_instruments == dummy_instruments_attached
 
 
-@pytest.mark.skipif(not hc.controller,
+@pytest.mark.skipif(not hc.Controller,
                     reason='hardware controller not available '
                            '(probably windows)')
 async def test_cache_instruments_hc(monkeypatch, hardware_controller_lockfile,