Enable black, isort, and doc formatters and checks (#774)

Follow on work to #766. This enabled both formatters and applies their changes to the repo. Additionally, since `black` does not make changes to comments nor docstrings, we also enable `docformatter` to reformat docstrings which better aligns with `pydocstyle` rules as well. Without this additional change (and some manual fixups), `pycodestyle` and `pylint` would still complain about line lengths, for instance. Finally, we make a minor adjustment to the max line length setting it to 99 (which is also accepted and mentioned in pep8) instead of 88 to avoid some comment (especially linter overrides) wrapping.
microsoft · Jul 12, 2024 · e40ac28 · e40ac28
1 parent fd9c8f9
commit e40ac28
Show file tree

Hide file tree

Showing 268 changed files with 9,141 additions and 7,338 deletions.
diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json
@@ -45,7 +45,6 @@
                 // Adjust the python interpreter path to point to the conda environment
                 "python.defaultInterpreterPath": "/opt/conda/envs/mlos/bin/python",
                 "python.testing.pytestPath": "/opt/conda/envs/mlos/bin/pytest",
-                "python.formatting.autopep8Path": "/opt/conda/envs/mlos/bin/autopep8",
                 "python.linting.pylintPath": "/opt/conda/envs/mlos/bin/pylint",
                 "pylint.path": [
                     "/opt/conda/envs/mlos/bin/pylint"
@@ -71,9 +70,8 @@
                 "lextudio.restructuredtext",
                 "matangover.mypy",
                 "ms-azuretools.vscode-docker",
-                // TODO: Enable additional formatter extensions:
-                //"ms-python.black-formatter",
-                //"ms-python.isort",
+                "ms-python.black-formatter",
+                "ms-python.isort",
                 "ms-python.pylint",
                 "ms-python.python",
                 "ms-python.vscode-pylance",

diff --git a/.editorconfig b/.editorconfig
@@ -12,6 +12,9 @@ charset = utf-8
 # Note: this is not currently supported by all editors or their editorconfig plugins.
 max_line_length = 132
 
+[*.py]
+max_line_length = 99
+
 # Makefiles need tab indentation
 [{Makefile,*.mk}]
 indent_style = tab

diff --git a/.gitignore b/.gitignore
@@ -1,3 +1,6 @@
+# Ignore git directory (ripgrep)
+.git/
+
 # Byte-compiled / optimized / DLL files
 __pycache__/
 *.py[cod]

diff --git a/.pylintrc b/.pylintrc
@@ -35,7 +35,7 @@ load-plugins=
 
 [FORMAT]
 # Maximum number of characters on a single line.
-max-line-length=132
+max-line-length=99
 
 [MESSAGE CONTROL]
 disable=
@@ -48,5 +48,5 @@ disable=
     missing-raises-doc
 
 [STRING]
-#check-quote-consistency=yes
+check-quote-consistency=yes
 check-str-concat-over-line-jumps=yes
diff --git a/.vscode/extensions.json b/.vscode/extensions.json
@@ -14,9 +14,8 @@
         "lextudio.restructuredtext",
         "matangover.mypy",
         "ms-azuretools.vscode-docker",
-	// TODO: Enable additional formatter extensions:
-        //"ms-python.black-formatter",
-        //"ms-python.isort",
+        "ms-python.black-formatter",
+        "ms-python.isort",
         "ms-python.pylint",
         "ms-python.python",
         "ms-python.vscode-pylance",

diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -125,14 +125,10 @@
     ],
     "esbonio.sphinx.confDir": "${workspaceFolder}/doc/source",
     "esbonio.sphinx.buildDir": "${workspaceFolder}/doc/build/",
-    "autopep8.args": [
-        "--experimental"
-    ],
     "[python]": {
-        // TODO: Enable black formatter
-        //"editor.defaultFormatter": "ms-python.black-formatter",
-        //"editor.formatOnSave": true,
-        //"editor.formatOnSaveMode": "modifications"
+        "editor.defaultFormatter": "ms-python.black-formatter",
+        "editor.formatOnSave": true,
+        "editor.formatOnSaveMode": "modifications"
     },
     // See Also .vscode/launch.json for environment variable args to pytest during debug sessions.
     // For the rest, see setup.cfg

diff --git a/Makefile b/Makefile
@@ -34,7 +34,7 @@ conda-env: build/conda-env.${CONDA_ENV_NAME}.build-stamp
 MLOS_CORE_CONF_FILES := mlos_core/pyproject.toml mlos_core/setup.py mlos_core/MANIFEST.in
 MLOS_BENCH_CONF_FILES := mlos_bench/pyproject.toml mlos_bench/setup.py mlos_bench/MANIFEST.in
 MLOS_VIZ_CONF_FILES := mlos_viz/pyproject.toml mlos_viz/setup.py mlos_viz/MANIFEST.in
-MLOS_GLOBAL_CONF_FILES := setup.cfg # pyproject.toml
+MLOS_GLOBAL_CONF_FILES := setup.cfg pyproject.toml
 
 MLOS_PKGS := mlos_core mlos_bench mlos_viz
 MLOS_PKG_CONF_FILES := $(MLOS_CORE_CONF_FILES) $(MLOS_BENCH_CONF_FILES) $(MLOS_VIZ_CONF_FILES) $(MLOS_GLOBAL_CONF_FILES)
@@ -69,9 +69,9 @@ ifneq (,$(filter format,$(MAKECMDGOALS)))
 endif
 
 build/format.${CONDA_ENV_NAME}.build-stamp: build/licenseheaders.${CONDA_ENV_NAME}.build-stamp
-# TODO: enable isort and black formatters
-#build/format.${CONDA_ENV_NAME}.build-stamp: build/isort.${CONDA_ENV_NAME}.build-stamp
-#build/format.${CONDA_ENV_NAME}.build-stamp: build/black.${CONDA_ENV_NAME}.build-stamp
+build/format.${CONDA_ENV_NAME}.build-stamp: build/isort.${CONDA_ENV_NAME}.build-stamp
+build/format.${CONDA_ENV_NAME}.build-stamp: build/black.${CONDA_ENV_NAME}.build-stamp
+build/format.${CONDA_ENV_NAME}.build-stamp: build/docformatter.${CONDA_ENV_NAME}.build-stamp
 build/format.${CONDA_ENV_NAME}.build-stamp:
 	touch $@
 
@@ -111,8 +111,8 @@ build/isort.${CONDA_ENV_NAME}.build-stamp:
 # NOTE: when using pattern rules (involving %) we can only add one line of
 # prerequisities, so we use this pattern to compose the list as variables.
 
-# Both isort and licenseheaders alter files, so only run one at a time, by
-# making licenseheaders an order-only prerequisite.
+# black, licenseheaders, isort, and docformatter all alter files, so only run
+# one at a time, by adding prerequisites, but only as necessary.
 ISORT_COMMON_PREREQS :=
 ifneq (,$(filter format licenseheaders,$(MAKECMDGOALS)))
 ISORT_COMMON_PREREQS += build/licenseheaders.${CONDA_ENV_NAME}.build-stamp
@@ -126,7 +126,7 @@ build/isort.mlos_viz.${CONDA_ENV_NAME}.build-stamp: $(MLOS_VIZ_PYTHON_FILES)
 
 build/isort.%.${CONDA_ENV_NAME}.build-stamp: $(ISORT_COMMON_PREREQS)
 	# Reformat python file imports with isort.
-	conda run -n ${CONDA_ENV_NAME} isort --verbose --only-modified --atomic -j0 $(filter %.py,$?)
+	conda run -n ${CONDA_ENV_NAME} isort --verbose --only-modified --atomic -j0 $(filter %.py,$+)
 	touch $@
 
 .PHONY: black
@@ -142,8 +142,8 @@ build/black.${CONDA_ENV_NAME}.build-stamp: build/black.mlos_viz.${CONDA_ENV_NAME
 build/black.${CONDA_ENV_NAME}.build-stamp:
 	touch $@
 
-# Both black, licenseheaders, and isort all alter files, so only run one at a time, by
-# making licenseheaders and isort an order-only prerequisite.
+# black, licenseheaders, isort, and docformatter all alter files, so only run
+# one at a time, by adding prerequisites, but only as necessary.
 BLACK_COMMON_PREREQS :=
 ifneq (,$(filter format licenseheaders,$(MAKECMDGOALS)))
 BLACK_COMMON_PREREQS += build/licenseheaders.${CONDA_ENV_NAME}.build-stamp
@@ -160,13 +160,52 @@ build/black.mlos_viz.${CONDA_ENV_NAME}.build-stamp: $(MLOS_VIZ_PYTHON_FILES)
 
 build/black.%.${CONDA_ENV_NAME}.build-stamp: $(BLACK_COMMON_PREREQS)
 	# Reformat python files with black.
-	conda run -n ${CONDA_ENV_NAME} black $(filter %.py,$?)
+	conda run -n ${CONDA_ENV_NAME} black $(filter %.py,$+)
 	touch $@
 
+.PHONY: docformatter
+docformatter: build/docformatter.${CONDA_ENV_NAME}.build-stamp
+
+ifneq (,$(filter docformatter,$(MAKECMDGOALS)))
+    FORMAT_PREREQS += build/docformatter.${CONDA_ENV_NAME}.build-stamp
+endif
+
+build/docformatter.${CONDA_ENV_NAME}.build-stamp: build/docformatter.mlos_core.${CONDA_ENV_NAME}.build-stamp
+build/docformatter.${CONDA_ENV_NAME}.build-stamp: build/docformatter.mlos_bench.${CONDA_ENV_NAME}.build-stamp
+build/docformatter.${CONDA_ENV_NAME}.build-stamp: build/docformatter.mlos_viz.${CONDA_ENV_NAME}.build-stamp
+build/docformatter.${CONDA_ENV_NAME}.build-stamp:
+	touch $@
+
+# black, licenseheaders, isort, and docformatter all alter files, so only run
+# one at a time, by adding prerequisites, but only as necessary.
+DOCFORMATTER_COMMON_PREREQS :=
+ifneq (,$(filter format licenseheaders,$(MAKECMDGOALS)))
+DOCFORMATTER_COMMON_PREREQS += build/licenseheaders.${CONDA_ENV_NAME}.build-stamp
+endif
+ifneq (,$(filter format isort,$(MAKECMDGOALS)))
+DOCFORMATTER_COMMON_PREREQS += build/isort.${CONDA_ENV_NAME}.build-stamp
+endif
+ifneq (,$(filter format black,$(MAKECMDGOALS)))
+DOCFORMATTER_COMMON_PREREQS += build/black.${CONDA_ENV_NAME}.build-stamp
+endif
+DOCFORMATTER_COMMON_PREREQS += build/conda-env.${CONDA_ENV_NAME}.build-stamp
+DOCFORMATTER_COMMON_PREREQS += $(MLOS_GLOBAL_CONF_FILES)
+
+build/docformatter.mlos_core.${CONDA_ENV_NAME}.build-stamp: $(MLOS_CORE_PYTHON_FILES)
+build/docformatter.mlos_bench.${CONDA_ENV_NAME}.build-stamp: $(MLOS_BENCH_PYTHON_FILES)
+build/docformatter.mlos_viz.${CONDA_ENV_NAME}.build-stamp: $(MLOS_VIZ_PYTHON_FILES)
+
+# docformatter returns non-zero when it changes anything so instead we ignore that
+# return code and just have it recheck itself immediately
+build/docformatter.%.${CONDA_ENV_NAME}.build-stamp: $(DOCFORMATTER_COMMON_PREREQS)
+	# Reformat python file docstrings with docformatter.
+	conda run -n ${CONDA_ENV_NAME} docformatter --in-place $(filter %.py,$+) || true
+	conda run -n ${CONDA_ENV_NAME} docformatter --check --diff $(filter %.py,$+)
+	touch $@
+
+
 .PHONY: check
-check: pycodestyle pydocstyle pylint mypy # cspell markdown-link-check
-# TODO: Enable isort and black checks
-#check: isort-check black-check pycodestyle pydocstyle pylint mypy # cspell markdown-link-check
+check: isort-check black-check docformatter-check pycodestyle pydocstyle pylint mypy # cspell markdown-link-check
 
 .PHONY: black-check
 black-check: build/black-check.mlos_core.${CONDA_ENV_NAME}.build-stamp
@@ -185,7 +224,27 @@ BLACK_CHECK_COMMON_PREREQS += $(MLOS_GLOBAL_CONF_FILES)
 build/black-check.%.${CONDA_ENV_NAME}.build-stamp: $(BLACK_CHECK_COMMON_PREREQS)
 	# Check for import sort order.
 	# Note: if this fails use "make format" or "make black" to fix it.
-	conda run -n ${CONDA_ENV_NAME} black --verbose --check --diff $(filter %.py,$?)
+	conda run -n ${CONDA_ENV_NAME} black --verbose --check --diff $(filter %.py,$+)
+	touch $@
+
+.PHONY: docformatter-check
+docformatter-check: build/docformatter-check.mlos_core.${CONDA_ENV_NAME}.build-stamp
+docformatter-check: build/docformatter-check.mlos_bench.${CONDA_ENV_NAME}.build-stamp
+docformatter-check: build/docformatter-check.mlos_viz.${CONDA_ENV_NAME}.build-stamp
+
+# Make sure docformatter format rules run before docformatter-check rules.
+build/docformatter-check.mlos_core.${CONDA_ENV_NAME}.build-stamp: $(MLOS_CORE_PYTHON_FILES)
+build/docformatter-check.mlos_bench.${CONDA_ENV_NAME}.build-stamp: $(MLOS_BENCH_PYTHON_FILES)
+build/docformatter-check.mlos_viz.${CONDA_ENV_NAME}.build-stamp: $(MLOS_VIZ_PYTHON_FILES)
+
+BLACK_CHECK_COMMON_PREREQS := build/conda-env.${CONDA_ENV_NAME}.build-stamp
+BLACK_CHECK_COMMON_PREREQS += $(FORMAT_PREREQS)
+BLACK_CHECK_COMMON_PREREQS += $(MLOS_GLOBAL_CONF_FILES)
+
+build/docformatter-check.%.${CONDA_ENV_NAME}.build-stamp: $(BLACK_CHECK_COMMON_PREREQS)
+	# Check for import sort order.
+	# Note: if this fails use "make format" or "make docformatter" to fix it.
+	conda run -n ${CONDA_ENV_NAME} docformatter --check --diff $(filter %.py,$+)
 	touch $@
 
 .PHONY: isort-check
@@ -204,7 +263,7 @@ ISORT_CHECK_COMMON_PREREQS += $(MLOS_GLOBAL_CONF_FILES)
 
 build/isort-check.%.${CONDA_ENV_NAME}.build-stamp: $(ISORT_CHECK_COMMON_PREREQS)
 	# Note: if this fails use "make format" or "make isort" to fix it.
-	conda run -n ${CONDA_ENV_NAME} isort --only-modified --check --diff -j0 $(filter %.py,$?)
+	conda run -n ${CONDA_ENV_NAME} isort --only-modified --check --diff -j0 $(filter %.py,$+)
 	touch $@
 
 .PHONY: pycodestyle
@@ -723,7 +782,12 @@ clean-doc:
 
 .PHONY: clean-format
 clean-format:
-	# TODO: add black and isort rules
+	rm -f build/black.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/black.mlos_*.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/docformatter.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/docformatter.mlos_*.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/isort.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/isort.mlos_*.${CONDA_ENV_NAME}.build-stamp
 	rm -f build/licenseheaders.${CONDA_ENV_NAME}.build-stamp
 	rm -f build/licenseheaders-prereqs.${CONDA_ENV_NAME}.build-stamp
 
@@ -733,6 +797,13 @@ clean-check:
 	rm -f build/pylint.${CONDA_ENV_NAME}.build-stamp
 	rm -f build/pylint.mlos_*.${CONDA_ENV_NAME}.build-stamp
 	rm -f build/mypy.mlos_*.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/black-check.build-stamp
+	rm -f build/black-check.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/black-check.mlos_*.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/docformatter-check.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/docformatter-check.mlos_*.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/isort-check.${CONDA_ENV_NAME}.build-stamp
+	rm -f build/isort-check.mlos_*.${CONDA_ENV_NAME}.build-stamp
 	rm -f build/pycodestyle.build-stamp
 	rm -f build/pycodestyle.${CONDA_ENV_NAME}.build-stamp
 	rm -f build/pycodestyle.mlos_*.${CONDA_ENV_NAME}.build-stamp

diff --git a/conda-envs/mlos-3.10.yml b/conda-envs/mlos-3.10.yml
@@ -25,10 +25,10 @@ dependencies:
   # See comments in mlos.yml.
   #- gcc_linux-64
   - pip:
-    - autopep8>=1.7.0
     - bump2version
     - check-jsonschema
     - isort
+    - docformatter
     - licenseheaders
     - mypy
     - pandas-stubs

diff --git a/conda-envs/mlos-3.11.yml b/conda-envs/mlos-3.11.yml
@@ -25,10 +25,10 @@ dependencies:
   # See comments in mlos.yml.
   #- gcc_linux-64
   - pip:
-    - autopep8>=1.7.0
     - bump2version
     - check-jsonschema
     - isort
+    - docformatter
     - licenseheaders
     - mypy
     - pandas-stubs

diff --git a/conda-envs/mlos-3.8.yml b/conda-envs/mlos-3.8.yml
@@ -25,10 +25,10 @@ dependencies:
   # See comments in mlos.yml.
   #- gcc_linux-64
   - pip:
-    - autopep8>=1.7.0
     - bump2version
     - check-jsonschema
     - isort
+    - docformatter
     - licenseheaders
     - mypy
     - pandas-stubs

diff --git a/conda-envs/mlos-3.9.yml b/conda-envs/mlos-3.9.yml
@@ -25,10 +25,10 @@ dependencies:
   # See comments in mlos.yml.
   #- gcc_linux-64
   - pip:
-    - autopep8>=1.7.0
     - bump2version
     - check-jsonschema
     - isort
+    - docformatter
     - licenseheaders
     - mypy
     - pandas-stubs

diff --git a/conda-envs/mlos-windows.yml b/conda-envs/mlos-windows.yml
@@ -28,10 +28,10 @@ dependencies:
   # This also requires a more recent vs2015_runtime from conda-forge.
   - pyrfr>=0.9.0
   - pip:
-    - autopep8>=1.7.0
     - bump2version
     - check-jsonschema
     - isort
+    - docformatter
     - licenseheaders
     - mypy
     - pandas-stubs

diff --git a/conda-envs/mlos.yml b/conda-envs/mlos.yml
@@ -24,10 +24,10 @@ dependencies:
   # FIXME: https://github.com/microsoft/MLOS/issues/727
   - python<3.12
   - pip:
-    - autopep8>=1.7.0
     - bump2version
     - check-jsonschema
     - isort
+    - docformatter
     - licenseheaders
     - mypy
     - pandas-stubs

diff --git a/conftest.py b/conftest.py
@@ -32,14 +32,23 @@ def pytest_configure(config: pytest.Config) -> None:
     Add some additional (global) configuration steps for pytest.
     """
     # Workaround some issues loading emukit in certain environments.
-    if os.environ.get('DISPLAY', None):
+    if os.environ.get("DISPLAY", None):
         try:
-            import matplotlib   # pylint: disable=import-outside-toplevel
-            matplotlib.rcParams['backend'] = 'agg'
-            if is_master(config) or dict(getattr(config, 'workerinput', {}))['workerid'] == 'gw0':
+            import matplotlib  # pylint: disable=import-outside-toplevel
+
+            matplotlib.rcParams["backend"] = "agg"
+            if is_master(config) or dict(getattr(config, "workerinput", {}))["workerid"] == "gw0":
                 # Only warn once.
-                warn(UserWarning('DISPLAY environment variable is set, which can cause problems in some setups (e.g. WSL). '
-                                 + f'Adjusting matplotlib backend to "{matplotlib.rcParams["backend"]}" to compensate.'))
+                warn(
+                    UserWarning(
+                        (
+                            "DISPLAY environment variable is set, "
+                            "which can cause problems in some setups (e.g. WSL). "
+                            f"Adjusting matplotlib backend to '{matplotlib.rcParams['backend']}' "
+                            "to compensate."
+                        )
+                    )
+                )
         except ImportError:
             pass
 

diff --git a/doc/source/conf.py b/doc/source/conf.py
@@ -87,10 +87,11 @@
 numpydoc_class_members_toctree = False
 
 autodoc_default_options = {
-    'members': True,
-    'undoc-members': True,
-    # Don't generate documentation for some (non-private) functions that are more for internal implementation use.
-    'exclude-members': 'mlos_bench.util.check_required_params'
+    "members": True,
+    "undoc-members": True,
+    # Don't generate documentation for some (non-private) functions that are more
+    # for internal implementation use.
+    "exclude-members": "mlos_bench.util.check_required_params",
 }
 
 # Generate the plots for the gallery

diff --git a/mlos_bench/mlos_bench/__init__.py b/mlos_bench/mlos_bench/__init__.py
@@ -2,7 +2,6 @@
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT License.
 #
-"""
-mlos_bench is a framework to help automate benchmarking and and
-OS/application parameter autotuning.
+"""mlos_bench is a framework to help automate benchmarking and and OS/application
+parameter autotuning.
 """
diff --git a/mlos_bench/mlos_bench/config/__init__.py b/mlos_bench/mlos_bench/config/__init__.py
@@ -2,6 +2,4 @@
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT License.
 #
-"""
-mlos_bench.config
-"""
+"""mlos_bench.config."""