style(api,robot-server): start using black to format some modules (#8158)

Author: mcous

.github/workflows/js-check.yaml

@@ -74,5 +74,3 @@ jobs:
         run: make lint-json
       - name: 'lint css'
         run: make lint-css
-      - name: 'stylecheck js'
-        run: make format

Makefile

@@ -179,14 +179,6 @@ test-js:
 .PHONY: lint
 lint: lint-py lint-js lint-json lint-css check-js circular-dependencies-js
 
-.PHONY: format
-format:
-ifeq ($(watch),true)
-	onchange $(FORMAT_FILE_GLOB) -- prettier --ignore-path .eslintignore --write {{changed}}
-else
-	prettier --ignore-path .eslintignore --write $(FORMAT_FILE_GLOB)
-endif
-
 .PHONY: lint-py
 lint-py:
 	$(MAKE) -C $(API_DIR) lint
@@ -208,6 +200,18 @@ lint-json:
 lint-css:
 	stylelint "**/*.css" "**/*.js"
 
+.PHONY: format
+format: format-js format-py
+
+.PHONY: format-py
+format-py:
+	$(MAKE) -C $(API_DIR) format
+	$(MAKE) -C $(ROBOT_SERVER_DIR) format
+
+.PHONY: format-js
+format-js:
+	prettier --ignore-path .eslintignore --write $(FORMAT_FILE_GLOB)
+
 .PHONY: check-js
 check-js: build-ts
 

api/Makefile

@@ -3,13 +3,6 @@
 include ../scripts/push.mk
 include ../scripts/python.mk
 
-# using bash instead of /bin/bash in SHELL prevents macOS optimizing away our PATH update
-SHELL := bash
-
-# add yarn CLI dev deps to PATH (for cross platform POSIX commands via shx)
-# and also make an explicit version for shx for use in the shell function,
-# where PATH won’t be propagated
-PATH := $(shell cd .. && yarn bin):$(PATH)
 SHX := npx shx
 
 # make push wheel file (= rather than := to expand at every use)
@@ -57,13 +50,27 @@ ot_shared_data_sources := $(filter %.json,$(shell $(SHX) find ../shared-data/))
 ot_resources := $(filter %,$(shell $(SHX) find src/opentrons/resources))
 ot_sources := $(ot_py_sources) $(ot_shared_data_sources) $(ot_resources)
 
+# test modules to typecheck
+# TODO(mc, 2021-07-23): expand this to all tests
 ot_tests_to_typecheck := \
 	tests/opentrons/protocol_api_experimental \
 	tests/opentrons/protocol_engine \
 	tests/opentrons/motion_planning \
 	tests/opentrons/file_runner \
 	tests/opentrons/protocols/runner
 
+# files and modules to format
+# TODO(mc, 2021-07-23): expand this to all files until we can format `.`
+ot_files_to_format := \
+	src/opentrons/file_runner \
+	src/opentrons/motion_planning \
+	src/opentrons/protocol_api_experimental \
+	src/opentrons/protocol_engine \
+	tests/opentrons/file_runner \
+	tests/opentrons/motion_planning  \
+	tests/opentrons/protocol_api_experimental \
+	tests/opentrons/protocol_engine
+
 # Defined separately than the clean target so the wheel file doesn’t have to
 # depend on a PHONY target
 clean_cmd = $(SHX) rm -rf build dist .coverage coverage.xml '*.egg-info' '**/__pycache__' '**/*.pyc' 'src/**/.mypy_cache'
@@ -99,10 +106,15 @@ test:
 	$(pytest) $(tests) $(test_opts)
 
 .PHONY: lint
-lint: $(ot_py_sources)
+lint:
 	$(python) -m mypy src/opentrons $(ot_tests_to_typecheck)
+	$(python) -m black --check $(ot_files_to_format)
 	$(python) -m flake8 setup.py src/opentrons tests
 
+.PHONY: format
+format:
+	$(python) -m black $(ot_files_to_format)
+
 docs/build/html/v%: docs/v%
 	$(sphinx_build) -b html -d docs/build/doctrees -n $< $@
 	$(SHX) cp docs/img/lightbulb.jpg $@/_images/ # sphinx wont automatically do this because it's only in a template

api/Pipfile

@@ -33,7 +33,8 @@ flake8-annotations = "~=2.6.2"
 flake8-docstrings = "~=1.6.0"
 flake8-noqa = "~=1.1.0"
 diff-match-patch = "*"
-decoy = "~=1.6.2"
+decoy = "~=1.6.4"
+black = "==21.7b0"
 
 [packages]
 aionotify = "==0.2.0"

api/Pipfile.lock

@@ -4,7 +4,7 @@
     DEFAULT_GENERAL_ARC_Z_MARGIN,
     DEFAULT_IN_LABWARE_ARC_Z_MARGIN,
     MINIMUM_Z_MARGIN,
-    get_waypoints
+    get_waypoints,
 )
 
 from .types import Waypoint, MoveType

api/src/opentrons/motion_planning/__init__.py

@@ -12,7 +12,7 @@ def __init__(
         clearance: float,
         min_travel_z: float,
         max_travel_z: float,
-        message: str
+        message: str,
     ) -> None:
         """Initialize an error with properties of the planned motion."""
         super().__init__(message)

api/src/opentrons/motion_planning/errors.py

@@ -62,7 +62,7 @@ def get_waypoints(
             clearance=MINIMUM_Z_MARGIN,
             min_travel_z=min_travel_z,
             max_travel_z=max_travel_z,
-            message="Destination out of bounds in the Z-axis"
+            message="Destination out of bounds in the Z-axis",
         )
 
     # ensure that the passed in min_travel_z and max_travel_z are compatible
@@ -73,7 +73,7 @@ def get_waypoints(
             clearance=MINIMUM_Z_MARGIN,
             min_travel_z=min_travel_z,
             max_travel_z=max_travel_z,
-            message="Arc out of bounds in the Z-axis"
+            message="Arc out of bounds in the Z-axis",
         )
 
     # set the z clearance according to the arc type
@@ -88,10 +88,7 @@ def get_waypoints(
     # if any of those exceed max_travel_z, just use max_travel_z
     # if max_travel_z does not provide enough clearance, check above would
     # raise an ArcOutOfBoundsError
-    travel_z = min(
-        max_travel_z,
-        max(min_travel_z + travel_z_margin, origin.z, dest.z)
-    )
+    travel_z = min(max_travel_z, max(min_travel_z + travel_z_margin, origin.z, dest.z))
 
     # if origin.z isn't the travel height: add waypoint to move to origin.z
     if travel_z > origin.z:

api/src/opentrons/motion_planning/waypoints.py

@@ -108,7 +108,7 @@ def quirks(self) -> List[str]:  # noqa: D102
     # operational logic, and its presence in this interface is no longer
     # necessary with Protocol Engine controlling execution. Can we get rid of it?
     @property
-    def magdeck_engage_height(self) -> Optional[float]:    # noqa: D102
+    def magdeck_engage_height(self) -> Optional[float]:  # noqa: D102
         definition = self._engine_client.state.labware.get_labware_definition(
             labware_id=self._labware_id
         )

api/src/opentrons/protocol_api_experimental/labware.py

@@ -45,7 +45,7 @@ async def get_deck_definition(
 
     async def get_deck_fixed_labware(
         self,
-        deck_definition: DeckDefinitionV2
+        deck_definition: DeckDefinitionV2,
     ) -> List[DeckFixedLabware]:
         """Get a list of all labware fixtures from a given deck definition."""
         labware = []