From 72c448e105b3ecb9757fc9e2a41ee846f077ea7c Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 7 Dec 2023 23:02:11 +0000 Subject: [PATCH 1/9] isort + yapf --- .pre-commit-config.yaml | 20 +++++++++++++++++++ ci/githooks/pre-commit | 1 - ci/scripts/github/checks.sh | 3 --- .../python/mrc_qs_python/_version.py | 3 +-- .../ex02_reactive_operators/run.py | 3 ++- docs/quickstart/python/versioneer.py | 3 +-- python/setup.py | 3 +-- 7 files changed, 25 insertions(+), 11 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bdfff9d22..c8b766f7a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,6 +6,26 @@ repos: hooks: - id: rapids-dependency-file-generator args: ["--clean"] + - repo: https://github.com/PyCQA/isort + rev: 5.12.0 + hooks: + - id: isort + # Use the config file specific to each subproject so that each + # project can specify its own first/third-party packages. + args: ["--config-root=python/", "--resolve-all-configs"] + files: python/.* + types_or: [python, cython, pyi] + # - repo: https://github.com/pre-commit/mirrors-clang-format + # rev: v16.0.6 + # hooks: + # - id: clang-format + # types_or: [c, c++, cuda] + # args: ["-fallback-style=none", "-style=file", "-i"] + - repo: https://github.com/google/yapf + rev: '' # Use the sha / tag you want to point at + hooks: + - id: yapf + args: ["--style", "./python/setup.cfg"] default_language_version: python: python3 diff --git a/ci/githooks/pre-commit b/ci/githooks/pre-commit index e74e35fb3..7fa4b83a1 100755 --- a/ci/githooks/pre-commit +++ b/ci/githooks/pre-commit @@ -41,6 +41,5 @@ export CHANGED_FILES=$(GIT_DIFF_ARGS="--cached --name-only" get_modified_files) if [[ "${CHANGED_FILES}" != "" ]]; then run_and_check "python3 ci/scripts/copyright.py --git-diff-staged --update-current-year --verify-apache-v2 --git-add" - run_and_check "ci/scripts/python_checks.sh" SKIP_CLANG_TIDY=1 SKIP_IWYU=1 run_and_check "ci/scripts/cpp_checks.sh" fi diff --git a/ci/scripts/github/checks.sh b/ci/scripts/github/checks.sh index e64b36183..56eb72b95 100755 --- a/ci/scripts/github/checks.sh +++ b/ci/scripts/github/checks.sh @@ -36,8 +36,5 @@ ${MRC_ROOT}/ci/scripts/version_checks.sh rapids-logger "Running C++ style checks" ${MRC_ROOT}/ci/scripts/cpp_checks.sh -rapids-logger "Runing Python style checks" -${MRC_ROOT}/ci/scripts/python_checks.sh - rapids-logger "Checking copyright headers" python ${MRC_ROOT}/ci/scripts/copyright.py --verify-apache-v2 --git-diff-commits ${CHANGE_TARGET} ${GIT_COMMIT} diff --git a/docs/quickstart/python/mrc_qs_python/_version.py b/docs/quickstart/python/mrc_qs_python/_version.py index 1ca6b055c..7fb4694cb 100644 --- a/docs/quickstart/python/mrc_qs_python/_version.py +++ b/docs/quickstart/python/mrc_qs_python/_version.py @@ -29,8 +29,7 @@ import re import subprocess import sys -from typing import Callable -from typing import Dict +from typing import Callable, Dict def get_keywords(): diff --git a/docs/quickstart/python/mrc_qs_python/ex02_reactive_operators/run.py b/docs/quickstart/python/mrc_qs_python/ex02_reactive_operators/run.py index e181ad053..0ed479f86 100644 --- a/docs/quickstart/python/mrc_qs_python/ex02_reactive_operators/run.py +++ b/docs/quickstart/python/mrc_qs_python/ex02_reactive_operators/run.py @@ -15,9 +15,10 @@ import dataclasses -import mrc from mrc.core import operators as ops +import mrc + @dataclasses.dataclass class MyCustomClass: diff --git a/docs/quickstart/python/versioneer.py b/docs/quickstart/python/versioneer.py index 5e21cd07d..350aa2069 100644 --- a/docs/quickstart/python/versioneer.py +++ b/docs/quickstart/python/versioneer.py @@ -286,8 +286,7 @@ import re import subprocess import sys -from typing import Callable -from typing import Dict +from typing import Callable, Dict class VersioneerConfig: diff --git a/python/setup.py b/python/setup.py index cc37c7077..4eaad7d0e 100644 --- a/python/setup.py +++ b/python/setup.py @@ -14,11 +14,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import versioneer from setuptools import find_namespace_packages from setuptools import setup -import versioneer - ############################################################################## # - Python package generation ------------------------------------------------ From 413f10fa60cb0047c603ea0dac50e12b0f953a0e Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 7 Dec 2023 23:10:47 +0000 Subject: [PATCH 2/9] flake8 --- .pre-commit-config.yaml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c8b766f7a..96d6144e3 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -13,19 +13,25 @@ repos: # Use the config file specific to each subproject so that each # project can specify its own first/third-party packages. args: ["--config-root=python/", "--resolve-all-configs"] - files: python/.* - types_or: [python, cython, pyi] + files: ^python/ # - repo: https://github.com/pre-commit/mirrors-clang-format # rev: v16.0.6 # hooks: # - id: clang-format # types_or: [c, c++, cuda] # args: ["-fallback-style=none", "-style=file", "-i"] + - repo: https://github.com/PyCQA/flake8 + rev: '' + hooks: + - id: flake8 + args: ["--config=./python/setup.cfg"] + files: ^python/ - repo: https://github.com/google/yapf rev: '' # Use the sha / tag you want to point at hooks: - id: yapf args: ["--style", "./python/setup.cfg"] + files: ^python/ default_language_version: python: python3 From e00596906ae74810d2c500966b312449d7c75ae1 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 7 Dec 2023 23:11:08 +0000 Subject: [PATCH 3/9] remove python_checks.sh --- ci/scripts/python_checks.sh | 105 ------------------------------------ 1 file changed, 105 deletions(-) delete mode 100755 ci/scripts/python_checks.sh diff --git a/ci/scripts/python_checks.sh b/ci/scripts/python_checks.sh deleted file mode 100755 index fb6015735..000000000 --- a/ci/scripts/python_checks.sh +++ /dev/null @@ -1,105 +0,0 @@ -#!/bin/bash - -# SPDX-FileCopyrightText: Copyright (c) 2021-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Based on style.sh from Morpheus - -SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" -source ${SCRIPT_DIR}/common.sh - -# Ignore errors and set path -set +e -LC_ALL=C.UTF-8 -LANG=C.UTF-8 - -# Pre-populate the return values in case they are skipped -ISORT_RETVAL=0 -FLAKE_RETVAL=0 -YAPF_RETVAL=0 - -get_modified_files ${PYTHON_FILE_REGEX} MRC_MODIFIED_FILES - -# When invoked by the git pre-commit hook CHANGED_FILES will already be defined -if [[ -n "${MRC_MODIFIED_FILES}" ]]; then - echo -e "Running Python checks on ${#MRC_MODIFIED_FILES[@]} files:" - - for f in "${MRC_MODIFIED_FILES[@]}"; do - echo " $f" - done - - if [[ "${SKIP_ISORT}" == "" ]]; then - ISORT_OUTPUT=`python3 -m isort --settings-file ${PY_CFG} --filter-files --check-only ${MRC_MODIFIED_FILES[@]} 2>&1` - ISORT_RETVAL=$? - fi - - if [[ "${SKIP_FLAKE}" == "" ]]; then - FLAKE_OUTPUT=`python3 -m flake8 --config ${PY_CFG} ${MRC_MODIFIED_FILES[@]} 2>&1` - FLAKE_RETVAL=$? - fi - - if [[ "${SKIP_YAPF}" == "" ]]; then - # Run yapf. Will return 1 if there are any diffs - YAPF_OUTPUT=`python3 -m yapf --style ${PY_CFG} --diff ${MRC_MODIFIED_FILES[@]} 2>&1` - YAPF_RETVAL=$? - fi -else - echo "No modified Python files to check" -fi - -# Output results if failure otherwise show pass -if [[ "${SKIP_ISORT}" != "" ]]; then - echo -e "\n\n>>>> SKIPPED: isort check\n\n" -elif [ "${ISORT_RETVAL}" != "0" ]; then - echo -e "\n\n>>>> FAILED: isort style check; begin output\n\n" - echo -e "${ISORT_OUTPUT}" - echo -e "\n\n>>>> FAILED: isort style check; end output\n\n" \ - "To auto-fix many issues (not all) run:\n" \ - " ./ci/scripts/fix_all.sh\n\n" -else - echo -e "\n\n>>>> PASSED: isort style check\n\n" -fi - -if [[ "${SKIP_FLAKE}" != "" ]]; then - echo -e "\n\n>>>> SKIPPED: flake8 check\n\n" -elif [ "${FLAKE_RETVAL}" != "0" ]; then - echo -e "\n\n>>>> FAILED: flake8 style check; begin output\n\n" - echo -e "${FLAKE_OUTPUT}" - echo -e "\n\n>>>> FAILED: flake8 style check; end output\n\n" \ - "To auto-fix many issues (not all) run:\n" \ - " ./ci/scripts/fix_all.sh\n\n" -else - echo -e "\n\n>>>> PASSED: flake8 style check\n\n" -fi - -if [[ "${SKIP_YAPF}" != "" ]]; then - echo -e "\n\n>>>> SKIPPED: yapf check\n\n" -elif [ "${YAPF_RETVAL}" != "0" ]; then - echo -e "\n\n>>>> FAILED: yapf style check; begin output\n\n" - echo -e "Incorrectly formatted files:" - YAPF_OUTPUT=`echo "${YAPF_OUTPUT}" | sed -nr 's/^\+\+\+ ([^ ]*) *\(reformatted\)$/\1/p'` - echo -e "${YAPF_OUTPUT}" - echo -e "\n\n>>>> FAILED: yapf style check; end output\n\n" \ - "To auto-fix many issues (not all) run:\n" \ - " ./ci/scripts/fix_all.sh\n\n" -else - echo -e "\n\n>>>> PASSED: yapf style check\n\n" -fi - -RETVALS=(${ISORT_RETVAL} ${FLAKE_RETVAL} ${YAPF_RETVAL}) -IFS=$'\n' -RETVAL=`echo "${RETVALS[*]}" | sort -nr | head -n1` - -exit $RETVAL From 58a8f1b3bb011ec0c01e7fd9beda563e68061175 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 7 Dec 2023 23:13:46 +0000 Subject: [PATCH 4/9] update dependencies.yaml --- .pre-commit-config.yaml | 10 ++-------- conda/environments/all_cuda-118_arch-x86_64.yaml | 2 -- conda/environments/ci_cuda-118_arch-x86_64.yaml | 2 -- dependencies.yaml | 2 -- 4 files changed, 2 insertions(+), 14 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 96d6144e3..b273207cc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,20 +14,14 @@ repos: # project can specify its own first/third-party packages. args: ["--config-root=python/", "--resolve-all-configs"] files: ^python/ - # - repo: https://github.com/pre-commit/mirrors-clang-format - # rev: v16.0.6 - # hooks: - # - id: clang-format - # types_or: [c, c++, cuda] - # args: ["-fallback-style=none", "-style=file", "-i"] - repo: https://github.com/PyCQA/flake8 - rev: '' + rev: 6.1.0 hooks: - id: flake8 args: ["--config=./python/setup.cfg"] files: ^python/ - repo: https://github.com/google/yapf - rev: '' # Use the sha / tag you want to point at + rev: v0.40.2 # Use the sha / tag you want to point at hooks: - id: yapf args: ["--style", "./python/setup.cfg"] diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index 313a30ff4..338c80a60 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -26,7 +26,6 @@ dependencies: - cuda-version=11.8 - cxx-compiler - doxygen=1.9.2 -- flake8 - gcovr=5.0 - gdb - glog=0.6 @@ -52,5 +51,4 @@ dependencies: - python=3.10 - scikit-build>=0.17 - ucx=1.14 -- yapf name: all_cuda-118_arch-x86_64 diff --git a/conda/environments/ci_cuda-118_arch-x86_64.yaml b/conda/environments/ci_cuda-118_arch-x86_64.yaml index 30288a878..62ffa522b 100644 --- a/conda/environments/ci_cuda-118_arch-x86_64.yaml +++ b/conda/environments/ci_cuda-118_arch-x86_64.yaml @@ -21,7 +21,6 @@ dependencies: - cuda-version=11.8 - cxx-compiler - doxygen=1.9.2 -- flake8 - gcovr=5.0 - glog=0.6 - graphviz=3.0 @@ -42,5 +41,4 @@ dependencies: - python=3.10 - scikit-build>=0.17 - ucx=1.14 -- yapf name: ci_cuda-118_arch-x86_64 diff --git a/dependencies.yaml b/dependencies.yaml index 8d6f0d3b2..f9b1b451a 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -93,8 +93,6 @@ dependencies: common: - output_types: [conda] packages: - - flake8 - - yapf - include-what-you-use=0.20 testing: From 43369c15f13d264da9dddb6a6ecf16f1062764ec Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 7 Dec 2023 23:16:10 +0000 Subject: [PATCH 5/9] . --- .pre-commit-config.yaml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b273207cc..9ddded445 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,9 +10,7 @@ repos: rev: 5.12.0 hooks: - id: isort - # Use the config file specific to each subproject so that each - # project can specify its own first/third-party packages. - args: ["--config-root=python/", "--resolve-all-configs"] + args: ["--settings-file=./python/setup.cfg"] files: ^python/ - repo: https://github.com/PyCQA/flake8 rev: 6.1.0 @@ -21,7 +19,7 @@ repos: args: ["--config=./python/setup.cfg"] files: ^python/ - repo: https://github.com/google/yapf - rev: v0.40.2 # Use the sha / tag you want to point at + rev: v0.40.2 hooks: - id: yapf args: ["--style", "./python/setup.cfg"] From 7d4fb2e153c0f6d8aac36b51a49ef83df884759d Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Thu, 7 Dec 2023 23:18:39 +0000 Subject: [PATCH 6/9] fix isort error --- python/setup.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/setup.py b/python/setup.py index 4eaad7d0e..cc37c7077 100644 --- a/python/setup.py +++ b/python/setup.py @@ -14,10 +14,11 @@ # See the License for the specific language governing permissions and # limitations under the License. -import versioneer from setuptools import find_namespace_packages from setuptools import setup +import versioneer + ############################################################################## # - Python package generation ------------------------------------------------ From d0141670584b75315e989f71890806d33b7e7224 Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Tue, 12 Dec 2023 18:09:28 +0000 Subject: [PATCH 7/9] add flake8 and yapf deps to developer_productivity --- conda/environments/all_cuda-118_arch-x86_64.yaml | 2 ++ dependencies.yaml | 2 ++ 2 files changed, 4 insertions(+) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index 338c80a60..313a30ff4 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -26,6 +26,7 @@ dependencies: - cuda-version=11.8 - cxx-compiler - doxygen=1.9.2 +- flake8 - gcovr=5.0 - gdb - glog=0.6 @@ -51,4 +52,5 @@ dependencies: - python=3.10 - scikit-build>=0.17 - ucx=1.14 +- yapf name: all_cuda-118_arch-x86_64 diff --git a/dependencies.yaml b/dependencies.yaml index f9b1b451a..ccb744c44 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -84,10 +84,12 @@ dependencies: - clang=16 - clangdev=16 - clangxx=16 + - flake8 - gdb - libclang-cpp=16 - libclang=16 - llvmdev=16 + - yapf code_style: common: From 1ddcefa2dbe2be7b0fc9e154e665b965b00f7eec Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Wed, 13 Dec 2023 19:11:12 +0000 Subject: [PATCH 8/9] rm ci/conda/environments --- ci/conda/environments/dev_env.yml | 70 ------------------------------- 1 file changed, 70 deletions(-) delete mode 100644 ci/conda/environments/dev_env.yml diff --git a/ci/conda/environments/dev_env.yml b/ci/conda/environments/dev_env.yml deleted file mode 100644 index ecd4003b6..000000000 --- a/ci/conda/environments/dev_env.yml +++ /dev/null @@ -1,70 +0,0 @@ -# SPDX-FileCopyrightText: Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Dependencies needed for development environment. Runtime deps are in meta.yml -name: mrc -channels: - - rapidsai - - nvidia/label/cuda-11.8.0 - - nvidia - - rapidsai-nightly - - conda-forge -dependencies: - - autoconf>=2.69 - - bash-completion - - benchmark=1.6.0 - - boost-cpp=1.82 - - ccache - - cmake=3.25 - - cuda-toolkit # Version comes from the channel above - - cxx-compiler # Sets up the distro versions of our compilers - - doxygen=1.9.2 - - flake8 - - flatbuffers=2.0 - - gcovr=5.0 - - gdb - - gflags=2.2 - - git>=2.35.3 # Needed for wildcards on safe.directory - - glog=0.6 - - gmock=1.13 - - graphviz=3.0 - - libgrpc=1.54.0 - - gtest=1.13 - - gxx=11.2 # Specifies which versions of GXX and GCC to use - - isort - - jinja2=3.0 - - lcov=1.15 - - libhwloc=2.9.2 - - libprotobuf=3.21 - - librmm=23.06 - - libtool - - ninja=1.10 - - nlohmann_json=3.9 - - numactl-libs-cos7-x86_64 - - numpy>=1.21 - - pip - - pkg-config=0.29 - - pybind11-stubgen=0.10 - - pytest - - pytest-timeout - - pytest-asyncio - - python=3.10 - - scikit-build>=0.17 - - sysroot_linux-64=2.17 - - ucx=1.14 - - yapf - - # Remove once `mamba repoquery whoneeds cudatoolkit` is empty. For now, we need to specify a version - - cudatoolkit=11.8 From 723b3f44d6f9eeb48928da319f98dded184e034d Mon Sep 17 00:00:00 2001 From: Christopher Harris Date: Fri, 15 Dec 2023 19:24:05 +0000 Subject: [PATCH 9/9] add numactl to dependencies.yaml --- conda/environments/all_cuda-118_arch-x86_64.yaml | 1 + conda/environments/ci_cuda-118_arch-x86_64.yaml | 1 + dependencies.yaml | 1 + 3 files changed, 3 insertions(+) diff --git a/conda/environments/all_cuda-118_arch-x86_64.yaml b/conda/environments/all_cuda-118_arch-x86_64.yaml index 313a30ff4..47efa30b9 100644 --- a/conda/environments/all_cuda-118_arch-x86_64.yaml +++ b/conda/environments/all_cuda-118_arch-x86_64.yaml @@ -42,6 +42,7 @@ dependencies: - llvmdev=16 - ninja=1.10 - nlohmann_json=3.9 +- numactl-libs-cos7-x86_64 - numpy>=1.21 - pkg-config=0.29 - pre-commit diff --git a/conda/environments/ci_cuda-118_arch-x86_64.yaml b/conda/environments/ci_cuda-118_arch-x86_64.yaml index 62ffa522b..4935bb82e 100644 --- a/conda/environments/ci_cuda-118_arch-x86_64.yaml +++ b/conda/environments/ci_cuda-118_arch-x86_64.yaml @@ -32,6 +32,7 @@ dependencies: - librmm=23.06 - ninja=1.10 - nlohmann_json=3.9 +- numactl-libs-cos7-x86_64 - pkg-config=0.29 - pre-commit - pybind11-stubgen=0.10 diff --git a/dependencies.yaml b/dependencies.yaml index ccb744c44..cc46ca5fb 100644 --- a/dependencies.yaml +++ b/dependencies.yaml @@ -63,6 +63,7 @@ dependencies: - librmm=23.06 - ninja=1.10 - nlohmann_json=3.9 + - numactl-libs-cos7-x86_64 - pkg-config=0.29 - pybind11-stubgen=0.10 - python=3.10