Merge pull request #42 from nicholasjng/staging

Make time processing optional, abbreviate home with proper Pathlib APIs

Author: nicholasjng

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="<git-ref>",
-            help="Source benchmark targets from a different git reference.",
+            help="Source benchmark targets from a different git reference <git-ref>.",
         )
         self.parser.add_argument(
             "--repetitions",
             type=int,
             default=5,
             metavar="<N>",
-            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="<context>",
             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,18 +127,21 @@ 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 []
         run_all: bool = runner_options.pop("run_all")
         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

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:

pybm/reporters/console.py

@@ -35,19 +35,26 @@ 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
 
 
 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

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]:

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