From 663527f8440ffc6609cbfe9e7e32ede33dd81a04 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 13:32:55 -0600 Subject: [PATCH 01/11] Added a pre-commit configuration The configuration runs the formatters currently used by the project, i.e. clang-format v15 and black v25. --- .pre-commit-config.yaml | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 000000000..77648477c --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,13 @@ +repos: + - repo: https://github.com/pre-commit/mirrors-clang-format + rev: v15.0.7 + hooks: + - id: clang-format + types_or: [c++, c] + files: 'libtiledbvcf/.*|apis/python/src/.*' + exclude: 'libtiledbvcf/src/external/.*' + - repo: https://github.com/psf/black + rev: 25.9.0 + hooks: + - id: black + files: 'apis/python/src/.*' From 8058fcb49046c3a4e19e88f724127f9ef02dae37 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 13:35:36 -0600 Subject: [PATCH 02/11] Removed the format commands from the Makefile These will now be handled automatically with pre-commit and GitHub workflows. --- Makefile | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/Makefile b/Makefile index d4de25e05..6a4889387 100644 --- a/Makefile +++ b/Makefile @@ -51,22 +51,6 @@ docs: @rm tiledbvcf quarto render --fail-if-warnings -# format -# ------------------------------------------------------------------- -.PHONY: check-format -check-format: - @./ci/run-clang-format.sh . clang-format 0 \ - `find libtiledbvcf/src -name "*.cc" -or -name "*.h"` - @./ci/run-clang-format.sh . clang-format 0 \ - `find libtiledbvcf/test -name "*.cc" -or -name "*.h"` - -.PHONY: format -format: - @./ci/run-clang-format.sh . clang-format 1 \ - `find libtiledbvcf/src -name "*.cc" -or -name "*.h"` - @./ci/run-clang-format.sh . clang-format 1 \ - `find libtiledbvcf/test -name "*.cc" -or -name "*.h"` - # venv # ------------------------------------------------------------------- .PHONY: venv @@ -116,8 +100,6 @@ Rules: notebooks Execute notebooks and update cell outputs docs Render the documentation docker Build and test docker images - check-format Run C++ format check - format Run C++ format venv Create a virtual environment wheel Build python wheel clean Remove build artifacts From bbd2439e9fafe578f110c181f84011672a1ba20b Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 13:44:56 -0600 Subject: [PATCH 03/11] Added a GitHub workflow that runs pre-commit --- .github/workflows/ci-linters.yml | 33 ++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 .github/workflows/ci-linters.yml diff --git a/.github/workflows/ci-linters.yml b/.github/workflows/ci-linters.yml new file mode 100644 index 000000000..de506b26b --- /dev/null +++ b/.github/workflows/ci-linters.yml @@ -0,0 +1,33 @@ +name: Run pre-commit + +on: + pull_request: + workflow_dispatch: + + +jobs: + lint: + runs-on: ubuntu-24.04 + steps: + - name: Checkout TileDB-VCF + uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Restore pre-commit cache + uses: actions/cache@v4 + with: + path: ~/.cache/pre-commit + key: pre-commit-ubuntu-24.04-3.12-${{ hashFiles('.github/workflows/lint-python.yml', '.pre-commit-config.yaml') }} + + - name: Install pre-commit + run: pip -v install pre-commit + + - name: Log pip dependencies + run: pip list + + - name: Run pre-commit hooks on all files + run: pre-commit run -a -v From 6a2e8bfac1aa39878688ebd774f84345f4b3d426 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 13:59:29 -0600 Subject: [PATCH 04/11] Removed Clang Tools from SuperBuild This module was already deprecated and is being removed since ClangFormat is now managed by pre-commit. --- .../cmake/Modules/FindClangTools.cmake | 73 ------------------- 1 file changed, 73 deletions(-) delete mode 100644 libtiledbvcf/cmake/Modules/FindClangTools.cmake diff --git a/libtiledbvcf/cmake/Modules/FindClangTools.cmake b/libtiledbvcf/cmake/Modules/FindClangTools.cmake deleted file mode 100644 index e0b3dbcfb..000000000 --- a/libtiledbvcf/cmake/Modules/FindClangTools.cmake +++ /dev/null @@ -1,73 +0,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. -# -# Tries to find the clang-tidy and clang-format modules -# -# Usage of this module as follows: -# -# find_package(ClangTools) -# -# Variables used by this module, they can change the default behaviour and need -# to be set before calling find_package: -# -# ClangToolsBin_HOME - -# When set, this path is inspected instead of standard library binary locations -# to find clang-tidy and clang-format -# -# This module defines -# CLANG_TIDY_BIN, The path to the clang tidy binary -# CLANG_TIDY_FOUND, Whether clang tidy was found -# CLANG_FORMAT_BIN, The path to the clang format binary -# CLANG_TIDY_FOUND, Whether clang format was found - -find_program(CLANG_TIDY_BIN - NAMES clang-tidy-5.0 - clang-tidy-4.0 - clang-tidy-3.9 - clang-tidy-3.8 - clang-tidy-3.7 - clang-tidy-3.6 - clang-tidy - PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin - NO_DEFAULT_PATH -) - -if ( "${CLANG_TIDY_BIN}" STREQUAL "CLANG_TIDY_BIN-NOTFOUND" ) - set(CLANG_TIDY_FOUND 0) - message(STATUS "Not found clang-tidy") -else() - set(CLANG_TIDY_FOUND 1) - message(STATUS "Found clang-tidy: ${CLANG_TIDY_BIN}") -endif() - -find_program(CLANG_FORMAT_BIN - NAMES - clang-format-15 - clang-format-5.0 - clang-format-4.0 - clang-format-3.9 - clang-format-3.8 - clang-format-3.7 - clang-format-3.6 - clang-format - PATHS ${ClangTools_PATH} $ENV{CLANG_TOOLS_PATH} /usr/local/bin /usr/bin - NO_DEFAULT_PATH -) - -if ( "${CLANG_FORMAT_BIN}" STREQUAL "CLANG_FORMAT_BIN-NOTFOUND" ) - set(CLANG_FORMAT_FOUND 0) - message(STATUS "Not found clang-format") -else() - set(CLANG_FORMAT_FOUND 1) - message(STATUS "Found clang-format: ${CLANG_FORMAT_BIN}") -endif() From d2d8dd5fd92e3a41d1df4a7b3309687b28c03378 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 14:09:11 -0600 Subject: [PATCH 05/11] Removed formatting from existing CI This is because formatting is now handled via the pre-commit CI. Note that pylint was removed here as well since this is the only place in the project that it is used. --- ci/azure-linux_mac.yml | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/ci/azure-linux_mac.yml b/ci/azure-linux_mac.yml index d76c41271..376cc3584 100755 --- a/ci/azure-linux_mac.yml +++ b/ci/azure-linux_mac.yml @@ -6,34 +6,6 @@ steps: addToPath: true displayName: 'Set Python Version' - - script: python -m pip install --upgrade black pylint - displayName: 'Install black (formatter) and pylint (linter)' - - - script: black --check . - displayName: 'Test Formatting' - - # pylint cannot lint all files in a directory, it requires an __init__.py file (https://github.com/PyCQA/pylint/issues/352) - # run pylint on all directories with an __init__.py file - - bash: | - set -eux pipefail - find . -name __init__.py | xargs dirname | xargs pylint --disable=all --enable=E1101 - displayName: 'Check function name/signature mismatches' - - - bash: | - set -e pipefail - sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 15CF4D18AF4F7421 - sudo add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-14 main' - sudo apt-get install -y clang-format-15 - src=$BUILD_REPOSITORY_LOCALPATH/libtiledbvcf - cd $BUILD_REPOSITORY_LOCALPATH - ci/run-clang-format.sh $src clang-format-15 0 \ - $(find $src/src $src/test -name "*.cc" -or -name "*.c" -or -name "*.h") - apis=$BUILD_REPOSITORY_LOCALPATH/apis - ci/run-clang-format.sh $apis clang-format-15 0 \ - $(find $apis -name "*.cc" -or -name "*.c" -or -name "*.h") - condition: eq(variables['Agent.OS'], 'Linux') - displayName: 'Check formatting (Linux only)' - - bash: | set -e pipefail From e1905fb4371a39ede4a2aef2d364f1967a933ca5 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 14:13:48 -0600 Subject: [PATCH 06/11] Removed run-clang-format.sh CI scripts This has been replaced by pre-commit. --- ci/run-clang-format.sh | 43 ----------------------------- libtiledbvcf/cmake/Superbuild.cmake | 20 -------------- 2 files changed, 63 deletions(-) delete mode 100755 ci/run-clang-format.sh diff --git a/ci/run-clang-format.sh b/ci/run-clang-format.sh deleted file mode 100755 index 68af13601..000000000 --- a/ci/run-clang-format.sh +++ /dev/null @@ -1,43 +0,0 @@ -#!/bin/bash -# -# 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. -# -# Runs clang format in the given directory -# Arguments: -# $1 - Path to the source tree -# $2 - Path to the clang format binary -# $3 - Apply fixes (will raise an error if false and not there where changes) -# $ARGN - Files to run clang format on -# -SOURCE_DIR=$1 -shift -CLANG_FORMAT=$1 -shift -APPLY_FIXES=$1 -shift - -# clang format will only find its configuration if we are in -# the source tree or in a path relative to the source tree -pushd "$SOURCE_DIR" || exit -if [ "$APPLY_FIXES" == "1" ]; then - $CLANG_FORMAT -i "$@" -else - $CLANG_FORMAT --version - $CLANG_FORMAT --dry-run "$@" - NUM_CORRECTIONS=$("$CLANG_FORMAT" --output-replacements-xml "$@" | grep --count offset) - if [ "$NUM_CORRECTIONS" -gt "0" ]; then - echo "clang-format suggested $NUM_CORRECTIONS changes, please run 'make format'!" - exit 1 - fi -fi -popd || exit diff --git a/libtiledbvcf/cmake/Superbuild.cmake b/libtiledbvcf/cmake/Superbuild.cmake index ced7e1f88..a200c1f25 100644 --- a/libtiledbvcf/cmake/Superbuild.cmake +++ b/libtiledbvcf/cmake/Superbuild.cmake @@ -73,26 +73,6 @@ include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindSpdlog.cmake) include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindTileDB_EP.cmake) include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindCatch2.cmake) -############################################################ -# 'make format' target -############################################################ - -set(SCRIPTS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../ci") - -find_package(ClangTools) -if (${CLANG_FORMAT_FOUND}) - # Runs clang-format and updates files in place. - add_custom_target(format ${SCRIPTS_DIR}/run-clang-format.sh ${CMAKE_CURRENT_SOURCE_DIR}/src ${CLANG_FORMAT_BIN} 1 - `find ${CMAKE_CURRENT_SOURCE_DIR}/src ${CMAKE_CURRENT_SOURCE_DIR}/test - -name \\*.cc -or -name \\*.c -or -name \\*.h`) - - # Runs clang-format and exits with a non-zero exit code# if any files need to - # be reformatted - add_custom_target(check-format ${SCRIPTS_DIR}/run-clang-format.sh ${CMAKE_CURRENT_SOURCE_DIR}/src ${CLANG_FORMAT_BIN} 0 - `find ${CMAKE_CURRENT_SOURCE_DIR}/src ${CMAKE_CURRENT_SOURCE_DIR}/test - -name \\*.cc -or -name \\*.c -or -name \\*.h`) -endif() - ############################################################ # Set up the regular build (i.e. non-superbuild). ############################################################ From 091fa2ef10d6430b7e4f4266140caa220fc419f1 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 14:19:08 -0600 Subject: [PATCH 07/11] Updated clang-format pre-commit configuration to include Java API --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 77648477c..8697344cd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -4,7 +4,7 @@ repos: hooks: - id: clang-format types_or: [c++, c] - files: 'libtiledbvcf/.*|apis/python/src/.*' + files: 'libtiledbvcf/.*|apis/java/src/main/java/io/tiledb/libvcfnative/.*|apis/python/src/.*' exclude: 'libtiledbvcf/src/external/.*' - repo: https://github.com/psf/black rev: 25.9.0 From f3dc18d291d808065499511a70b717ba75f9d780 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 14:20:02 -0600 Subject: [PATCH 08/11] Removed clang-format from Java JNI script --- apis/java/src/main/java/io/tiledb/libvcfnative/generateJNI.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/apis/java/src/main/java/io/tiledb/libvcfnative/generateJNI.sh b/apis/java/src/main/java/io/tiledb/libvcfnative/generateJNI.sh index e25b3f6d8..43879f603 100644 --- a/apis/java/src/main/java/io/tiledb/libvcfnative/generateJNI.sh +++ b/apis/java/src/main/java/io/tiledb/libvcfnative/generateJNI.sh @@ -1,4 +1,3 @@ javac -cp .:commons-io-2.14.0.jar *.java -h .; -clang-format -i io_tiledb_libvcfnative_LibVCFNative.h; mv io_tiledb_libvcfnative_LibVCFNative.h LibVCFNative.h; -rm *.class; \ No newline at end of file +rm *.class; From 9d69352d01972bebaa81d4575efd4a9d64147cdb Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 14:42:45 -0600 Subject: [PATCH 09/11] Removed black from Python API dependencies This is because black is now managed by pre-commit. --- apis/python/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apis/python/pyproject.toml b/apis/python/pyproject.toml index 552b6a3dd..978e6c789 100644 --- a/apis/python/pyproject.toml +++ b/apis/python/pyproject.toml @@ -44,7 +44,7 @@ dependencies = ["pandas", "pyarrow>=14.0.1", "numpy"] [project.optional-dependencies] test = ["pytest", "tiledb"] -dev = ["black", "tiledb-cloud", "pybind11[global]"] +dev = ["tiledb-cloud", "pybind11[global]"] [project.urls] homepage = "https://tiledb.com" From 1a8695e25447ccb6073ee9fe24e8fcda2ce51d29 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 14:53:15 -0600 Subject: [PATCH 10/11] Update formatting related documentation Specifically, clang-format was dropped from the Java API documentation and pre-commit in now mentioned in the contributing documentation. --- CONTRIBUTING.md | 9 +++++++++ apis/java/src/main/java/io/tiledb/libvcfnative/README.md | 7 ++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e4d3ab3b2..cb7cb2b54 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,3 +18,12 @@ Thanks for your interest in TileDB-VCF. The notes below give some pointers for f - Make changes locally, then rebuild as appropriate for the level of changes (e.g.: `make` for `libtilebvcf` or `python setup.py develop` for `apis/python`). - Make sure to run `make check`, or `pytest` to verify changes against tests (add new tests where applicable). - Please submit [pull requests](https://help.github.com/en/desktop/contributing-to-projects/creating-a-pull-request) against the default [`main` branch of TileDB-VCF](https://github.com/TileDB-Inc/TileDB-VCF/tree/main). +- If you edit the Python files, please run the pre-commit hooks + + ```sh + python -m venv ./pre-commit + source ./pre-commit/bin/activate + python -m pip -v install pre-commit + pre-commit run -a -v + deactivate + ``` diff --git a/apis/java/src/main/java/io/tiledb/libvcfnative/README.md b/apis/java/src/main/java/io/tiledb/libvcfnative/README.md index d3d87291b..60074d748 100644 --- a/apis/java/src/main/java/io/tiledb/libvcfnative/README.md +++ b/apis/java/src/main/java/io/tiledb/libvcfnative/README.md @@ -5,9 +5,8 @@ To build the binding you must first compile the java file to a header file by us javac -cp .:commons-io-2.14.0.jar *.java -h .; ``` -Next, format the header and replace the old +Next, replace the old header ``` -clang-format -i io_tiledb_libvcfnative_LibVCFNative.h; mv io_tiledb_libvcfnative_LibVCFNative.h LibVCFNative.h; ``` @@ -15,4 +14,6 @@ Finally, remove all ```.class``` files ``` rm *.class; -``` \ No newline at end of file +``` + +This process can be done autmatically using the `genearteJNI.sh` script. From 17b0a6369404194de8020c35a408910bdd71f590 Mon Sep 17 00:00:00 2001 From: Alan Cleary Date: Fri, 31 Oct 2025 15:17:40 -0600 Subject: [PATCH 11/11] Corrected external sources directory path in pre-commit configuration --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8697344cd..18bbc48a0 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,7 +5,7 @@ repos: - id: clang-format types_or: [c++, c] files: 'libtiledbvcf/.*|apis/java/src/main/java/io/tiledb/libvcfnative/.*|apis/python/src/.*' - exclude: 'libtiledbvcf/src/external/.*' + exclude: 'libtiledbvcf/external/.*' - repo: https://github.com/psf/black rev: 25.9.0 hooks: