From 259d0387671a9ea7efcb9e6874f13515cc18241d Mon Sep 17 00:00:00 2001 From: Kelly <40868256+lykelly19@users.noreply.github.com> Date: Wed, 4 Sep 2024 15:17:08 -0400 Subject: [PATCH] Run scripts-syntax on all files and update issues to unblock gate (#3326) * Move path filtering to unblock status check requirements * Update condition * Update variable * test using 2 jobs * update types * revert some changes * duplicated gate for testing * [testing] variables update * [testing] add python script for testing * [testing] update job dependency * [testing] use new output var * [testing] revert small change * [testing] echo statement * [testing] use env var and update test py script * [testing] update condition * [testing] remove if condition * [testing] update if for job 2 and more testing * [testing] update condition * [testing] update again * [testing] var update * [testing] run scripts-syntax on all files * Update files based on scripts-syntax * Update spacing * [scripts-syntax] updates 2 * [scripts-syntax] update copyright * [scripts-syntax] update spacing * [scripts-syntax] add new line * [scripts-syntax] more updates * [scripts-syntax] update again * [scripts-syntax] update * Remove testing gate and update scripts-syntax to run on all files * remove if condition * remove test file --------- Co-authored-by: lykelly19 --- .github/workflows/scripts-syntax.yaml | 17 ++--------------- .../environments/lightgbm-3.3/tests/src/main.py | 8 ++++++++ .../vision/components/src/common/__init__.py | 4 ++++ .../vision/components/src/common/settings.py | 13 +++++++++++++ .../vision/components/src/common/utils.py | 7 +++++++ .../src/image_classification/__init__.py | 3 +++ .../components/src/image_classification/run.py | 5 +++++ .../src/instance_segmentation/__init__.py | 3 +++ .../components/src/instance_segmentation/run.py | 5 +++++ .../components/src/object_detection/__init__.py | 3 +++ .../components/src/object_detection/run.py | 5 +++++ .../prepare_data.py | 5 +++++ .../src/predict.py | 4 ++++ scripts/azureml-assets/azureml/__init__.py | 2 ++ .../azureml/assets/environment/pin_versions.py | 4 ++++ .../azureml/assets/util/template.py | 3 +++ test/conftest.py | 5 +++++ .../test-asset-1/tests/test_component.py | 4 ++++ .../test-asset-1/tests/test_component.py | 4 ++++ .../test-asset-1/tests/test_component.py | 4 ++++ .../test-asset-2/tests/test_component.py | 4 ++++ test/resources/release/src/code/run.py | 8 +++++--- test/test_build_environments.py | 3 +++ test/test_release_assets.py | 4 ++++ 24 files changed, 109 insertions(+), 18 deletions(-) diff --git a/.github/workflows/scripts-syntax.yaml b/.github/workflows/scripts-syntax.yaml index 0003ff3313..5dcf9e85db 100644 --- a/.github/workflows/scripts-syntax.yaml +++ b/.github/workflows/scripts-syntax.yaml @@ -4,8 +4,6 @@ on: pull_request: branches: - main - paths: - - "**.py" workflow_dispatch: env: @@ -30,16 +28,7 @@ jobs: uses: actions/checkout@v3 with: fetch-depth: 2 - - - name: Get changed Python scripts - id: changed-scripts - uses: tj-actions/changed-files@v35 - with: - files_separator: ',' - separator: ',' - files: | - **/*.py - + - name: Use Python 3.10 or newer uses: actions/setup-python@v4 with: @@ -54,7 +43,5 @@ jobs: - name: Check code health run: python $scripts_validation_dir/code_health.py -i . - # TODO: Remove "if" property, -c argument, and changed-scripts step above - name: Check code documentation - if: steps.changed-scripts.outputs.all_modified_files != '' - run: python $scripts_validation_dir/doc_style.py -i . -c "${{ steps.changed-scripts.outputs.all_modified_files }}" + run: python $scripts_validation_dir/doc_style.py -i . \ No newline at end of file diff --git a/assets/training/general/environments/lightgbm-3.3/tests/src/main.py b/assets/training/general/environments/lightgbm-3.3/tests/src/main.py index 197229886d..f97c95793b 100644 --- a/assets/training/general/environments/lightgbm-3.3/tests/src/main.py +++ b/assets/training/general/environments/lightgbm-3.3/tests/src/main.py @@ -1,5 +1,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. + +"""Functions for lightgbm 3.3 environment.""" + import mlflow import argparse @@ -12,6 +15,7 @@ # define functions def main(args): + """Read, process, and train model.""" # enable auto logging mlflow.autolog() @@ -41,6 +45,7 @@ def main(args): def process_data(df): + """Process data.""" # split dataframe into X and y X = df.drop(["species"], axis=1) y = df["species"] @@ -59,6 +64,7 @@ def process_data(df): def train_model(params, num_boost_round, X_train, X_test, y_train, y_test): + """Train model.""" # create lightgbm datasets train_data = lgbm.Dataset(X_train, label=y_train) test_data = lgbm.Dataset(X_test, label=y_test) @@ -77,6 +83,7 @@ def train_model(params, num_boost_round, X_train, X_test, y_train, y_test): def parse_args(): + """Parse arguments.""" # setup arg parser parser = argparse.ArgumentParser() @@ -101,6 +108,7 @@ def parse_args(): # run script if __name__ == "__main__": + """Parse arguments and run main function.""" # parse args args = parse_args() diff --git a/assets/training/vision/components/src/common/__init__.py b/assets/training/vision/components/src/common/__init__.py index e69de29bb2..09c56e0e3f 100644 --- a/assets/training/vision/components/src/common/__init__.py +++ b/assets/training/vision/components/src/common/__init__.py @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""Init.""" diff --git a/assets/training/vision/components/src/common/settings.py b/assets/training/vision/components/src/common/settings.py index eaab9b3d36..b3369a5c05 100644 --- a/assets/training/vision/components/src/common/settings.py +++ b/assets/training/vision/components/src/common/settings.py @@ -1,15 +1,20 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Classes for ML tasks.""" + + import argparse from azureml.automl.core.shared.constants import Tasks from azureml.automl.dnn.vision.common import utils class CommonSettings: + """CommonSettings class.""" def __init__(self, training_data: str, validation_data: str, mlflow_model_output: str, pytorch_model_output: str) -> None: + """Init function for CommonSettings class.""" self.training_data = training_data self.validation_data = validation_data self.mlflow_model_output = mlflow_model_output @@ -17,6 +22,7 @@ def __init__(self, training_data: str, validation_data: str, mlflow_model_output @classmethod def create_from_parsing_current_cmd_line_args(cls) -> "CommonSettings": + """Create object from parsing current cmd line args.""" parser = argparse.ArgumentParser() parser.add_argument(utils._make_arg('training_data'), type=str) parser.add_argument(utils._make_arg('validation_data'), type=str) @@ -29,9 +35,11 @@ def create_from_parsing_current_cmd_line_args(cls) -> "CommonSettings": class ClassificationSettings(CommonSettings): + """ClassificationSettings class.""" def __init__(self, training_data: str, validation_data: str, mlflow_model_output: str, pytorch_model_output: str, task_type: str) -> None: + """Init function for ClassificationSettings class.""" super().__init__(training_data, validation_data, mlflow_model_output, pytorch_model_output) self.multilabel = False if task_type == Tasks.IMAGE_CLASSIFICATION_MULTILABEL: @@ -39,6 +47,7 @@ def __init__(self, training_data: str, validation_data: str, mlflow_model_output @classmethod def create_from_parsing_current_cmd_line_args(cls) -> "ClassificationSettings": + """Create object from parsing current cmd line args.""" # Create common settings common_settings = CommonSettings.create_from_parsing_current_cmd_line_args() @@ -52,8 +61,12 @@ def create_from_parsing_current_cmd_line_args(cls) -> "ClassificationSettings": class ObjectDetectionSettings(CommonSettings): + """ObjectDetectionSettings class.""" + pass class InstanceSegmentationSettings(CommonSettings): + """InstanceSegmentationSettings class.""" + pass diff --git a/assets/training/vision/components/src/common/utils.py b/assets/training/vision/components/src/common/utils.py index 32a79ab6aa..286898f73c 100644 --- a/assets/training/vision/components/src/common/utils.py +++ b/assets/training/vision/components/src/common/utils.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Util functions.""" + + import json import os import shutil @@ -50,6 +53,7 @@ def wrapper(*args, **kwargs): def create_mltable_json(settings: CommonSettings) -> str: + """Create MLTable in JSON.""" mltable_data_dict = { MLTableDataLabel.TrainData.value: { MLTableLiterals.MLTABLE_RESOLVEDURI: settings.training_data @@ -65,16 +69,19 @@ def create_mltable_json(settings: CommonSettings) -> str: def get_local_rank() -> int: + """Get local rank.""" return int(os.environ["LOCAL_RANK"]) def validate_running_on_gpu_compute() -> None: + """Check if GPU compute is available.""" if not torch.cuda.is_available() or torch.cuda.device_count() == 0: raise AutoMLVisionValidationException( "This component requires compute that contains one or more GPU.") def download_models(run: Run, mlflow_output: str, pytorch_output: str): + """Download models.""" TMP_OUTPUT = '/tmp/outputs' TMP_MLFLOW = TMP_OUTPUT + '/mlflow-model' diff --git a/assets/training/vision/components/src/image_classification/__init__.py b/assets/training/vision/components/src/image_classification/__init__.py index 89cede260a..187d04a563 100644 --- a/assets/training/vision/components/src/image_classification/__init__.py +++ b/assets/training/vision/components/src/image_classification/__init__.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Init.""" + + import sys diff --git a/assets/training/vision/components/src/image_classification/run.py b/assets/training/vision/components/src/image_classification/run.py index 5e8f149e78..163d7598f3 100644 --- a/assets/training/vision/components/src/image_classification/run.py +++ b/assets/training/vision/components/src/image_classification/run.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Run an image classification task with specified component settings.""" + + from azureml.automl.core.shared.constants import Tasks from azureml.automl.dnn.vision.common.constants import SettingsLiterals from azureml.automl.dnn.vision.classification import runner @@ -11,6 +14,7 @@ def run(task, component_settings): + """Run task with given component settings.""" @utils.create_component_telemetry_wrapper(task) def run_component(task): mltable_data_json = utils.create_mltable_json(component_settings) @@ -24,6 +28,7 @@ def run_component(task): if __name__ == "__main__": + """Check GPU compute and run component.""" utils.validate_running_on_gpu_compute() # Run the component. diff --git a/assets/training/vision/components/src/instance_segmentation/__init__.py b/assets/training/vision/components/src/instance_segmentation/__init__.py index 89cede260a..187d04a563 100644 --- a/assets/training/vision/components/src/instance_segmentation/__init__.py +++ b/assets/training/vision/components/src/instance_segmentation/__init__.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Init.""" + + import sys diff --git a/assets/training/vision/components/src/instance_segmentation/run.py b/assets/training/vision/components/src/instance_segmentation/run.py index d4ecab286a..b89007a011 100644 --- a/assets/training/vision/components/src/instance_segmentation/run.py +++ b/assets/training/vision/components/src/instance_segmentation/run.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Run instance segmentation task.""" + + from azureml.automl.core.shared.constants import Tasks from azureml.automl.dnn.vision.common.constants import SettingsLiterals from azureml.automl.dnn.vision.object_detection import runner @@ -12,6 +15,7 @@ @utils.create_component_telemetry_wrapper(Tasks.IMAGE_INSTANCE_SEGMENTATION) def run(): + """Run the instance segmentation task.""" component_settings = InstanceSegmentationSettings.create_from_parsing_current_cmd_line_args() mltable_data_json = utils.create_mltable_json(component_settings) runner.run( @@ -22,6 +26,7 @@ def run(): if __name__ == "__main__": + """Check GPU compute and run component.""" utils.validate_running_on_gpu_compute() # Run the component. diff --git a/assets/training/vision/components/src/object_detection/__init__.py b/assets/training/vision/components/src/object_detection/__init__.py index 89cede260a..187d04a563 100644 --- a/assets/training/vision/components/src/object_detection/__init__.py +++ b/assets/training/vision/components/src/object_detection/__init__.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Init.""" + + import sys diff --git a/assets/training/vision/components/src/object_detection/run.py b/assets/training/vision/components/src/object_detection/run.py index 01531f852b..c7fc208fc0 100644 --- a/assets/training/vision/components/src/object_detection/run.py +++ b/assets/training/vision/components/src/object_detection/run.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Run object detection task.""" + + from azureml.automl.core.shared.constants import Tasks from azureml.automl.dnn.vision.common.constants import SettingsLiterals from azureml.automl.dnn.vision.object_detection import runner @@ -12,6 +15,7 @@ @utils.create_component_telemetry_wrapper(Tasks.IMAGE_OBJECT_DETECTION) def run(): + """Run object detection task.""" component_settings = ObjectDetectionSettings.create_from_parsing_current_cmd_line_args() mltable_data_json = utils.create_mltable_json(component_settings) runner.run( @@ -22,6 +26,7 @@ def run(): if __name__ == "__main__": + """Check GPU compute and run component.""" utils.validate_running_on_gpu_compute() # Run the component. diff --git a/assets/training/vision/jobs/object-detection-using-built-in-component/prepare_data.py b/assets/training/vision/jobs/object-detection-using-built-in-component/prepare_data.py index 371554a667..0847cfd2fe 100644 --- a/assets/training/vision/jobs/object-detection-using-built-in-component/prepare_data.py +++ b/assets/training/vision/jobs/object-detection-using-built-in-component/prepare_data.py @@ -1,6 +1,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Prepare data.""" + import argparse import json import os @@ -19,6 +21,7 @@ def create_jsonl_files(uri_folder_data_path): + """Create jsonl files.""" print("Creating jsonl files") src_images = os.path.join(data_dir, "odFridgeObjects") @@ -102,6 +105,7 @@ def create_jsonl_files(uri_folder_data_path): def upload_data_and_create_jsonl_files(ml_client): + """Upload data and create jsonl files.""" # Download data from public url # download data @@ -137,6 +141,7 @@ def upload_data_and_create_jsonl_files(ml_client): if __name__ == "__main__": + """Prepare data for image classification.""" parser = argparse.ArgumentParser( description="Prepare data for image classification" ) diff --git a/assets/training/vision/jobs/object-detection-using-built-in-component/src/predict.py b/assets/training/vision/jobs/object-detection-using-built-in-component/src/predict.py index 6a7929c0cb..bf6e4ed081 100644 --- a/assets/training/vision/jobs/object-detection-using-built-in-component/src/predict.py +++ b/assets/training/vision/jobs/object-detection-using-built-in-component/src/predict.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Download, extract, and score images using trained MLflow model.""" + + import argparse import base64 import os @@ -38,6 +41,7 @@ # Define helper method to read the bytes of a file from disk def read_file_bytes(image_path): + """Define helper method to read the bytes of a file from disk.""" with open(image_path, "rb") as f: return f.read() diff --git a/scripts/azureml-assets/azureml/__init__.py b/scripts/azureml-assets/azureml/__init__.py index 1d8b476658..4d5a46e045 100644 --- a/scripts/azureml-assets/azureml/__init__.py +++ b/scripts/azureml-assets/azureml/__init__.py @@ -1,2 +1,4 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. + +"""Init.""" diff --git a/scripts/azureml-assets/azureml/assets/environment/pin_versions.py b/scripts/azureml-assets/azureml/assets/environment/pin_versions.py index 5a1051fc15..aeb99114a2 100644 --- a/scripts/azureml-assets/azureml/assets/environment/pin_versions.py +++ b/scripts/azureml-assets/azureml/assets/environment/pin_versions.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Pin images and packages and write results.""" + + import argparse from pathlib import Path @@ -9,6 +12,7 @@ def transform_file(input_file: Path, output_file: Path = None): + """Transform file.""" # Read file with open(input_file) as f: contents = f.read() diff --git a/scripts/azureml-assets/azureml/assets/util/template.py b/scripts/azureml-assets/azureml/assets/util/template.py index fa214b41db..6e57ceef31 100644 --- a/scripts/azureml-assets/azureml/assets/util/template.py +++ b/scripts/azureml-assets/azureml/assets/util/template.py @@ -1,6 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Functions for replacing template tags.""" + + TAG_PREFIX = "{{" TAG_SUFFIX = "}}" TAG_SEPARATOR = "." diff --git a/test/conftest.py b/test/conftest.py index 4cceed49ef..ea94fef636 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -1,12 +1,17 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test various pytest cases with .""" + + def pytest_addoption(parser): + """Add pytest options.""" parser.addoption("--resource-group", action="store") parser.addoption("--registry", action="store") def pytest_generate_tests(metafunc): + """Generate test cases based on options.""" resource_group_value = metafunc.config.option.resource_group if 'resource_group' in metafunc.fixturenames and resource_group_value is not None: metafunc.parametrize('resource_group', [resource_group_value]) diff --git a/test/resources/pytest/bad-assets/test-asset-1/tests/test_component.py b/test/resources/pytest/bad-assets/test-asset-1/tests/test_component.py index 1a5951f34e..806fa8cf74 100644 --- a/test/resources/pytest/bad-assets/test-asset-1/tests/test_component.py +++ b/test/resources/pytest/bad-assets/test-asset-1/tests/test_component.py @@ -1,5 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test assets via pytest.""" + + def test_failure(): + """Mocks a test.""" assert False diff --git a/test/resources/pytest/good-assets-with-requirements/test-asset-1/tests/test_component.py b/test/resources/pytest/good-assets-with-requirements/test-asset-1/tests/test_component.py index bb7cf7545f..65590449c9 100644 --- a/test/resources/pytest/good-assets-with-requirements/test-asset-1/tests/test_component.py +++ b/test/resources/pytest/good-assets-with-requirements/test-asset-1/tests/test_component.py @@ -1,5 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test assets via pytest.""" + + def test_success(): + """Mocks a test.""" assert True diff --git a/test/resources/pytest/mixed-assets/test-asset-1/tests/test_component.py b/test/resources/pytest/mixed-assets/test-asset-1/tests/test_component.py index bb7cf7545f..65590449c9 100644 --- a/test/resources/pytest/mixed-assets/test-asset-1/tests/test_component.py +++ b/test/resources/pytest/mixed-assets/test-asset-1/tests/test_component.py @@ -1,5 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test assets via pytest.""" + + def test_success(): + """Mocks a test.""" assert True diff --git a/test/resources/pytest/mixed-assets/test-asset-2/tests/test_component.py b/test/resources/pytest/mixed-assets/test-asset-2/tests/test_component.py index 1a5951f34e..806fa8cf74 100644 --- a/test/resources/pytest/mixed-assets/test-asset-2/tests/test_component.py +++ b/test/resources/pytest/mixed-assets/test-asset-2/tests/test_component.py @@ -1,5 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test assets via pytest.""" + + def test_failure(): + """Mocks a test.""" assert False diff --git a/test/resources/release/src/code/run.py b/test/resources/release/src/code/run.py index acfb8c720e..bece471d4d 100644 --- a/test/resources/release/src/code/run.py +++ b/test/resources/release/src/code/run.py @@ -1,9 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -""" -Creates random images. -""" +"""Creates random images.""" + + import os import numpy as np @@ -12,6 +12,7 @@ def generate(path, samples, classes, width, height): + """Generate and save random images.""" os.makedirs(path, exist_ok=True) for i in range(samples): @@ -30,6 +31,7 @@ def generate(path, samples, classes, width, height): def main(cli_args: list = None) -> None: + """Parse args and generate random images for training and validation sets.""" parser = argparse.ArgumentParser(__doc__) parser.add_argument( "--output_train", diff --git a/test/test_build_environments.py b/test/test_build_environments.py index ab000eb9a4..55b4453884 100644 --- a/test/test_build_environments.py +++ b/test/test_build_environments.py @@ -1,6 +1,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test environment build scripts.""" + import tempfile from pathlib import Path from typing import Tuple @@ -12,6 +14,7 @@ def test_build_assets(build_subdir_expected_pair: Tuple[str, bool], resource_group: str, registry: str): + """Test building images.""" this_dir = Path(__file__).parent test_subdir, expected = build_subdir_expected_pair diff --git a/test/test_release_assets.py b/test/test_release_assets.py index 1619ad9e05..926f8edfec 100644 --- a/test/test_release_assets.py +++ b/test/test_release_assets.py @@ -1,6 +1,8 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +"""Test release folder scripts.""" + import azureml.assets.util as util import pytest from pathlib import Path @@ -36,6 +38,7 @@ ] ) def test_release_paths(test_subdir: str, expected: List[Path]): + """Test release paths.""" this_dir = Path(__file__).parent test_dir = this_dir / RESOURCES_DIR / test_subdir @@ -73,6 +76,7 @@ def test_release_paths(test_subdir: str, expected: List[Path]): ] ) def test_find_common_directory(test_subdir: str, expected: Tuple[Path, List[Path]]): + """Test for finding common directory.""" this_dir = Path(__file__).parent test_dir = this_dir / RESOURCES_DIR / test_subdir