From c7c5b03a74e1c1ffc14b09b45b3e968231706ee1 Mon Sep 17 00:00:00 2001 From: Peter Hessey Date: Tue, 21 Mar 2023 09:21:09 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=92=84=20Add=20black=20styling=20and=20co?= =?UTF-8?q?nfiguration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MNT: Fix incorrect type annotations (#846) ENH: Refactor Swin Transformer checkpointing (#843) Refactor Swin Transformer checkpointing to enable patch embedding checkpointing overriding without duplication the whole custom forward MNT: Fix incorrect type annotations (#846) ENH: Refactor Swin Transformer checkpointing (#843) Refactor Swin Transformer checkpointing to enable patch embedding checkpointing overriding without duplication the whole custom forward Add multi-modal to root makefile 🐛 Fix Makefile updates 📝 Update dev documentation 🔧 Add pyproject.toml 👷 Update makefiles to use black config 📝 Add VS Code + black docs 🔧 Update settings.json for all repos --- .git-blame-ignore-revs | 2 + .pre-commit-config.yaml | 9 +-- Makefile | 23 +++++-- README.md | 2 +- docs/source/coding_guidelines.md | 36 ++++++++++- docs/source/developers.md | 2 +- hi-ml-azure/.flake8 | 3 + hi-ml-azure/.vscode/settings.json | 4 +- hi-ml-azure/Makefile | 6 +- .../testazure/testazure/test_logging.py | 10 +-- hi-ml-cpath/.flake8 | 3 + hi-ml-cpath/.vscode/settings.json | 4 +- hi-ml-cpath/Makefile | 6 +- .../configs/classification/BaseMIL.py | 5 +- .../src/health_cpath/models/encoders.py | 16 +++-- .../src/health_cpath/utils/deepmil_utils.py | 1 - .../testhisto/models/test_transforms.py | 6 +- .../testhisto/preprocessing/test_loading.py | 8 +-- hi-ml-multimodal/.flake8 | 3 + hi-ml-multimodal/.vscode/settings.json | 4 +- hi-ml-multimodal/Makefile | 6 +- hi-ml/.flake8 | 3 + hi-ml/.vscode/settings.json | 4 +- hi-ml/Makefile | 6 +- hi-ml/environment.yml | 61 ++++++++++--------- hi-ml/testhiml/testhiml/test_runner.py | 2 +- hi-ml/testhiml/testhiml/utils/test_logging.py | 11 ++-- new_project_template/.flake8 | 3 + new_project_template/README.md | 3 +- pyproject.toml | 4 ++ test_requirements.txt | 2 +- 31 files changed, 180 insertions(+), 78 deletions(-) create mode 100644 .git-blame-ignore-revs create mode 100644 pyproject.toml diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 00000000..a8d3a8fc --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,2 @@ +# black styling refactor +hashgoeshere diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ca41fc0f..6e0d2c47 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -49,8 +49,9 @@ repos: files: ^hi-ml-multimodal/ args: [--config, hi-ml-multimodal/.flake8] -- repo: https://github.com/pre-commit/mirrors-autopep8 - rev: v2.0.2 +- repo: https://github.com/psf/black + rev: 23.1.0 hooks: - - id: autopep8 - args: [--in-place, --max-line-length, "120"] + - id: black + extend-exclude: ^docs/source/amulet/amulet_script.py + args: [--line-length, "120", --skip-string-normalization] diff --git a/Makefile b/Makefile index a3c7342d..53637689 100644 --- a/Makefile +++ b/Makefile @@ -8,8 +8,15 @@ env: # call make for each sub package define call_packages - cd hi-ml-azure && $(MAKE) $(1) cd hi-ml && $(MAKE) $(1) + cd hi-ml-azure && $(MAKE) $(1) + cd hi-ml-cpath && $(MAKE) $(1) + cd hi-ml-multimodal && $(MAKE) $(1) +endef + +define call_pip_packages + cd hi-ml && $(MAKE) $(1) + cd hi-ml-azure && $(MAKE) $(1) endef ## Package management @@ -28,7 +35,7 @@ pip_test: pip_upgrade # pip install local packages in editable mode for development and testing call_pip_local: - $(call call_packages,call_pip_local) + $(call call_pip_packages,call_pip_local) # pip upgrade and install local packages in editable mode pip_local: pip_upgrade call_pip_local @@ -55,7 +62,7 @@ clean: # build package, assuming build requirements already installed call_build: - $(call call_packages,call_build) + $(call call_pip_packages,call_build) # pip install build requirements and build package build: pip_build call_build @@ -68,6 +75,10 @@ flake8: mypy: $(call call_packages,mypy) +# run black styling, assuming test requirements already installed +black: + $(call call_packages,black) + # run pyright, assuming test requirements already installed call_pyright: npm install -g pyright @@ -77,7 +88,7 @@ call_pyright: pyright: conda call_pyright # run basic checks, assuming test requirements already installed -check: flake8 mypy +check: flake8 mypy black # run pytest on package, assuming test requirements already installed pytest: @@ -85,11 +96,11 @@ pytest: # run pytest fast subset on package, assuming test requirements already installed pytest_fast: - $(call call_packages,pytest_fast) + $(call call_pip_packages,pytest_fast) # run pytest with coverage on package, and format coverage output as a text file, assuming test requirements already installed call_pytest_and_coverage: - $(call call_packages,call_pytest_and_coverage) + $(call call_pip_packages,call_pytest_and_coverage) # install test requirements and run pytest coverage pytest_and_coverage: pip_test call_pytest_and_coverage diff --git a/README.md b/README.md index b53f2163..38eeff85 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # Microsoft Health Intelligence Machine Learning Toolbox -[![Codecov coverage](https://codecov.io/gh/microsoft/hi-ml/branch/main/graph/badge.svg?token=kMr2pSIJ2U)](https://codecov.io/gh/microsoft/hi-ml) +[![Codecov coverage](https://codecov.io/gh/microsoft/hi-ml/branch/main/graph/badge.svg?token=kMr2pSIJ2U)](https://codecov.io/gh/microsoft/hi-ml) [![Code style: black](https://camo.githubusercontent.com/d91ed7ac7abbd5a6102cbe988dd8e9ac21bde0a73d97be7603b891ad08ce3479/68747470733a2f2f696d672e736869656c64732e696f2f62616467652f636f64652532307374796c652d626c61636b2d3030303030302e737667)](https://github.com/psf/black) ## Overview diff --git a/docs/source/coding_guidelines.md b/docs/source/coding_guidelines.md index 093d60b9..1b29a356 100644 --- a/docs/source/coding_guidelines.md +++ b/docs/source/coding_guidelines.md @@ -35,15 +35,47 @@ Please follow the general [PEP 8](https://peps.python.org/pep-0008/) Python rule To improve readability, functions or methods that return boolean values should follow a `is_...`, `has_...`, `use...` pattern, like `is_status_ok` instead of `ok`. -### Static Analysis and Linting +### Static Analysis, Linting and Styling + +We use `flake8` as a linter, `mypy` and pyright for static typechecking, and `black` for styling. All these tools run as part of the PR workflow, and must run without errors for a contribution to be accepted. -We use `flake8` as a linter, and `mypy` and pyright for static typechecking. Both tools run as part of the PR workflow, and must run without errors for a contribution to be accepted. `mypy` requires that all functions and methods carry type annotations. See the [`mypy` documentation](https://mypy.readthedocs.io/en/latest/getting_started.html#function-signatures-and-dynamic-vs-static-typing) for more information. We highly recommend to run all those tools _before_ pushing the latest changes to a PR. If you have `make` installed, you can run both tools in one go via `make check` (from the repository root folder). +### Black styling + +We use `black` for styling all of our code. To pass the tests carried out in `make check` (or `make black`) you can run the following command to reformat your changed file as per our guidelines: + + ```shell + black + ``` + +You can also run `black` on the whole repository by running the following command from the head of the repo: + + ```shell + black . + ``` + +#### Black VS Code Integration + +If you're using VS Code as your IDE, you can avoid having to call `black` every time you want to style some files by installing the [`black` formatter extension](https://marketplace.visualstudio.com/items?itemName=ms-python.black-formatter) and adding the following to your vscode `settings.json` file: + + ```json + "[python]": { + "editor.formatOnSave": true, + "editor.defaultFormatter": "ms-python.black-formatter", + }, + ``` + +Now every time you save a file it will automatically be formatted by `black`. + +#### Black pre-commit hook + +If you do not style your code yourself, then the pre-commit hook on your PR will attempt to do this automatically. Please be aware that this may cause conflicts if you're working on an open pull request. We highly recommend styling your code before submitting! + ### Documentation For general information around docstrings, please check [PEP 257](https://peps.python.org/pep-0257/). diff --git a/docs/source/developers.md b/docs/source/developers.md index b1c30dd2..fb61aaf9 100644 --- a/docs/source/developers.md +++ b/docs/source/developers.md @@ -91,7 +91,7 @@ dependencies: The repository contains a makefile with definitions for common operations. -* `make check`: Run `flake8` and `mypy` on the repository. +* `make check`: Run `flake8`, `mypy` and `black` on the repository. * `make test`: Run `flake8` and `mypy` on the repository, then all tests via `pytest` * `make pip`: Install all packages for running and testing in the current interpreter. * `make conda`: Update the hi-ml Conda environment and activate it diff --git a/hi-ml-azure/.flake8 b/hi-ml-azure/.flake8 index 3a9cc963..acc07d38 100644 --- a/hi-ml-azure/.flake8 +++ b/hi-ml-azure/.flake8 @@ -1,6 +1,9 @@ [flake8] max-line-length = 120 max-complexity = 25 +select = B950 extend-ignore = E731, W503, + E203, + E501, diff --git a/hi-ml-azure/.vscode/settings.json b/hi-ml-azure/.vscode/settings.json index acfaa0f6..67acf59e 100644 --- a/hi-ml-azure/.vscode/settings.json +++ b/hi-ml-azure/.vscode/settings.json @@ -47,7 +47,9 @@ "[python]": { "editor.rulers": [ 120 - ] + ], + "editor.formatOnSave": true, + "editor.defaultFormatter": "ms-python.black-formatter", }, // The settings below are specific to this project, // and may override settings in the workspace configuration. diff --git a/hi-ml-azure/Makefile b/hi-ml-azure/Makefile index 0eae1c94..e4172758 100644 --- a/hi-ml-azure/Makefile +++ b/hi-ml-azure/Makefile @@ -55,8 +55,12 @@ mypy: mypy --install-types --show-error-codes --non-interactive --package health_azure mypy --install-types --show-error-codes --non-interactive --package testazure +# run black check +black: + black . --check --diff + # run basic checks, assuming test requirements already installed -check: flake8 mypy +check: flake8 mypy black # run pytest on package, assuming test requirements already installed pytest: diff --git a/hi-ml-azure/testazure/testazure/test_logging.py b/hi-ml-azure/testazure/testazure/test_logging.py index e2061807..5686c693 100644 --- a/hi-ml-azure/testazure/testazure/test_logging.py +++ b/hi-ml-azure/testazure/testazure/test_logging.py @@ -1,7 +1,7 @@ import logging import os import pytest -from _pytest.capture import SysCapture +from pytest import CaptureFixture, LogCaptureFixture from time import sleep from health_azure.logging import format_time_from_seconds, elapsed_timer, print_message_with_rank_pid, logging_section from health_azure.utils import ENV_LOCAL_RANK @@ -26,7 +26,7 @@ def test_format_time_from_second() -> None: @pytest.mark.parametrize("level", ['DEBUG', 'INFO']) -def test_print_message_with_rank_pid(level: str, capsys: SysCapture) -> None: +def test_print_message_with_rank_pid(level: str, capsys: CaptureFixture) -> None: rank = "0" os.environ[ENV_LOCAL_RANK] = rank message = "test" @@ -39,16 +39,16 @@ def test_print_message_with_rank_pid(level: str, capsys: SysCapture) -> None: @pytest.mark.parametrize("format_seconds", [True, False]) -def test_elapsed_timer(format_seconds: bool, capsys: SysCapture) -> None: +def test_elapsed_timer(format_seconds: bool, capsys: CaptureFixture) -> None: rank = "0" os.environ[ENV_LOCAL_RANK] = rank with elapsed_timer("test", format_seconds): sleep(0.1) # Sleep for 100 ms - stdout: str = capsys.readouterr().out # type: ignore + stdout: str = capsys.readouterr().out assert "0.10 seconds" in stdout -def test_logging_section(caplog: pytest.LogCaptureFixture) -> None: +def test_logging_section(caplog: LogCaptureFixture) -> None: rank = "0" os.environ[ENV_LOCAL_RANK] = rank with logging_section("test"): diff --git a/hi-ml-cpath/.flake8 b/hi-ml-cpath/.flake8 index 3a9cc963..acc07d38 100644 --- a/hi-ml-cpath/.flake8 +++ b/hi-ml-cpath/.flake8 @@ -1,6 +1,9 @@ [flake8] max-line-length = 120 max-complexity = 25 +select = B950 extend-ignore = E731, W503, + E203, + E501, diff --git a/hi-ml-cpath/.vscode/settings.json b/hi-ml-cpath/.vscode/settings.json index 2ce514fb..d619e7d4 100644 --- a/hi-ml-cpath/.vscode/settings.json +++ b/hi-ml-cpath/.vscode/settings.json @@ -47,7 +47,9 @@ "[python]": { "editor.rulers": [ 120 - ] + ], + "editor.formatOnSave": true, + "editor.defaultFormatter": "ms-python.black-formatter", }, // The settings below are specific to this project, // and may override settings in the workspace configuration. diff --git a/hi-ml-cpath/Makefile b/hi-ml-cpath/Makefile index 51b4f05b..0a714374 100644 --- a/hi-ml-cpath/Makefile +++ b/hi-ml-cpath/Makefile @@ -70,8 +70,12 @@ mypy: mypy --install-types --show-error-codes --non-interactive --package testhisto mypy --install-types --show-error-codes --non-interactive --package testSSL +# run black check +black: + black . --check --diff + # run basic checks -check: flake8 mypy +check: flake8 mypy black # run pytest on package, assuming test requirements already installed pytest: diff --git a/hi-ml-cpath/src/health_cpath/configs/classification/BaseMIL.py b/hi-ml-cpath/src/health_cpath/configs/classification/BaseMIL.py index 2349cd69..a1cc34e7 100644 --- a/hi-ml-cpath/src/health_cpath/configs/classification/BaseMIL.py +++ b/hi-ml-cpath/src/health_cpath/configs/classification/BaseMIL.py @@ -208,6 +208,9 @@ class BaseMIL(LightningContainer, LoadingParams, EncoderParams, PoolingParams, C random see. See `SlidesDataModule` for an example how to achieve that.""" return None + def get_encoder_params(self) -> EncoderParams: + return create_from_matching_params(self, EncoderParams) + def create_model(self) -> DeepMILModule: self.data_module = self.get_data_module() outputs_handler = self.get_outputs_handler() @@ -216,7 +219,7 @@ class BaseMIL(LightningContainer, LoadingParams, EncoderParams, PoolingParams, C class_names=self.class_names, class_weights=self.data_module.class_weights, outputs_folder=self.outputs_folder, - encoder_params=create_from_matching_params(self, EncoderParams), + encoder_params=self.get_encoder_params(), pooling_params=create_from_matching_params(self, PoolingParams), classifier_params=create_from_matching_params(self, ClassifierParams), optimizer_params=create_from_matching_params(self, OptimizerParams), diff --git a/hi-ml-cpath/src/health_cpath/models/encoders.py b/hi-ml-cpath/src/health_cpath/models/encoders.py index 273c2565..b3fec8d5 100644 --- a/hi-ml-cpath/src/health_cpath/models/encoders.py +++ b/hi-ml-cpath/src/health_cpath/models/encoders.py @@ -327,7 +327,7 @@ class SwinTransformerCheckpointingMixin: def __init__( self, - feature_extractor_fn: ResNet, + feature_extractor_fn: SwinTransformer, checkpoint_segments_size: int = 2, ) -> None: """ @@ -338,12 +338,20 @@ class SwinTransformerCheckpointingMixin: self.feature_extractor_fn = feature_extractor_fn self.checkpoint_segments_size = checkpoint_segments_size - def custom_forward(self, images: torch.Tensor) -> torch.Tensor: - """Custom forward pass that uses activation checkpointing to save memory.""" - # patch embedding checkpointing + def custom_patch_embedding_forward(self, images: torch.Tensor) -> torch.Tensor: + """Custom patch partitioning checkpointing""" + _, _, H, W = images.shape + img_size = self.feature_extractor_fn.patch_embed.img_size + assert H == img_size[0] and W == img_size[1], \ + f"Input image size ({H}*{W}) doesn't match model ({img_size[0]}*{img_size[1]})." images = checkpoint(self.feature_extractor_fn.patch_embed.proj, images) images = images.flatten(2).transpose(1, 2) # BCHW -> BNC images = checkpoint(self.feature_extractor_fn.patch_embed.norm, images) + return images + + def custom_forward(self, images: torch.Tensor) -> torch.Tensor: + """Custom forward pass that uses activation checkpointing to save memory.""" + images = self.custom_patch_embedding_forward(images) # do not checkpoint dropout images = self.feature_extractor_fn.pos_drop(images) # sequential layers checkpointing diff --git a/hi-ml-cpath/src/health_cpath/utils/deepmil_utils.py b/hi-ml-cpath/src/health_cpath/utils/deepmil_utils.py index 70d72b10..00076391 100644 --- a/hi-ml-cpath/src/health_cpath/utils/deepmil_utils.py +++ b/hi-ml-cpath/src/health_cpath/utils/deepmil_utils.py @@ -94,7 +94,6 @@ class EncoderParams(param.Parameterized): """Given the current encoder parameters, returns the encoder object. :param outputs_folder: The output folder where SSL checkpoint should be saved. - :param encoder_params: The encoder arguments that define the encoder class object depending on the encoder type. :raises ValueError: If the encoder type is not supported. :return: A TileEncoder instance for deepmil module. """ diff --git a/hi-ml-cpath/testhisto/testhisto/models/test_transforms.py b/hi-ml-cpath/testhisto/testhisto/models/test_transforms.py index 91270905..e2b7b9c6 100644 --- a/hi-ml-cpath/testhisto/testhisto/models/test_transforms.py +++ b/hi-ml-cpath/testhisto/testhisto/models/test_transforms.py @@ -8,7 +8,7 @@ from pathlib import Path import time from typing import Any, Callable, Dict, Sequence, Union import numpy as np -from _pytest.capture import SysCapture +from pytest import CaptureFixture import pytest import torch from monai.data.meta_tensor import MetaTensor @@ -376,7 +376,7 @@ def test_metatensor_to_tensor_d_transform() -> None: _ = transform(new_sample) -def test_timer_wrapper_transform(capsys: SysCapture) -> None: +def test_timer_wrapper_transform(capsys: CaptureFixture) -> None: sample = {"a": 1, SlideKey.SLIDE_ID: "0"} class DummyTransform: @@ -387,7 +387,7 @@ def test_timer_wrapper_transform(capsys: SysCapture) -> None: transform = TimerWrapper(DummyTransform()) out = transform(sample) assert out == sample - message = capsys.readouterr().out # type: ignore + message = capsys.readouterr().out assert "Rank " in message assert "DummyTransform, Slide 0 took 0.10 seconds" in message diff --git a/hi-ml-cpath/testhisto/testhisto/preprocessing/test_loading.py b/hi-ml-cpath/testhisto/testhisto/preprocessing/test_loading.py index c2d5d54f..2e48756d 100644 --- a/hi-ml-cpath/testhisto/testhisto/preprocessing/test_loading.py +++ b/hi-ml-cpath/testhisto/testhisto/preprocessing/test_loading.py @@ -7,11 +7,12 @@ import shutil from unittest.mock import MagicMock, patch import numpy as np import pytest -from _pytest.capture import SysCapture +from pytest import CaptureFixture from pathlib import Path from typing import List, Optional, Tuple from monai.transforms import LoadImaged from monai.data.wsi_reader import CuCIMWSIReader, OpenSlideWSIReader, WSIReader +from PIL import Image from health_cpath.datasets.default_paths import PANDA_DATASET_ID from health_cpath.datasets.panda_dataset import PandaDataset from health_cpath.preprocessing.loading import ( @@ -20,7 +21,6 @@ from health_cpath.preprocessing.loading import ( from health_cpath.scripts.mount_azure_dataset import mount_dataset from health_cpath.utils.naming import SlideKey from health_ml.utils.common_utils import is_gpu_available -from PIL import Image from testhiml.utils_testhiml import DEFAULT_WORKSPACE @@ -86,7 +86,7 @@ def test_load_slide(tmp_path: Path) -> None: @pytest.mark.skipif(no_gpu, reason="Test requires GPU") @pytest.mark.gpu def test_failed_to_estimate_foreground( - roi_type: ROIType, mock_panda_slides_root_dir: Path, capsys: SysCapture + roi_type: ROIType, mock_panda_slides_root_dir: Path, capsys: CaptureFixture ) -> None: loading_params = LoadingParams(roi_type=roi_type, level=2) load_transform: BaseLoadROId = loading_params.get_load_roid_transform() # type: ignore @@ -99,7 +99,7 @@ def test_failed_to_estimate_foreground( with patch.object(load_transform.reader, "get_data", return_value=(MagicMock(), MagicMock())): _ = load_transform(sample) mock_get_wsi_bbox.assert_called_once() - stdout: str = capsys.readouterr().out # type: ignore + stdout: str = capsys.readouterr().out assert "Failed to estimate bounding box for slide _0: The input mask is empty" in stdout diff --git a/hi-ml-multimodal/.flake8 b/hi-ml-multimodal/.flake8 index 3a9cc963..acc07d38 100644 --- a/hi-ml-multimodal/.flake8 +++ b/hi-ml-multimodal/.flake8 @@ -1,6 +1,9 @@ [flake8] max-line-length = 120 max-complexity = 25 +select = B950 extend-ignore = E731, W503, + E203, + E501, diff --git a/hi-ml-multimodal/.vscode/settings.json b/hi-ml-multimodal/.vscode/settings.json index 3d59b5f0..3a93ea6b 100644 --- a/hi-ml-multimodal/.vscode/settings.json +++ b/hi-ml-multimodal/.vscode/settings.json @@ -47,7 +47,9 @@ "[python]": { "editor.rulers": [ 120 - ] + ], + "editor.formatOnSave": true, + "editor.defaultFormatter": "ms-python.black-formatter", }, // The settings below are specific to this project, // and may override settings in the workspace configuration. diff --git a/hi-ml-multimodal/Makefile b/hi-ml-multimodal/Makefile index 1635442f..9aaa26a2 100644 --- a/hi-ml-multimodal/Makefile +++ b/hi-ml-multimodal/Makefile @@ -46,6 +46,10 @@ build: flake8: flake8 --count --statistics . +# run black check +black: + black . --check --diff + # run mypy, assuming test requirements already installed mypy: mypy --install-types --show-error-codes --non-interactive setup.py @@ -75,7 +79,7 @@ pyright: pyright # run basic checks -check: flake8 mypy pyright +check: flake8 mypy pyright black # bump version from specified part (major, minor or patch) (dry run). Usage: # $ make bump_version_dry part=patch diff --git a/hi-ml/.flake8 b/hi-ml/.flake8 index 3a9cc963..acc07d38 100644 --- a/hi-ml/.flake8 +++ b/hi-ml/.flake8 @@ -1,6 +1,9 @@ [flake8] max-line-length = 120 max-complexity = 25 +select = B950 extend-ignore = E731, W503, + E203, + E501, diff --git a/hi-ml/.vscode/settings.json b/hi-ml/.vscode/settings.json index c5fa888b..49dd5bd1 100644 --- a/hi-ml/.vscode/settings.json +++ b/hi-ml/.vscode/settings.json @@ -47,7 +47,9 @@ "[python]": { "editor.rulers": [ 120 - ] + ], + "editor.formatOnSave": true, + "editor.defaultFormatter": "ms-python.black-formatter", }, // The settings below are specific to this project, // and may override settings in the workspace configuration. diff --git a/hi-ml/Makefile b/hi-ml/Makefile index 8a461c96..eb1ca58b 100644 --- a/hi-ml/Makefile +++ b/hi-ml/Makefile @@ -55,8 +55,12 @@ mypy: mypy --install-types --show-error-codes --non-interactive --package health_ml mypy --install-types --show-error-codes --non-interactive --package testhiml +# run black check +black: + black . --check --diff + # run basic checks, assuming test requirements already installed -check: flake8 mypy +check: flake8 mypy black # run pytest on package, assuming test requirements already installed pytest: diff --git a/hi-ml/environment.yml b/hi-ml/environment.yml index 031791d4..56f9533e 100644 --- a/hi-ml/environment.yml +++ b/hi-ml/environment.yml @@ -28,9 +28,9 @@ dependencies: - libsqlite=3.40.0=h753d276_0 - libstdcxx-ng=12.2.0=h46fd767_19 - libuv=1.44.2=h166bdaf_0 - - libxml2=2.10.3=h7463322_0 + - libxml2=2.10.3=hca2bb57_3 - libzlib=1.2.13=h166bdaf_4 - - llvm-openmp=15.0.7=h0cdce71_0 + - llvm-openmp=16.0.0=h417c0b6_0 - mkl=2022.1.0=h84fe81f_915 - mkl-devel=2022.1.0=ha770c72_916 - mkl-include=2022.1.0=h84fe81f_915 @@ -40,22 +40,24 @@ dependencies: - python=3.7.3 - pytorch-mutex=1.0=cuda - readline=8.1.2=h0f457ee_0 - - setuptools=67.4.0=pyhd8ed1ab_0 + - setuptools=67.6.0=pyhd8ed1ab_0 - sqlite=3.40.0=h4ff8645_0 - tbb=2021.8.0=hf52228f_0 - tk=8.6.12=h27826a3_0 + - typing_extensions=4.5.0=pyha770c72_0 - xz=5.2.6=h166bdaf_0 - zlib=1.2.13=h166bdaf_4 + - zstd=1.5.2=h3eb15da_6 - pip: - absl-py==1.4.0 - adal==1.2.7 - aiohttp==3.8.4 - aiosignal==1.3.1 - alabaster==0.7.13 - - alembic==1.9.4 + - alembic==1.10.2 - applicationinsights==0.11.10 - - argcomplete==2.0.0 - - astroid==2.14.2 + - argcomplete==2.1.2 + - astroid==2.15.0 - async-timeout==4.0.2 - asynctest==0.13.0 - attrs==22.2.0 @@ -67,21 +69,21 @@ dependencies: - azure-mgmt-authorization==3.0.0 - azure-mgmt-containerregistry==10.1.0 - azure-mgmt-core==1.3.2 - - azure-mgmt-keyvault==10.1.0 + - azure-mgmt-keyvault==10.2.0 - azure-mgmt-resource==21.2.1 - azure-mgmt-storage==20.1.0 - azure-storage-blob==12.10.0 - - azure-storage-file-datalake==12.10.0 - - azure-storage-file-share==12.11.0 + - azure-storage-file-datalake==12.10.1 + - azure-storage-file-share==12.11.1 - azureml-core==1.49.0 - - azureml-dataprep==4.9.2 + - azureml-dataprep==4.9.5 - azureml-dataprep-native==38.0.0 - - azureml-dataprep-rslex==2.16.3 + - azureml-dataprep-rslex==2.16.4 - azureml-dataset-runtime==1.49.0 - azureml-mlflow==1.49.0 - azureml-telemetry==1.49.0 - azureml-tensorboard==1.49.0 - - azureml-train-core==1.49.0 + - azureml-train-core==1.49.0.post1 - azureml-train-restclients-hyperdrive==1.49.0 - babel==2.12.1 - backports-tempfile==1.0 @@ -93,16 +95,16 @@ dependencies: - certifi==2022.12.7 - cffi==1.15.1 - cfgv==3.3.1 - - charset-normalizer==3.0.1 + - charset-normalizer==3.1.0 - click==8.1.3 - cloudpickle==2.2.1 - colorama==0.4.6 - conda-merge==0.2.0 - contextlib2==21.6.0 - coverage==7.1.0 - - cryptography==39.0.1 + - cryptography==39.0.2 - cycler==0.11.0 - - databricks-cli==0.17.4 + - databricks-cli==0.17.5 - dataclasses-json==0.5.2 - dill==0.3.6 - distlib==0.3.6 @@ -111,9 +113,10 @@ dependencies: - docutils==0.16 - dotnetcore2==3.1.23 - entrypoints==0.4 - - exceptiongroup==1.1.0 - - filelock==3.9.0 + - exceptiongroup==1.1.1 + - filelock==3.10.0 - flake8==5.0.4 + - flake8-bugbear==23.3.12 - flask==2.2.3 - fonttools==4.38.0 - frozenlist==1.3.3 @@ -121,14 +124,13 @@ dependencies: - fusepy==3.0.1 - gitdb==4.0.10 - gitpython==3.1.31 - - google-auth==2.16.1 + - google-auth==2.16.2 - google-auth-oauthlib==0.4.6 - greenlet==2.0.2 - grpcio==1.51.3 - gunicorn==20.1.0 - - hi-ml-azure==0.2.18 - humanfriendly==10.0 - - identify==2.5.18 + - identify==2.5.21 - idna==3.4 - imagesize==1.4.1 - importlib-metadata==5.2.0 @@ -180,13 +182,13 @@ dependencies: - opencv-python-headless==4.7.0.72 - packaging==21.3 - pandas==1.3.5 - - param==1.12.3 + - param==1.13.0 - paramiko==2.12.0 - - pathspec==0.11.0 + - pathspec==0.11.1 - pillow==9.3.0 - pkginfo==1.9.6 - pkgutil-resolve-name==1.3.10 - - platformdirs==3.0.0 + - platformdirs==3.1.1 - pluggy==1.0.0 - portalocker==2.7.0 - pre-commit==2.21.0 @@ -245,9 +247,9 @@ dependencies: - sphinxcontrib-jsmath==1.0.1 - sphinxcontrib-qthelp==1.0.3 - sphinxcontrib-serializinghtml==1.1.5 - - sqlalchemy==1.4.46 + - sqlalchemy==1.4.47 - sqlparse==0.4.3 - - strictyaml==1.6.2 + - strictyaml==1.7.3 - stringcase==1.2.0 - tabulate==0.9.0 - tensorboard==2.11.2 @@ -257,15 +259,14 @@ dependencies: - tomli==2.0.1 - tomlkit==0.11.6 - torch==1.13.1 - - torchmetrics==0.11.3 + - torchmetrics==0.11.4 - torchvision==0.14.1 - - tqdm==4.64.1 + - tqdm==4.65.0 - twine==3.3.0 - typed-ast==1.5.4 - - typing-extensions==4.5.0 - typing-inspect==0.8.0 - - urllib3==1.26.14 - - virtualenv==20.20.0 + - urllib3==1.26.15 + - virtualenv==20.21.0 - webencodings==0.5.1 - websocket-client==1.5.1 - werkzeug==2.2.3 diff --git a/hi-ml/testhiml/testhiml/test_runner.py b/hi-ml/testhiml/testhiml/test_runner.py index d5e54850..420e07b4 100644 --- a/hi-ml/testhiml/testhiml/test_runner.py +++ b/hi-ml/testhiml/testhiml/test_runner.py @@ -380,7 +380,7 @@ def test_runner_help(mock_runner: Runner, capsys: CaptureFixture) -> None: with pytest.raises(SystemExit): with patch.object(sys, "argv", arguments): mock_runner.run() - stdout: str = capsys.readouterr().out # type: ignore + stdout: str = capsys.readouterr().out # There are at least 3 parameters in ExperimentConfig that should print with defaults assert stdout.count("(default: ") > 3 diff --git a/hi-ml/testhiml/testhiml/utils/test_logging.py b/hi-ml/testhiml/testhiml/utils/test_logging.py index 41c0bba0..318504bf 100644 --- a/hi-ml/testhiml/testhiml/utils/test_logging.py +++ b/hi-ml/testhiml/testhiml/utils/test_logging.py @@ -14,7 +14,7 @@ from unittest.mock import MagicMock, PropertyMock, patch import pytest import torch -from _pytest.capture import SysCapture +from pytest import CaptureFixture from _pytest.logging import LogCaptureFixture from azureml._restclient.constants import RunStatus from azureml.core import Run @@ -372,7 +372,7 @@ def test_progress_bar_enable() -> None: assert bar.is_enabled -def test_progress_bar(capsys: SysCapture) -> None: +def test_progress_bar(capsys: CaptureFixture) -> None: bar = AzureMLProgressBar(refresh_rate=1) mock_module = mock.MagicMock(global_step=34) mock_trainer = mock.MagicMock(current_epoch=12, @@ -386,7 +386,8 @@ def test_progress_bar(capsys: SysCapture) -> None: assert bar.trainer == mock_trainer def latest_message() -> str: - return capsys.readouterr().out.splitlines()[-1] # type: ignore + out: str = capsys.readouterr().out + return out.splitlines()[-1] # Messages in training bar.on_train_epoch_start(mock_trainer, None) # type: ignore @@ -449,7 +450,7 @@ def test_progress_bar_to_logging(caplog: LogCaptureFixture) -> None: @pytest.mark.parametrize("print_timestamp", [True, False]) -def test_progress_bar_to_stdout(capsys: SysCapture, print_timestamp: bool) -> None: +def test_progress_bar_to_stdout(capsys: CaptureFixture, print_timestamp: bool) -> None: """ Check that the progress bar correctly writes to stdout, and that timestamps are generated if requested. """ @@ -457,7 +458,7 @@ def test_progress_bar_to_stdout(capsys: SysCapture, print_timestamp: bool) -> No today = datetime.utcnow().strftime("%Y-%m-%d") to_stdout = AzureMLProgressBar(write_to_logging_info=False, print_timestamp=print_timestamp) to_stdout._print(message) - stdout: str = capsys.readouterr().out # type: ignore + stdout: str = capsys.readouterr().out print(f"Output: {stdout}") assert message in stdout assert stdout.startswith(today) == print_timestamp diff --git a/new_project_template/.flake8 b/new_project_template/.flake8 index 3a9cc963..acc07d38 100644 --- a/new_project_template/.flake8 +++ b/new_project_template/.flake8 @@ -1,6 +1,9 @@ [flake8] max-line-length = 120 max-complexity = 25 +select = B950 extend-ignore = E731, W503, + E203, + E501, diff --git a/new_project_template/README.md b/new_project_template/README.md index 084a0671..d9851feb 100644 --- a/new_project_template/README.md +++ b/new_project_template/README.md @@ -24,5 +24,6 @@ Make file commands: * `make pip_local` to install the package in editable mode. This must happen before running tests. * `make build` to build the package * `make mypy` to run `mypy` -* `make check` to run `flake8`, `mypy` and `pyright` +* `make black` to run `black` checks (not reformatting) +* `make check` to run `flake8`, `mypy`, `pyright` and `black` * `make clean` to clean up all temporary files and folders diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 00000000..2009d5ff --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,4 @@ +[tool.black] +line-length = 120 +skip-string-normalization = true +extend-exclude = "docs/source/amulet/amulet_script.py" diff --git a/test_requirements.txt b/test_requirements.txt index f16d3a07..b5c367f7 100644 --- a/test_requirements.txt +++ b/test_requirements.txt @@ -1,6 +1,6 @@ black==23.1.0 coverage==7.1.0 -flake8==5.0.4 +flake8-bugbear==23.3.12 mypy==1.0.1 pre-commit==2.21.0 pylint==2.16.2