From 597ed413252a9a7b52e54f13cc53534d787e2f3e Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Tue, 8 Feb 2022 15:43:20 +0100 Subject: [PATCH] Make time processing optional, abbreviate home with proper Pathlib APIs This commit contains two main fixes: 1) Time values are now processed conditionally on whether a "time_unit" key is found in the benchmark object. 2) The `abbrev_home` function now collapses the home directory with only pathlib's APIs. --- pybm/commands/run.py | 34 ++++++++++++++++++---------------- pybm/git.py | 4 ++-- pybm/reporters/console.py | 18 +++++++++++++----- pybm/reporters/util.py | 1 - pybm/util/print.py | 13 +++++++++---- 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/pybm/commands/run.py b/pybm/commands/run.py index 6d0d74f..a6e0e1e 100644 --- a/pybm/commands/run.py +++ b/pybm/commands/run.py @@ -22,7 +22,6 @@ class RunCommand(CLICommand): def __init__(self): super(RunCommand, self).__init__(name="run") self.config = PybmConfig.load() - self.use_legacy_checkout: bool = self.config.get_value("git.legacycheckout") def add_arguments(self): self.parser.add_argument( @@ -56,8 +55,8 @@ def add_arguments(self): action="store_true", default=False, help="Run benchmarks in checkout mode in environment 'root'. Here, instead " - "of persisted git worktrees, different refs are benchmarked using " - "`git checkout` commands.", + "of persisted git worktrees, different refs are benchmarked using `git " + "checkout` commands.", ) self.parser.add_argument( "--all", @@ -73,14 +72,14 @@ def add_arguments(self): default=None, dest="source_ref", metavar="", - help="Source benchmark targets from a different git reference.", + help="Source benchmark targets from a different git reference .", ) self.parser.add_argument( "--repetitions", type=int, default=5, metavar="", - help="Number of repetitions for the target benchmarks.", + help="Number of times to repeat the target benchmarks.", ) self.parser.add_argument( "--filter", @@ -98,8 +97,8 @@ def add_arguments(self): dest="benchmark_context", metavar="", help="Additional global context, given as strings in the format " - "--context='key'='value'. Keys must be unique. Supplying two or more " - "context values for the same key results in an error.", + "--context='key'='value'. Keys must be unique, supplying more than one " + "value for the same key results in an error.", ) runner: BaseRunner = get_component_class("runner", config=self.config) @@ -128,6 +127,9 @@ def run(self, args: List[str]) -> int: runner: BaseRunner = get_component_class("runner", config=self.config) reporter: BaseReporter = get_component_class("reporter", config=self.config) + # whether to use legacy checkouts (git < 2.17) + use_legacy_checkout: bool = self.config.get_value("git.legacycheckout") + verbose: bool = runner_options.pop("verbose") env_ids: List[str] = runner_options.pop("environments") or [] @@ -135,11 +137,11 @@ def run(self, args: List[str]) -> int: checkout_mode: bool = runner_options.pop("checkout") source_ref: Optional[str] = runner_options.pop("source_ref") source_path = Path(runner_options.pop("benchmarks")) - run_as_module = runner_options.pop("run_as_module") + run_as_module: bool = runner_options.pop("run_as_module") # runner dispatch arguments - repetitions = runner_options.pop("repetitions") - benchmark_filter = runner_options.pop("benchmark_filter") + repetitions: int = runner_options.pop("repetitions") + benchmark_filter: Optional[str] = runner_options.pop("benchmark_filter") benchmark_context = runner_options.pop("benchmark_context") # at this point, runner_options only include the additional runner kwargs @@ -198,7 +200,7 @@ def run(self, args: List[str]) -> int: worktree=worktree, source_path=source_path, source_ref=source_ref, - use_legacy_checkout=self.use_legacy_checkout, + use_legacy_checkout=use_legacy_checkout, ) as benchmark_targets: n = len(benchmark_targets) if n > 0: @@ -208,9 +210,9 @@ def run(self, args: List[str]) -> int: ) else: msg = ( - f"Benchmark selector {str(source_path)!r} did not " - f"match any directory or Python files for {ref_type} " - f"{ref!r} in environment {environment.name!r}." + f"Benchmark selector {str(source_path)!r} did not match any " + f"directory or Python files for {ref_type} {ref!r} in " + f"environment {environment.name!r}." ) if runner.fail_fast: @@ -247,8 +249,8 @@ def run(self, args: List[str]) -> int: "benchmark subprocess. Please check that the configured " "benchmark runner actually writes the results to stdout." ) - else: - reporter.write(ref, benchmark, data) + + reporter.write(ref, benchmark, data) if checkout_mode: root_ref, root_type = root_checkout diff --git a/pybm/git.py b/pybm/git.py index 1dcf3d3..229a64a 100644 --- a/pybm/git.py +++ b/pybm/git.py @@ -13,7 +13,7 @@ disambiguate_info, resolve_ref, is_main_worktree, - checkout, + checkout as git_checkout, resolve_commit, ) from pybm.util.path import current_folder @@ -325,7 +325,7 @@ def switch(self, worktree: Worktree, ref: str, ref_type: Optional[str] = None): f"Object {ref!r} could not be understood as a valid git reference." ) - checkout(ref=ref, cwd=worktree.root) + git_checkout(ref=ref, cwd=worktree.root) # null the old reference type if necessary if ref_type != old_type: diff --git a/pybm/reporters/console.py b/pybm/reporters/console.py index 49acfa2..f3512e4 100644 --- a/pybm/reporters/console.py +++ b/pybm/reporters/console.py @@ -35,12 +35,16 @@ def process(bm: Dict[str, Any], target_time_unit: str, shalength: int): bm["name"] = format_benchmark(bm.pop("name"), bm.pop("executable")) bm["reference"] = format_ref(bm.pop("ref"), bm.pop("commit"), shalength=shalength) - time_unit: str = bm.pop("time_unit") - time_values: Dict[str, Any] = dfilter_regex(".*time", bm) + time_unit: Optional[str] = bm.pop("time_unit", None) - rescale_fn = partial(rescale, current_unit=time_unit, target_unit=target_time_unit) + if time_unit is not None: + time_values: Dict[str, Any] = dfilter_regex(".*time", bm) - bm.update(dvmap(rescale_fn, time_values)) + rescale_fn = partial( + rescale, current_unit=time_unit, target_unit=target_time_unit + ) + + bm.update(dvmap(rescale_fn, time_values)) return bm @@ -48,6 +52,9 @@ def process(bm: Dict[str, Any], target_time_unit: str, shalength: int): def compare(results: List[Dict[str, Any]]): """Compare results between different refs with respect to an anchor ref. Assumes that the results are sorted in the same order.""" + if len(results) == 1: + return results + anchor_result = results[0] for result in results: @@ -152,7 +159,7 @@ def compare( processed_results = lmap(process_fn, reduced) - # group results by again benchmark name + # group results again by benchmark name grouped_results = groupby("name", processed_results) if absolute: @@ -164,6 +171,7 @@ def compare( formatted_results = lmap(transform_fn, compared_results) log_to_console(formatted_results, padding=self.padding) + # TODO: Print summary about improvements etc. def transform_result(self, bm: Dict[str, Any], anchor_ref: str) -> Dict[str, str]: """Finalize column header names, cast all values to string and diff --git a/pybm/reporters/util.py b/pybm/reporters/util.py index ea188c8..08e3aef 100644 --- a/pybm/reporters/util.py +++ b/pybm/reporters/util.py @@ -91,7 +91,6 @@ def log_to_console(results: List[Dict[str, str]], padding: int = 1): print(make_separator(column_widths, padding=padding)) print(make_line(res.values(), column_widths, padding=padding)) - # TODO: Print summary about improvements etc. def reduce(results: List[Dict[str, Any]]) -> Dict[str, Any]: diff --git a/pybm/util/print.py b/pybm/util/print.py index dcca873..5056c73 100644 --- a/pybm/util/print.py +++ b/pybm/util/print.py @@ -5,8 +5,14 @@ def abbrev_home(path: Union[str, Path]) -> str: - homepath = Path(path).resolve().relative_to(Path.home()) - return str(Path("~") / homepath) + path = Path(path).resolve() + try: + homepath = path.relative_to(Path.home()) + abbrev_path = str(Path("~") / homepath) + except ValueError: + # case where the env path is not a subpath of the home directory + abbrev_path = str(path) + return abbrev_path def calculate_column_widths(data: Iterable[Iterable[str]]) -> List[int]: @@ -17,9 +23,8 @@ def calculate_column_widths(data: Iterable[Iterable[str]]) -> List[int]: def make_line(values: Iterable[str], column_widths: Iterable[int], padding: int) -> str: pad_char = " " * padding sep = "|".join([pad_char] * 2) - offset = pad_char - line = offset + sep.join(f"{n:<{w}}" for n, w in zip(values, column_widths)) + line = pad_char + sep.join(f"{n:<{w}}" for n, w in zip(values, column_widths)) return line