From 50299ab7bc1f40d97ce2b7a72b2792b210f8348c Mon Sep 17 00:00:00 2001 From: antazoey Date: Fri, 1 Nov 2024 17:59:34 -0500 Subject: [PATCH] perf: use `p1.relative(p2)` when it makes sense (#2369) --- src/ape/managers/project.py | 6 ++---- src/ape/pytest/coverage.py | 4 ++-- src/ape/utils/os.py | 12 ++++++------ src/ape_pm/compiler.py | 3 +-- tests/functional/test_compilers.py | 2 +- tests/functional/utils/test_os.py | 15 --------------- 6 files changed, 12 insertions(+), 30 deletions(-) diff --git a/src/ape/managers/project.py b/src/ape/managers/project.py index 2f6f658945..1b154a573c 100644 --- a/src/ape/managers/project.py +++ b/src/ape/managers/project.py @@ -43,7 +43,7 @@ def _path_to_source_id(path: Path, root_path: Path) -> str: - return f"{get_relative_path(path.absolute(), root_path.absolute())}" + return f"{path.relative_to(root_path)}" class SourceManager(BaseManager): @@ -495,9 +495,7 @@ def _compile( ): self._compile_contracts(needs_compile) - src_ids = [ - f"{get_relative_path(Path(p).absolute(), self.project.path)}" for p in path_ls_final - ] + src_ids = [f"{Path(p).relative_to(self.project.path)}" for p in path_ls_final] for contract_type in (self.project.manifest.contract_types or {}).values(): if contract_type.source_id and contract_type.source_id in src_ids: yield ContractContainer(contract_type) diff --git a/src/ape/pytest/coverage.py b/src/ape/pytest/coverage.py index 768c385492..eebe47c670 100644 --- a/src/ape/pytest/coverage.py +++ b/src/ape/pytest/coverage.py @@ -7,7 +7,7 @@ from ape.logging import logger from ape.utils.basemodel import ManagerAccessMixin from ape.utils.misc import get_current_timestamp_ms -from ape.utils.os import get_full_extension, get_relative_path +from ape.utils.os import get_full_extension from ape.utils.trace import parse_coverage_tables if TYPE_CHECKING: @@ -92,7 +92,7 @@ def cover( self, src_path: Path, pcs: Iterable[int], inc_fn_hits: bool = True ) -> tuple[set[int], list[str]]: if hasattr(self.project, "path"): - source_id = str(get_relative_path(src_path.absolute(), self.project.path)) + source_id = f"{src_path.relative_to(self.project.path)}" else: source_id = str(src_path) diff --git a/src/ape/utils/os.py b/src/ape/utils/os.py index f66d9736c1..bc6e3f37dd 100644 --- a/src/ape/utils/os.py +++ b/src/ape/utils/os.py @@ -34,7 +34,12 @@ def get_relative_path(target: Path, anchor: Path) -> Path: Compute the relative path of ``target`` relative to ``anchor``, which may or may not share a common ancestor. - **NOTE**: Both paths must be absolute. + **NOTE ON PERFORMANCE**: Both paths must be absolute to + use this method. If you know both methods are absolute, + this method is a performance boost. If you have to first + call `.absolute()` on the paths, use + `target.relative_to(anchor)` instead; as it will be + faster in that case. Args: target (pathlib.Path): The path we are interested in. @@ -43,11 +48,6 @@ def get_relative_path(target: Path, anchor: Path) -> Path: Returns: pathlib.Path: The new path to the target path from the anchor path. """ - if not target.is_absolute(): - raise ValueError("'target' must be an absolute path.") - if not anchor.is_absolute(): - raise ValueError("'anchor' must be an absolute path.") - # Calculate common prefix length common_parts = 0 for target_part, anchor_part in zip(target.parts, anchor.parts): diff --git a/src/ape_pm/compiler.py b/src/ape_pm/compiler.py index ebd5ea97ee..98a4d5c377 100644 --- a/src/ape_pm/compiler.py +++ b/src/ape_pm/compiler.py @@ -11,7 +11,6 @@ from ape.api.compiler import CompilerAPI from ape.exceptions import CompilerError, ContractLogicError from ape.logging import logger -from ape.utils.os import get_relative_path if TYPE_CHECKING: from ape.managers.project import ProjectManager @@ -41,7 +40,7 @@ def compile( ) -> Iterator[ContractType]: project = project or self.local_project source_ids = { - p: f"{get_relative_path(p, project.path.absolute())}" if p.is_absolute() else str(p) + p: f"{p.relative_to(project.path)}" if p.is_absolute() else str(p) for p in contract_filepaths } logger.info(f"Compiling {', '.join(source_ids.values())}.") diff --git a/tests/functional/test_compilers.py b/tests/functional/test_compilers.py index 58db5f44b8..5e9ed7ac2c 100644 --- a/tests/functional/test_compilers.py +++ b/tests/functional/test_compilers.py @@ -77,7 +77,7 @@ def test_compile(compilers, project_with_contract, factory): Testing both stringified paths and path-object paths. """ path = next(iter(project_with_contract.sources.paths)) - actual = compilers.compile((factory(path),)) + actual = compilers.compile((factory(path),), project=project_with_contract) contract_name = path.stem assert contract_name in [x.name for x in actual] diff --git a/tests/functional/utils/test_os.py b/tests/functional/utils/test_os.py index 0acca31efc..5aa5c34785 100644 --- a/tests/functional/utils/test_os.py +++ b/tests/functional/utils/test_os.py @@ -23,21 +23,6 @@ def test_get_relative_path_from_project(): assert actual == expected -def test_get_relative_path_given_relative_path(): - relative_script_path = Path("../deploy.py") - with pytest.raises(ValueError) as err: - get_relative_path(relative_script_path, _TEST_DIRECTORY_PATH) - - assert str(err.value) == "'target' must be an absolute path." - - relative_project_path = Path("../This/is/a/test") - - with pytest.raises(ValueError) as err: - get_relative_path(_TEST_FILE_PATH, relative_project_path) - - assert str(err.value) == "'anchor' must be an absolute path." - - def test_get_relative_path_same_path(): actual = get_relative_path(_TEST_FILE_PATH, _TEST_FILE_PATH) assert actual == Path()