Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Python] Configure pre-commit hook to run ruff on archery and other developer tools #45754

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,24 @@ repos:
?^ci/docker/python-.*-wheel-windows-test-vs2022.*\.dockerfile$|
)
types: []
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.9.10
hooks:
# Run the linter.
- id: ruff
args: [--fix]
files: >-
^dev/archery/
# Run the formatter.
- id: ruff-format
files: >-
^dev/
- repo: https://github.com/pycqa/flake8
rev: 6.1.0
hooks:
- id: flake8
name: Python Format
files: ^(python|dev|c_glib|integration)/
files: ^(python|c_glib|integration)/
types:
- file
- python
Expand Down
117 changes: 55 additions & 62 deletions c_glib/tool/generate-version-header.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,39 +26,31 @@

def main():
parser = argparse.ArgumentParser(
description="Generate C header with version macros")
description="Generate C header with version macros"
)
parser.add_argument(
"--library",
required=True,
help="The library name to use in macro prefixes")
"--library", required=True, help="The library name to use in macro prefixes"
)
parser.add_argument("--version", required=True, help="The library version number")
parser.add_argument(
"--version",
required=True,
help="The library version number")
"--input", type=Path, required=True, help="Path to the input template file"
)
parser.add_argument(
"--input",
type=Path,
required=True,
help="Path to the input template file")
parser.add_argument(
"--output",
type=Path,
required=True,
help="Path to the output file to generate")
"--output", type=Path, required=True, help="Path to the output file to generate"
)

args = parser.parse_args()

with open(args.input, "r", encoding="utf-8") as input_file, \
open(args.output, "w", encoding="utf-8") as output_file:
write_header(
input_file, output_file, args.library, args.version)
with (
open(args.input, "r", encoding="utf-8") as input_file,
open(args.output, "w", encoding="utf-8") as output_file,
):
write_header(input_file, output_file, args.library, args.version)


