From 64c853ce4391e7392b87bd13f8d443ce8d3da9db Mon Sep 17 00:00:00 2001 From: Warren Date: Fri, 8 Sep 2023 15:24:45 +1000 Subject: [PATCH] refactor: use division operator to join paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Starting with `resolve_package_path` and its associated tests, this refactoring seeks to make path concatenation more readable and consistent. To be consistent, all uses of `.joinpath()` need to be replaced with the division operator or the other way around. I have chosen to standardise the use of the division operator because 1) it is already used in the tests 2) it is more succinct and readable 3) casual profiling with `%timeit` didn't reveal a performance loss division operator: 273 µs ± 86.6 µs `.joinpath()`: 269 µs ± 99.4 µs Eventual consistency within and across all files is the ultimate goal. This is not unachievable. There are currently 428 uses of `.joinpath()` across 45 files. Counting assignment expressions only, there are 111 uses of the division operator across 23 files. This is fewer. However, even if there is a usage pattern of 4:1 against the division operator, I do not think that makes standardising it an unwise choice. --- src/_pytest/pathlib.py | 2 +- testing/test_pathlib.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 63b1345b4..68372f67c 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -680,7 +680,7 @@ def resolve_package_path(path: Path) -> Optional[Path]: result = None for parent in itertools.chain((path,), path.parents): if parent.is_dir(): - if not parent.joinpath("__init__.py").is_file(): + if not (parent / "__init__.py").is_file(): break if not parent.name.isidentifier(): break diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 678fd27fe..50c1d5967 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -345,18 +345,18 @@ def test_resolve_package_path(tmp_path: Path) -> None: (pkg / "subdir").mkdir() (pkg / "subdir/__init__.py").touch() assert resolve_package_path(pkg) == pkg - assert resolve_package_path(pkg.joinpath("subdir", "__init__.py")) == pkg + assert resolve_package_path(pkg / "subdir/__init__.py") == pkg def test_package_unimportable(tmp_path: Path) -> None: pkg = tmp_path / "pkg1-1" pkg.mkdir() pkg.joinpath("__init__.py").touch() - subdir = pkg.joinpath("subdir") + subdir = pkg / "subdir" subdir.mkdir() - pkg.joinpath("subdir/__init__.py").touch() + (pkg / "subdir/__init__.py").touch() assert resolve_package_path(subdir) == subdir - xyz = subdir.joinpath("xyz.py") + xyz = subdir / "xyz.py" xyz.touch() assert resolve_package_path(xyz) == subdir assert not resolve_package_path(pkg)