Merged PR 610: Adding more codestyle checkers and fixups

pylint apparently removed the bad-whitespace check at some point [1] This adds pycodestyle back to the mix to check for it. [1] pylint-dev/pylint#3512
microsoft · Sep 2, 2022 · 176dd1d · 176dd1d
1 parent 3361178
commit 176dd1d
Show file tree

Hide file tree

Showing 22 changed files with 163 additions and 94 deletions.
diff --git a/.azure-pipelines/linux.yml b/.azure-pipelines/linux.yml
@@ -72,21 +72,21 @@ jobs:
     displayName: Workaround a mamba cache bug in ADO
     # https://github.com/mamba-org/mamba/issues/488
 
-  - bash: make CONDA_DEFAULT_ENV=$(conda_env) CONDA_INFO_LEVEL=-v conda-env
+  - bash: make CONDA_ENV_NAME=$(conda_env) CONDA_INFO_LEVEL=-v conda-env
     displayName: 'Create/update mlos_core conda environment'
 
   - bash: conda run -n $(conda_env) pip install pytest-azurepipelines
     displayName: 'Install pytest-azurepipelines'
 
-  - bash: make CONDA_DEFAULT_ENV=$(conda_env) check
+  - bash: make CONDA_ENV_NAME=$(conda_env) check
     displayName: 'Run lint checks'
-  - bash: make CONDA_DEFAULT_ENV=$(conda_env) test
+  - bash: make CONDA_ENV_NAME=$(conda_env) test
     displayName: 'Run tests'
 
-  - bash: make CONDA_DEFAULT_ENV=$(conda_env) dist dist-test
+  - bash: make CONDA_ENV_NAME=$(conda_env) dist dist-test
     displayName: 'Generate and test binary distribution files'
 
-  - bash: make CONDA_DEFAULT_ENV=$(conda_env) doc
+  - bash: make CONDA_ENV_NAME=$(conda_env) doc
     # Only build docs for the default env.
     condition: eq(variables['conda_env'], 'mlos_core')
     displayName: 'Generate documentation'

diff --git a/.gitignore b/.gitignore
@@ -150,6 +150,7 @@ junit/test-results.xml
 .dist-test-env.*.build-stamp
 .dist-test.*.build-stamp
 .doc-prereqs.build-stamp
+.pycodestyle.build-stamp
 .pylint.build-stamp
 .pytest.build-stamp
 

diff --git a/.pylintrc b/.pylintrc
@@ -1,15 +1,36 @@
 # vim: set ft=dosini:
 
 [MAIN]
-
 # Specify a score threshold to be exceeded before program exits with error.
-fail-under=9.5
+fail-under=9.7
 
-[FORMAT]
+# Load some extra checkers.
+load-plugins=
+    pylint.extensions.bad_builtin,
+    pylint.extensions.code_style,
+    pylint.extensions.docparams,
+    pylint.extensions.docstyle,
+    pylint.extensions.for_any_all,
+    pylint.extensions.mccabe,
+    pylint.extensions.no_self_use,
+    pylint.extensions.private_import,
+    pylint.extensions.redefined_loop_name,
+    pylint.extensions.redefined_variable_type,
+    pylint.extensions.set_membership,
+    pylint.extensions.typing
 
+[FORMAT]
 # Maximum number of characters on a single line.
 max-line-length=132
 
 [MESSAGE CONTROL]
+disable=
+    no-else-return,
+    consider-using-assignment-expr,
+    deprecated-typing-alias,    # disable for now - only deprecated recently
+    docstring-first-line-empty,
+    missing-raises-doc
 
-disable=no-else-return
+[STRING]
+#check-quote-consistency=yes
+check-str-concat-over-line-jumps=yes
diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
-CONDA_DEFAULT_ENV := mlos_core
-PYTHON_VERSION := $(shell echo "${CONDA_DEFAULT_ENV}" | sed -r -e 's/^mlos_core[-]?//')
-ENV_YML := conda-envs/${CONDA_DEFAULT_ENV}.yml
+CONDA_ENV_NAME := mlos_core
+PYTHON_VERSION := $(shell echo "${CONDA_ENV_NAME}" | sed -r -e 's/^mlos_core[-]?//')
+ENV_YML := conda-envs/${CONDA_ENV_NAME}.yml
 
 # Find the non-build python files we should consider as rule dependencies.
 PYTHON_FILES := $(shell find ./ -type f -name '*.py' 2>/dev/null | egrep -v -e '^./((mlos_core|mlos_bench)/)?build/' -e '^./doc/source/')