def write_header(
input_file: TextIOBase,
output_file: TextIOBase,
library_name: str,
version: str):
input_file: TextIOBase, output_file: TextIOBase, library_name: str, version: str
):
if "-" in version:
version, version_tag = version.split("-")
else:
Expand All @@ -70,17 +62,18 @@ def write_header(
availability_macros = generate_availability_macros(library_name)

replacements = {
"VERSION_MAJOR": str(version_major),
"VERSION_MINOR": str(version_minor),
"VERSION_MICRO": str(version_micro),
"VERSION_TAG": version_tag,
"ENCODED_VERSIONS": encoded_versions,
"VISIBILITY_MACROS": visibility_macros,
"AVAILABILITY_MACROS": availability_macros,
"VERSION_MAJOR": str(version_major),
"VERSION_MINOR": str(version_minor),
"VERSION_MICRO": str(version_micro),
"VERSION_TAG": version_tag,
"ENCODED_VERSIONS": encoded_versions,
"VISIBILITY_MACROS": visibility_macros,
"AVAILABILITY_MACROS": availability_macros,
}

output_file.write(re.sub(
r"@([A-Z_]+)@", lambda match: replacements[match[1]], input_file.read()))
output_file.write(
re.sub(r"@([A-Z_]+)@", lambda match: replacements[match[1]], input_file.read())
)


def generate_visibility_macros(library: str) -> str:
Expand Down Expand Up @@ -140,36 +133,36 @@ def generate_availability_macros(library: str) -> str:


ALL_VERSIONS = [
(20, 0),
(19, 0),
(18, 0),
(17, 0),
(16, 0),
(15, 0),
(14, 0),
(13, 0),
(12, 0),
(11, 0),
(10, 0),
(9, 0),
(8, 0),
(7, 0),
(6, 0),
(5, 0),
(4, 0),
(3, 0),
(2, 0),
(1, 0),
(0, 17),
(0, 16),
(0, 15),
(0, 14),
(0, 13),
(0, 12),
(0, 11),
(0, 10),
(20, 0),
(19, 0),
(18, 0),
(17, 0),
(16, 0),
(15, 0),
(14, 0),
(13, 0),
(12, 0),
(11, 0),
(10, 0),
(9, 0),
(8, 0),
(7, 0),
(6, 0),
(5, 0),
(4, 0),
(3, 0),
(2, 0),
(1, 0),
(0, 17),
(0, 16),
(0, 15),
(0, 14),
(0, 13),
(0, 12),
(0, 11),
(0, 10),
]


if __name__ == '__main__':
if __name__ == "__main__":
main()
12 changes: 5 additions & 7 deletions dev/archery/archery/benchmark/codec.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

from __future__ import annotations

import json

from ..benchmark.compare import BenchmarkComparator
from ..benchmark.core import Benchmark, BenchmarkSuite
from ..benchmark.runner import BenchmarkRunner, StaticBenchmarkRunner
from ..benchmark.compare import BenchmarkComparator


class JsonEncoder(json.JSONEncoder):
Expand Down Expand Up @@ -63,13 +63,12 @@ class BenchmarkSuiteCodec:
def encode(bs):
return {
"name": bs.name,
"benchmarks": [BenchmarkCodec.encode(b) for b in bs.benchmarks]
"benchmarks": [BenchmarkCodec.encode(b) for b in bs.benchmarks],
}

@staticmethod
def decode(dct, **kwargs):
benchmarks = [BenchmarkCodec.decode(b)
for b in dct.pop("benchmarks", [])]
benchmarks = [BenchmarkCodec.decode(b) for b in dct.pop("benchmarks", [])]
return BenchmarkSuite(benchmarks=benchmarks, **dct, **kwargs)


Expand All @@ -80,8 +79,7 @@ def encode(br):

@staticmethod
def decode(dct, **kwargs):
suites = [BenchmarkSuiteCodec.decode(s)
for s in dct.pop("suites", [])]
suites = [BenchmarkSuiteCodec.decode(s) for s in dct.pop("suites", [])]
return StaticBenchmarkRunner(suites=suites, **dct, **kwargs)


Expand Down
53 changes: 29 additions & 24 deletions dev/archery/archery/benchmark/compare.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,37 @@

# Define a global regression threshold as 5%. This is purely subjective and
# flawed. This does not track cumulative regression.
from __future__ import annotations

DEFAULT_THRESHOLD = 0.05


def items_per_seconds_fmt(value):
if value < 1000:
return "{} items/sec".format(value)
return f"{value} items/sec"
if value < 1000**2:
return "{:.3f}K items/sec".format(value / 1000)
return f"{value / 1000:.3f}K items/sec"
if value < 1000**3:
return "{:.3f}M items/sec".format(value / 1000**2)
return f"{value / 1000**2:.3f}M items/sec"
else:
return "{:.3f}G items/sec".format(value / 1000**3)
return f"{value / 1000**3:.3f}G items/sec"


def bytes_per_seconds_fmt(value):
if value < 1024:
return "{} bytes/sec".format(value)
return f"{value} bytes/sec"
if value < 1024**2:
return "{:.3f} KiB/sec".format(value / 1024)
return f"{value / 1024:.3f} KiB/sec"
if value < 1024**3:
return "{:.3f} MiB/sec".format(value / 1024**2)
return f"{value / 1024**2:.3f} MiB/sec"
if value < 1024**4:
return "{:.3f} GiB/sec".format(value / 1024**3)
return f"{value / 1024**3:.3f} GiB/sec"
else:
return "{:.3f} TiB/sec".format(value / 1024**4)
return f"{value / 1024**4:.3f} TiB/sec"


def change_fmt(value):
return "{:.3%}".format(value)
return f"{value:.3%}"


def formatter_for_unit(unit):
Expand All @@ -59,14 +61,15 @@ def formatter_for_unit(unit):


class BenchmarkComparator:
""" Compares two benchmarks.
"""Compares two benchmarks.

Encodes the logic of comparing two benchmarks and taking a decision on
if it induce a regression.
"""

def __init__(self, contender, baseline, threshold=DEFAULT_THRESHOLD,
suite_name=None):
def __init__(
self, contender, baseline, threshold=DEFAULT_THRESHOLD, suite_name=None
):
self.contender = contender
self.baseline = baseline
self.threshold = threshold
Expand Down Expand Up @@ -98,14 +101,14 @@ def change(self):

@property
def confidence(self):
""" Indicate if a comparison of benchmarks should be trusted. """
"""Indicate if a comparison of benchmarks should be trusted."""
return True

@property
def regression(self):
change = self.change
adjusted_change = change if self.less_is_better else -change
return (self.confidence and adjusted_change > self.threshold)
return self.confidence and adjusted_change > self.threshold

@property
def formatted(self):
Expand All @@ -118,7 +121,7 @@ def formatted(self):
"contender": fmt(self.contender.value),
"unit": self.unit,
"less_is_better": self.less_is_better,
"counters": str(self.baseline.counters)
"counters": str(self.baseline.counters),
}

def compare(self, comparator=None):
Expand All @@ -130,7 +133,7 @@ def compare(self, comparator=None):
"contender": self.contender.value,
"unit": self.unit,
"less_is_better": self.less_is_better,
"counters": self.baseline.counters
"counters": self.baseline.counters,
}

def __call__(self, **kwargs):
Expand All @@ -141,12 +144,12 @@ def pairwise_compare(contender, baseline):
dict_contender = {e.name: e for e in contender}
dict_baseline = {e.name: e for e in baseline}

for name in (dict_contender.keys() & dict_baseline.keys()):
for name in dict_contender.keys() & dict_baseline.keys():
yield name, (dict_contender[name], dict_baseline[name])


class RunnerComparator:
""" Compares suites/benchmarks from runners.
"""Compares suites/benchmarks from runners.

It is up to the caller that ensure that runners are compatible (both from
the same language implementation).
Expand All @@ -164,10 +167,12 @@ def comparisons(self):
suites = pairwise_compare(contender, baseline)

for suite_name, (suite_cont, suite_base) in suites:
benchmarks = pairwise_compare(
suite_cont.benchmarks, suite_base.benchmarks)
benchmarks = pairwise_compare(suite_cont.benchmarks, suite_base.benchmarks)

for _, (bench_cont, bench_base) in benchmarks:
yield BenchmarkComparator(bench_cont, bench_base,
threshold=self.threshold,
suite_name=suite_name)
yield BenchmarkComparator(
bench_cont,
bench_base,
threshold=self.threshold,
suite_name=suite_name,
)
12 changes: 6 additions & 6 deletions dev/archery/archery/benchmark/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from __future__ import annotations


def median(values):
Expand All @@ -27,8 +28,9 @@ def median(values):


class Benchmark:
def __init__(self, name, unit, less_is_better, values, time_unit,
times, counters=None):
def __init__(
self, name, unit, less_is_better, values, time_unit, times, counters=None
):
self.name = name
self.unit = unit
self.less_is_better = less_is_better
Expand All @@ -43,7 +45,7 @@ def value(self):
return self.median

def __repr__(self):
return "Benchmark[name={},value={}]".format(self.name, self.value)
return f"Benchmark[name={self.name},value={self.value}]"


class BenchmarkSuite:
Expand All @@ -52,6 +54,4 @@ def __init__(self, name, benchmarks):
self.benchmarks = benchmarks

def __repr__(self):
return "BenchmarkSuite[name={}, benchmarks={}]".format(
self.name, self.benchmarks
)
return f"BenchmarkSuite[name={self.name}, benchmarks={self.benchmarks}]"
Loading
Loading