From 61dda3efe2f19d1fcad894ce035202c5c916f879 Mon Sep 17 00:00:00 2001 From: Seth Foster Date: Wed, 3 Oct 2018 10:02:26 -0400 Subject: [PATCH] chore(api): Fix more mypy type errors (#2399) * chore(api): Require mypy pass for lint pass and fix type errors --- api/Makefile | 2 +- api/opentrons/broker/__init__.py | 2 +- api/opentrons/commands/commands.py | 36 ++++++++++++------- api/opentrons/config/advanced_settings.py | 8 ++--- api/opentrons/data_storage/database.py | 12 ++++--- .../data_storage/old_container_loading.py | 7 ++-- api/opentrons/data_storage/serializers.py | 2 +- api/opentrons/drivers/__init__.py | 3 -- api/opentrons/drivers/mag_deck/__init__.py | 2 +- api/opentrons/drivers/mag_deck/driver.py | 3 +- api/opentrons/drivers/temp_deck/__init__.py | 2 +- api/opentrons/hardware_control/__init__.py | 25 +++++++------ api/opentrons/helpers/__init__.py | 2 +- api/opentrons/legacy_api/api.py | 3 +- api/opentrons/protocol_api/__init__.py | 2 +- .../server/endpoints/serverlib_fallback.py | 3 +- .../hardware_control/test_instantiation.py | 2 +- .../hardware_control/test_instruments.py | 2 +- 18 files changed, 68 insertions(+), 50 deletions(-) diff --git a/api/Makefile b/api/Makefile index 17a7a844de8..55ce6a6a3d8 100755 --- a/api/Makefile +++ b/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 diff --git a/api/opentrons/broker/__init__.py b/api/opentrons/broker/__init__.py index 6326a7f4763..9248f066364 100644 --- a/api/opentrons/broker/__init__.py +++ b/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'] diff --git a/api/opentrons/commands/commands.py b/api/opentrons/commands/commands.py index f2e8c6c80d3..c3e537775d4 100755 --- a/api/opentrons/commands/commands.py +++ b/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) diff --git a/api/opentrons/config/advanced_settings.py b/api/opentrons/config/advanced_settings.py index b50fbdfb33f..718acab56a1 100644 --- a/api/opentrons/config/advanced_settings.py +++ b/api/opentrons/config/advanced_settings.py @@ -65,7 +65,7 @@ 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: @@ -73,7 +73,7 @@ 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) diff --git a/api/opentrons/data_storage/database.py b/api/opentrons/data_storage/database.py index 45c3ba01da3..0f7719284f8 100644 --- a/api/opentrons/data_storage/database.py +++ b/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: diff --git a/api/opentrons/data_storage/old_container_loading.py b/api/opentrons/data_storage/old_container_loading.py index 9829496cbc7..07fdc7e6a84 100755 --- a/api/opentrons/data_storage/old_container_loading.py +++ b/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, diff --git a/api/opentrons/data_storage/serializers.py b/api/opentrons/data_storage/serializers.py index f8415b4f8ee..387b199fa7c 100755 --- a/api/opentrons/data_storage/serializers.py +++ b/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) diff --git a/api/opentrons/drivers/__init__.py b/api/opentrons/drivers/__init__.py index c12d062dafc..d4e8627f480 100644 --- a/api/opentrons/drivers/__init__.py +++ b/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( diff --git a/api/opentrons/drivers/mag_deck/__init__.py b/api/opentrons/drivers/mag_deck/__init__.py index c7755e34742..dbc7578f759 100644 --- a/api/opentrons/drivers/mag_deck/__init__.py +++ b/api/opentrons/drivers/mag_deck/__init__.py @@ -1,5 +1,5 @@ from opentrons.drivers.mag_deck.driver import MagDeck __all__ = [ - MagDeck + 'MagDeck' ] diff --git a/api/opentrons/drivers/mag_deck/driver.py b/api/opentrons/drivers/mag_deck/driver.py index 535bef7eb74..2990a426394 100644 --- a/api/opentrons/drivers/mag_deck/driver.py +++ b/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: diff --git a/api/opentrons/drivers/temp_deck/__init__.py b/api/opentrons/drivers/temp_deck/__init__.py index 58aafcb670f..bc6f8251975 100644 --- a/api/opentrons/drivers/temp_deck/__init__.py +++ b/api/opentrons/drivers/temp_deck/__init__.py @@ -1,5 +1,5 @@ from opentrons.drivers.temp_deck.driver import TempDeck __all__ = [ - TempDeck + 'TempDeck' ] diff --git a/api/opentrons/hardware_control/__init__.py b/api/opentrons/hardware_control/__init__.py index 31fef414733..d182aef151e 100644 --- a/api/opentrons/hardware_control/__init__.py +++ b/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) diff --git a/api/opentrons/helpers/__init__.py b/api/opentrons/helpers/__init__.py index 0db18682d54..80498597d0f 100644 --- a/api/opentrons/helpers/__init__.py +++ b/api/opentrons/helpers/__init__.py @@ -1,3 +1,3 @@ from .helpers import flip_coordinates -__all__ = [flip_coordinates] +__all__ = ['flip_coordinates'] diff --git a/api/opentrons/legacy_api/api.py b/api/opentrons/legacy_api/api.py index ba2fccad74c..51fa7d9a45f 100644 --- a/api/opentrons/legacy_api/api.py +++ b/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(): diff --git a/api/opentrons/protocol_api/__init__.py b/api/opentrons/protocol_api/__init__.py index 412c274d248..580e56c2ccb 100644 --- a/api/opentrons/protocol_api/__init__.py +++ b/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 diff --git a/api/opentrons/server/endpoints/serverlib_fallback.py b/api/opentrons/server/endpoints/serverlib_fallback.py index 0d5865c485e..8550b04e09b 100644 --- a/api/opentrons/server/endpoints/serverlib_fallback.py +++ b/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] } diff --git a/api/tests/opentrons/hardware_control/test_instantiation.py b/api/tests/opentrons/hardware_control/test_instantiation.py index 34f3a40062b..537e3803922 100644 --- a/api/tests/opentrons/hardware_control/test_instantiation.py +++ b/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) diff --git a/api/tests/opentrons/hardware_control/test_instruments.py b/api/tests/opentrons/hardware_control/test_instruments.py index d3070f104f0..ed36aa0ff72 100644 --- a/api/tests/opentrons/hardware_control/test_instruments.py +++ b/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,