@@ -16,27 +16,36 @@ CONDA_INFO_LEVEL := -q
 all: check test dist # doc
 
 .PHONY: conda-env
-conda-env: .conda-env.${CONDA_DEFAULT_ENV}.build-stamp
+conda-env: .conda-env.${CONDA_ENV_NAME}.build-stamp
 
-.conda-env.${CONDA_DEFAULT_ENV}.build-stamp: ${ENV_YML} mlos_core/setup.py mlos_bench/setup.py
-	conda env list -q | grep -q "^${CONDA_DEFAULT_ENV} " || conda env create --experimental-solver ${CONDA_SOLVER} ${CONDA_INFO_LEVEL} -n ${CONDA_DEFAULT_ENV} -f ${ENV_YML}
-	conda env update --experimental-solver ${CONDA_SOLVER} ${CONDA_INFO_LEVEL} -n ${CONDA_DEFAULT_ENV} --prune -f ${ENV_YML}
+.conda-env.${CONDA_ENV_NAME}.build-stamp: ${ENV_YML} mlos_core/setup.py mlos_bench/setup.py
+	conda env list -q | grep -q "^${CONDA_ENV_NAME} " || conda env create --experimental-solver ${CONDA_SOLVER} ${CONDA_INFO_LEVEL} -n ${CONDA_ENV_NAME} -f ${ENV_YML}
+	conda env update --experimental-solver ${CONDA_SOLVER} ${CONDA_INFO_LEVEL} -n ${CONDA_ENV_NAME} --prune -f ${ENV_YML}
 	$(MAKE) clean-check clean-test clean-doc
-	touch .conda-env.${CONDA_DEFAULT_ENV}.build-stamp
+	touch .conda-env.${CONDA_ENV_NAME}.build-stamp
 
 .PHONY: clean-conda-env
 clean-conda-env:
-	@conda env remove -n ${CONDA_DEFAULT_ENV}
-	rm -f .conda-env.${CONDA_DEFAULT_ENV}.build-stamp
+	conda env remove -n ${CONDA_ENV_NAME}
+	rm -f .conda-env.${CONDA_ENV_NAME}.build-stamp
 
 .PHONY: check
-check: pylint
+check: pycodestyle pylint
+
+.PHONY: pycodestyle
+pycodestyle: conda-env .pycodestyle.build-stamp
+
+.pycodestyle.build-stamp: $(PYTHON_FILES) setup.cfg
+	# Check for decent pep8 code style with pycodestyle.
+	# Note: if this fails, try using autopep8 to fix it.
+	conda run -n ${CONDA_ENV_NAME} pycodestyle $(PYTHON_FILES)
+	touch .pycodestyle.build-stamp
 
 .PHONY: pylint
 pylint: conda-env .pylint.build-stamp
 
 .pylint.build-stamp: $(PYTHON_FILES) .pylintrc
-	conda run -n ${CONDA_DEFAULT_ENV} pylint -j0 $(PYTHON_FILES)
+	conda run -n ${CONDA_ENV_NAME} pylint -j0 $(PYTHON_FILES)
 	touch .pylint.build-stamp
 
 .PHONY: test
@@ -48,8 +57,8 @@ pytest: conda-env .pytest.build-stamp
 # Make sure pytest can find our pytest_configure.py file.
 .pytest.build-stamp: export PYTHONPATH := $(PWD):$(PYTHONPATH)
 .pytest.build-stamp: $(PYTHON_FILES) pytest.ini
