Merge pull request #295 from faster-cpython/path-typing

More careful Path typing

Author: mdboom

bench_runner/config.py

@@ -13,7 +13,7 @@
 
 @functools.cache
 def get_bench_runner_config(
-    filepath: Path = Path("bench_runner.toml"),
+    filepath: Path | str = Path("bench_runner.toml"),
 ):
-    with open(filepath, "rb") as fd:
+    with Path(filepath).open("rb") as fd:
         return tomllib.load(fd)

bench_runner/git.py

@@ -10,9 +10,12 @@
 import rich
 
 
+from .util import PathLike
+
+
 def get_log(
     format: str,
-    dirname: Path,
+    dirname: PathLike,
     ref: str | None = None,
     n: int = 1,
     extra: list[str] | None = None,
@@ -42,22 +45,22 @@ def get_log(
     ).strip()
 
 
-def get_git_hash(dirname: Path) -> str:
+def get_git_hash(dirname: PathLike) -> str:
     return get_log("%h", dirname)
 
 
-def get_git_commit_date(dirname: Path) -> str:
+def get_git_commit_date(dirname: PathLike) -> str:
     return get_log("%cI", dirname)
 
 
-def remove(repodir: Path, path: Path) -> None:
+def remove(repodir: Path, path: PathLike) -> None:
     subprocess.check_output(
         ["git", "rm", str(path)],
         cwd=repodir,
     )
 
 
-def get_git_merge_base(dirname: Path) -> str | None:
+def get_git_merge_base(dirname: PathLike) -> str | None:
     # We need to make sure we have commits from main that are old enough to be
     # the base of this branch, but not so old that we waste a ton of bandwidth
     commit_date = datetime.datetime.fromisoformat(get_git_commit_date(dirname))
@@ -109,14 +112,14 @@ def get_git_merge_base(dirname: Path) -> str | None:
         return merge_base
 
 
-def get_tags(dirname: Path) -> list[str]:
+def get_tags(dirname: PathLike) -> list[str]:
     subprocess.check_call(["git", "fetch", "--tags"], cwd=dirname)
     return subprocess.check_output(
         ["git", "tag"], cwd=dirname, encoding="utf-8"
     ).splitlines()
 
 
-def get_commits_between(dirname: Path, ref1: str, ref2: str) -> list[str]:
+def get_commits_between(dirname: PathLike, ref1: str, ref2: str) -> list[str]:
     return list(
         subprocess.check_output(
             ["git", "rev-list", "--ancestry-path", f"{ref1}..{ref2}"],
@@ -126,6 +129,6 @@ def get_commits_between(dirname: Path, ref1: str, ref2: str) -> list[str]:
     )
 
 
-def bisect_commits(dirname: Path, ref1: str, ref2: str) -> str:
+def bisect_commits(dirname: PathLike, ref1: str, ref2: str) -> str:
     commits = get_commits_between(dirname, ref1, ref2)
     return commits[len(commits) // 2]

bench_runner/hpt.py

@@ -35,16 +35,16 @@
 from numpy.typing import NDArray
 
 
-from bench_runner import util
-
+from . import util
+from .util import PathLike
 
 ACC_MAXSU = 2
 
 
 def load_from_json(
-    json_path: Path,
+    json_path: PathLike,
 ) -> dict[str, NDArray[np.float64]]:
-    with json_path.open() as fd:
+    with Path(json_path).open() as fd:
         content = json.load(fd)
 
     return load_data(content)
@@ -311,7 +311,7 @@ def maxspeedup(
             return base_su
 
 
-def make_report(ref: Path, head: Path, alpha=0.1):
+def make_report(ref: PathLike, head: PathLike, alpha=0.1):
     # The original code inverted the inputs from the standard in bench_runner,
     # and it's easier to just flip them here.
     a, b = head, ref

bench_runner/plot.py

@@ -24,6 +24,7 @@
 from . import config as mconfig
 from . import flags as mflags
 from . import result
+from .util import PathLike
 
 
 INTERPRETER_HEAVY = {
@@ -48,7 +49,7 @@
 }
 
 
-def savefig(output_filename: Path, **kwargs):
+def savefig(output_filename: PathLike, **kwargs):
     class Options:
         quiet = True
         remove_descriptive_elements = True
@@ -58,6 +59,8 @@ class Options:
         shorten_ids = True
         digits = 3
 
+    output_filename = Path(output_filename)
+
     plt.savefig(output_filename, **kwargs)
     plt.close("all")
 
@@ -139,7 +142,7 @@ def formatter(val, pos):
 
 def plot_diff(
     combined_data: result.CombinedData,
-    output_filename: Path,
+    output_filename: PathLike,
     title: str,
     differences: tuple[str, str],
 ) -> None:
@@ -227,7 +230,7 @@ def add_axvline(ax, dt: datetime.datetime, name: str):
 
 def longitudinal_plot(
     results: Iterable[result.Result],
-    output_filename: Path,
+    output_filename: PathLike,
     getter: Callable[
         [result.BenchmarkComparison], float | None
     ] = lambda r: r.hpt_percentile_float(99),
@@ -243,6 +246,7 @@ def get_comparison_value(ref, r, base):
             data[key] = value
             return value
 
+    output_filename = Path(output_filename)
     cfg = get_plot_config()
 
     data_cache = output_filename.with_suffix(".json")
@@ -252,7 +256,7 @@ def get_comparison_value(ref, r, base):
     else:
         data = {}
 
-    axs: Sequence[matplotlib.Axes]
+    axs: Sequence[matplotlib.Axes]  # pyright: ignore
 
     fig, axs = plt.subplots(
         len(cfg["versions"]),
@@ -350,7 +354,7 @@ def get_comparison_value(ref, r, base):
         json.dump(data, fd, indent=2)
 
 
-def _standardize_xlims(axs: Sequence[matplotlib.Axes]) -> None:
+def _standardize_xlims(axs: Sequence[matplotlib.Axes]) -> None:  # pyright: ignore
     if not len(axs):
         return
 
@@ -369,13 +373,15 @@ def _standardize_xlims(axs: Sequence[matplotlib.Axes]) -> None:
 
 def flag_effect_plot(
     results: Iterable[result.Result],
-    output_filename: Path,
+    output_filename: PathLike,
     getter: Callable[
         [result.BenchmarkComparison], float | None
     ] = lambda r: r.hpt_percentile_float(99),
     differences: tuple[str, str] = ("slower", "faster"),
     title="Performance improvement by configuration",
 ):
+    output_filename = Path(output_filename)
+
     # We don't need to track the performance of the Tier 2 configuration
     all_flags = [flag for flag in mflags.FLAGS if flag.name != "PYTHON_UOPS"]
     flags = [flag.name for flag in reversed(all_flags)]
@@ -399,7 +405,7 @@ def get_comparison_value(ref, r):
     else:
         data = {}
 
-    axs: Sequence[matplotlib.Axes]
+    axs: Sequence[matplotlib.Axes]  # pyright: ignore
 
     fig, axs = plt.subplots(
         len(flags), 1, figsize=(10, 5 * len(flags)), layout="constrained"

bench_runner/result.py

@@ -30,6 +30,7 @@
 from . import plot
 from . import runners
 from . import util
+from .util import PathLike
 
 
 CombinedData = list[tuple[str, np.ndarray | None, float]]
@@ -54,7 +55,7 @@ def _clean_for_url(string: str) -> str:
     return string.replace("-", "%2d")
 
 
-def _get_platform_value(python: Path, item: str) -> str:
+def _get_platform_value(python: PathLike, item: str) -> str:
     """
     Get a value from the platform module of the given Python interpreter.
     """
@@ -64,7 +65,7 @@ def _get_platform_value(python: Path, item: str) -> str:
     return output.strip().lower()
 
 
-def _get_architecture(python: Path) -> str:
+def _get_architecture(python: PathLike) -> str:
     machine = _get_platform_value(python, "machine")
     bits = eval(_get_platform_value(python, "architecture"))[0]
     if bits == "32bit":
@@ -152,7 +153,9 @@ def _generate_contents(self) -> str:
         fd.write(f"- memory change: {self._calculate_memory_change()}")
         return fd.getvalue()
 
-    def write_table(self, filename: Path) -> str | None:
+    def write_table(self, filename: PathLike) -> str | None:
+        filename = Path(filename)
+
         entries = [
             ("fork", unquote(self.head.fork)),
             ("ref", self.head.ref),
@@ -212,7 +215,7 @@ def get_timing_diff(self) -> CombinedData:
         head_data = self.head.get_timing_data()
         return self._get_combined_data(ref_data, head_data)
 
-    def write_timing_plot(self, filename: Path) -> None:
+    def write_timing_plot(self, filename: PathLike) -> None:
         plot.plot_diff(
             self.get_timing_diff(),
             filename,
@@ -231,7 +234,7 @@ def get_memory_diff(self) -> CombinedData:
         # Explicitly reversed so higher is bigger
         return self._get_combined_data(head_data, ref_data)
 
-    def write_memory_plot(self, filename: Path) -> None:
+    def write_memory_plot(self, filename: PathLike) -> None:
         plot.plot_diff(
             self.get_memory_diff(),
             filename,
@@ -376,7 +379,9 @@ def get_files(self) -> Iterable[tuple[Callable, str, str]]:
             return
         yield (self.write_pystats_diff, ".md", "pystats diff")
 
-    def write_pystats_diff(self, filename: Path) -> None:
+    def write_pystats_diff(self, filename: PathLike) -> None:
+        filename = Path(filename)
+
         try:
             contents = subprocess.check_output(
                 [
@@ -434,7 +439,8 @@ def __init__(
         self.bases = {}
 
     @classmethod
-    def from_filename(cls, filename: Path) -> "Result":
+    def from_filename(cls, filename: PathLike) -> "Result":
+        filename = Path(filename)
         (
             name,
             _,
@@ -469,7 +475,7 @@ def from_filename(cls, filename: Path) -> "Result":
     @classmethod
     def from_scratch(
         cls,
-        python: Path,
+        python: PathLike,
         fork: str,
         ref: str,
         extra: Iterable[str] | None = None,
@@ -687,7 +693,7 @@ def memory_value(metadata):
 
 
 def has_result(
-    results_dir: Path,
+    results_dir: PathLike,
     commit_hash: str,
     machine: str,
     pystats: bool,
@@ -822,14 +828,14 @@ def track(it, *_args, **_kwargs):
 
 def load_all_results(
     bases: Sequence[str] | None,
-    results_dir: Path,
+    results_dir: PathLike,
     sorted: bool = True,
     match: bool = True,
     progress: bool = True,
 ) -> list[Result]:
     results = []
 
-    for entry in results_dir.glob("**/*.json"):
+    for entry in Path(results_dir).glob("**/*.json"):
         result = Result.from_filename(entry)
         if result.result_info[0] not in ["raw results", "pystats raw"]:
             continue