-	#conda run -n ${CONDA_DEFAULT_ENV} pytest -n auto --cov=mlos_core --cov-report=xml mlos_core/ mlos_bench/
-	conda run -n ${CONDA_DEFAULT_ENV} pytest --cov --cov-report=xml mlos_core/ mlos_bench/ --junitxml=junit/test-results.xml
+	#conda run -n ${CONDA_ENV_NAME} pytest -n auto --cov=mlos_core --cov-report=xml mlos_core/ mlos_bench/
+	conda run -n ${CONDA_ENV_NAME} pytest --cov --cov-report=xml mlos_core/ mlos_bench/ --junitxml=junit/test-results.xml
 	touch .pytest.build-stamp
 
 .PHONY: dist
@@ -61,14 +70,14 @@ bdist_wheel: conda-env mlos_core/dist/mlos_core-*-py3-none-any.whl mlos_bench/di
 mlos_core/dist/mlos_core-*-py3-none-any.whl: mlos_core/setup.py $(MLOS_CORE_PYTHON_FILES)
 	rm -f mlos_core/dist/mlos_core-*-py3-none-any.whl \
 	    && cd mlos_core/ \
-	    && conda run -n ${CONDA_DEFAULT_ENV} python3 setup.py bdist_wheel \
+	    && conda run -n ${CONDA_ENV_NAME} python3 setup.py bdist_wheel \
 	    && cd .. \
 	    && ls mlos_core/dist/mlos_core-*-py3-none-any.whl
 
 mlos_bench/dist/mlos_bench-*-py3-none-any.whl: mlos_bench/setup.py $(MLOS_BENCH_PYTHON_FILES)
 	rm -f mlos_bench/dist/mlos_bench-*-py3-none-any.whl \
 	    && cd mlos_bench/ \
-	    && conda run -n ${CONDA_DEFAULT_ENV} python3 setup.py bdist_wheel \
+	    && conda run -n ${CONDA_ENV_NAME} python3 setup.py bdist_wheel \
 	    && cd .. \
 	    && ls mlos_bench/dist/mlos_bench-*-py3-none-any.whl
 
@@ -120,7 +129,7 @@ dist-test-clean: dist-test-env-clean
 	rm -f .dist-test.$(PYTHON_VERSION).build-stamp
 
 .doc-prereqs.build-stamp: doc/requirements.txt
-	conda run -n ${CONDA_DEFAULT_ENV} pip install -r doc/requirements.txt
+	conda run -n ${CONDA_ENV_NAME} pip install -r doc/requirements.txt
 	touch .doc-prereqs.build-stamp
 
 .PHONY: doc-prereqs
@@ -132,9 +141,9 @@ clean-doc-env:
 
 .PHONY: doc
 doc: conda-env doc-prereqs clean-doc
-	cd doc/ && conda run -n ${CONDA_DEFAULT_ENV} sphinx-apidoc -f -e -M -o source/api/mlos_core/ ../mlos_core/ ../mlos_*/setup.py ../pytest_configure.py
-	cd doc/ && conda run -n ${CONDA_DEFAULT_ENV} sphinx-apidoc -f -e -M -o source/api/mlos_bench/ ../mlos_bench/ ../mlos_*/setup.py ../pytest_configure.py
-	conda run -n ${CONDA_DEFAULT_ENV} make -j -C doc/ html
+	cd doc/ && conda run -n ${CONDA_ENV_NAME} sphinx-apidoc -f -e -M -o source/api/mlos_core/ ../mlos_core/ ../mlos_*/setup.py ../pytest_configure.py
+	cd doc/ && conda run -n ${CONDA_ENV_NAME} sphinx-apidoc -f -e -M -o source/api/mlos_bench/ ../mlos_bench/ ../mlos_*/setup.py ../pytest_configure.py
+	conda run -n ${CONDA_ENV_NAME} make -j -C doc/ html
 	test -s doc/build/html/index.html
 	test -s doc/build/html/generated/mlos_core.optimizers.BaseOptimizer.html
 	test -s doc/build/html/generated/mlos_bench.main.optimize.html
@@ -149,6 +158,7 @@ clean-doc:
 .PHONY: clean-check
 clean-check:
 	rm -f .pylint.build-stamp
+	rm -f .pycodestyle.build-stamp
 
 .PHONY: clean-test
 clean-test:

diff --git a/conda-envs/mlos_core-3.8.yml b/conda-envs/mlos_core-3.8.yml
@@ -10,6 +10,7 @@ dependencies:
   - configspace
   - pip
   - pylint
+  - pycodestyle
   - autopep8
   - pytest
   - setuptools

diff --git a/conda-envs/mlos_core-3.9.yml b/conda-envs/mlos_core-3.9.yml
@@ -10,6 +10,7 @@ dependencies:
   - configspace
   - pip
   - pylint
+  - pycodestyle
   - autopep8
   - pytest
   - setuptools

diff --git a/conda-envs/mlos_core.yml b/conda-envs/mlos_core.yml
@@ -10,6 +10,7 @@ dependencies:
   - configspace
   - pip
   - pylint
+  - pycodestyle
   - autopep8
   - pytest
   - setuptools

diff --git a/mlos_bench/mlos_bench/environment/azure/azure_services.py b/mlos_bench/mlos_bench/environment/azure/azure_services.py
@@ -5,15 +5,15 @@
 import json
 import logging
 import time
-from typing import Tuple, Dict
+from typing import Any, Tuple, Dict
 import requests
 
 from mlos_bench.environment import Service, Status, _check_required_params
 
 _LOG = logging.getLogger(__name__)
 
 
-class AzureVMService(Service):
+class AzureVMService(Service):  # pylint: disable=too-many-instance-attributes
     """
     Helper methods to manage VMs on Azure.
     """
@@ -22,11 +22,11 @@ class AzureVMService(Service):
     # https://docs.microsoft.com/en-us/rest/api/resources/deployments
 
     _URL_DEPLOY = (
-        "https://management.azure.com"
-        "/subscriptions/{subscription}"
-        "/resourceGroups/{resource_group}"
-        "/providers/Microsoft.Resources"
-        "/deployments/{deployment_name}"
+        "https://management.azure.com" +
+        "/subscriptions/{subscription}" +
+        "/resourceGroups/{resource_group}" +
+        "/providers/Microsoft.Resources" +
+        "/deployments/{deployment_name}" +
         "?api-version=2022-05-01"
     )
 
@@ -35,56 +35,56 @@ class AzureVMService(Service):
 
     # From: https://docs.microsoft.com/en-us/rest/api/compute/virtual-machines/start?tabs=HTTP
     _URL_START = (
-        "https://management.azure.com"
-        "/subscriptions/{subscription}"
-        "/resourceGroups/{resource_group}"
-        "/providers/Microsoft.Compute"
-        "/virtualMachines/{vm_name}"
-        "/start"
+        "https://management.azure.com" +
+        "/subscriptions/{subscription}" +
+        "/resourceGroups/{resource_group}" +
+        "/providers/Microsoft.Compute" +
+        "/virtualMachines/{vm_name}" +
+        "/start" +
         "?api-version=2022-03-01"
     )
 
     # From: https://docs.microsoft.com/en-us/rest/api/compute/virtual-machines/power-off?tabs=HTTP
     _URL_STOP = (
-        "https://management.azure.com"
-        "/subscriptions/{subscription}"
-        "/resourceGroups/{resource_group}"
-        "/providers/Microsoft.Compute"
-        "/virtualMachines/{vm_name}"
-        "/powerOff"
+        "https://management.azure.com" +
+        "/subscriptions/{subscription}" +
+        "/resourceGroups/{resource_group}" +
+        "/providers/Microsoft.Compute" +
+        "/virtualMachines/{vm_name}" +
+        "/powerOff" +
         "?api-version=2022-03-01"
     )
 
     # From: https://docs.microsoft.com/en-us/rest/api/compute/virtual-machines/deallocate?tabs=HTTP
     _URL_DEPROVISION = (
-        "https://management.azure.com"
-        "/subscriptions/{subscription}"
-        "/resourceGroups/{resource_group}"
-        "/providers/Microsoft.Compute"
-        "/virtualMachines/{vm_name}"
-        "/deallocate"
+        "https://management.azure.com" +
+        "/subscriptions/{subscription}" +
+        "/resourceGroups/{resource_group}" +
+        "/providers/Microsoft.Compute" +
+        "/virtualMachines/{vm_name}" +
+        "/deallocate" +
         "?api-version=2022-03-01"
     )
 
     # From: https://docs.microsoft.com/en-us/rest/api/compute/virtual-machines/restart?tabs=HTTP
     _URL_REBOOT = (
-        "https://management.azure.com"
-        "/subscriptions/{subscription}"
-        "/resourceGroups/{resource_group}"
-        "/providers/Microsoft.Compute"
-        "/virtualMachines/{vm_name}"
-        "/restart"
+        "https://management.azure.com" +
+        "/subscriptions/{subscription}" +
+        "/resourceGroups/{resource_group}" +
+        "/providers/Microsoft.Compute" +
+        "/virtualMachines/{vm_name}" +
+        "/restart" +
         "?api-version=2022-03-01"
     )
 
     # From: https://docs.microsoft.com/en-us/rest/api/compute/virtual-machines/run-command?tabs=HTTP
     _URL_REXEC_RUN = (
-        "https://management.azure.com"
-        "/subscriptions/{subscription}"
-        "/resourceGroups/{resource_group}"
-        "/providers/Microsoft.Compute"
-        "/virtualMachines/{vm_name}"
-        "/runCommand"
+        "https://management.azure.com" +
+        "/subscriptions/{subscription}" +
+        "/resourceGroups/{resource_group}" +
+        "/providers/Microsoft.Compute" +
+        "/virtualMachines/{vm_name}" +
+        "/runCommand" +
         "?api-version=2022-03-01"
     )
 
@@ -276,7 +276,7 @@ def check_vm_operation_status(self, params: Dict) -> Tuple[Status, Dict]:
         return Status.FAILED, {}
 
     def wait_vm_operation(self,
-        params: Dict,
+        params: Dict[str, Any],
         default_poll_period: float = 1.0,
         timeout: float = 3600,
     ) -> Tuple[Status, Dict]:
@@ -441,13 +441,14 @@ def vm_reboot(self) -> Tuple[Status, Dict]:
 
         return self._azure_vm_post_helper(self._url_reboot)
 
-    def remote_exec(self, params: Dict) -> Tuple[Status, Dict]:
+    # TODO: Consider replacing this dict with expected params with a custom object with named attrs.
+    def remote_exec(self, params: Dict[str, Any]) -> Tuple[Status, Dict]:
         """
         Run a command on Azure VM.
 
         Parameters
         ----------
-        tunables : dict
+        params : dict
             Flat dictionary of (key, value) pairs of tunable parameters.
             Must have "commandId", "script", and "parameters" keys.
 

diff --git a/mlos_bench/mlos_bench/environment/azure/tests/azure_services_test.py b/mlos_bench/mlos_bench/environment/azure/tests/azure_services_test.py
@@ -11,6 +11,7 @@
 # pylint: disable=missing-function-docstring
 # pylint: disable=too-many-arguments
 
+
 @pytest.mark.parametrize(
     ("operation_name", "accepts_params"), [
         ("vm_start", True),

diff --git a/mlos_bench/mlos_bench/environment/azure/tests/conftest.py b/mlos_bench/mlos_bench/environment/azure/tests/conftest.py
@@ -1,9 +1,17 @@
+"""
+Configuration test fixtures for azure_services in mlos_bench.
+"""
+
 import pytest
 
 from mlos_bench.environment.azure.azure_services import AzureVMService
 
+
 @pytest.fixture
 def azure_vm_service():
+    """
+    Creates a dummy Azure VM service for tests that require it.
+    """
     service = AzureVMService(config={
         "deployTemplatePath": "./mlos_bench/config/azure/azuredeploy-ubuntu-vm.json",
         "deploymentName": "TEST_DEPLOYMENT",

diff --git a/mlos_bench/mlos_bench/environment/base_environment.py b/mlos_bench/mlos_bench/environment/base_environment.py
@@ -137,7 +137,7 @@ def tunable_params(self):
         """
         return self._tunable_params
 
-    def setup(self): # pylint: disable=no-self-use
+    def setup(self):    # pylint: disable=no-self-use
         """
         Set up a new benchmark environment, if necessary. This method must be
         idempotent, i.e., calling it several times in a row should be