Skip to content

Commit 19c6294

Browse files
sirosenwebknjaz
andcommitted
Apply various suggestions from code review
- Minor fixups and adjustments - Update several docstrings to PEP 257 style, with an imperative description as the first line - Simplify `rewrite_comes_from` by making it always a callable Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
1 parent 8f3cba7 commit 19c6294

2 files changed

Lines changed: 45 additions & 38 deletions

File tree

piptools/_compat/pip_compat.py

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,12 @@ def parse_requirements(
8787
comes_from_stdin: bool = False,
8888
) -> Iterator[InstallRequirement]:
8989
# the `comes_from` data will be rewritten in different ways in different conditions
90-
# each rewrite rule is expressible as a str->str function or as a static replacement
91-
rewrite_comes_from: Callable[[str], str] | str
90+
# each rewrite rule is expressible as a str->str function
91+
rewrite_comes_from: Callable[[str], str]
9292

9393
if comes_from_stdin:
9494
# if data is coming from stdin, then `comes_from="-r -"`
95-
rewrite_comes_from = "-r -"
95+
rewrite_comes_from = _rewrite_comes_from_to_hardcoded_stdin_value
9696
elif pathlib.Path(filename).is_absolute():
9797
# if the input path is absolute, just normalize paths to posix-style
9898
rewrite_comes_from = _normalize_comes_from_location
@@ -113,28 +113,32 @@ def parse_requirements(
113113
install_req.link = file_link
114114
install_req = copy_install_requirement(install_req)
115115

116-
if isinstance(rewrite_comes_from, str):
117-
install_req.comes_from = rewrite_comes_from
118-
else:
119-
install_req.comes_from = rewrite_comes_from(install_req.comes_from)
116+
install_req.comes_from = rewrite_comes_from(install_req.comes_from)
120117

121118
yield install_req
122119

123120

121+
def _rewrite_comes_from_to_hardcoded_stdin_value(_: str, /) -> str:
122+
"""Produce the hardcoded ``comes_from`` value for stdin."""
123+
return "-r -"
124+
125+
124126
def _relativize_comes_from_location(original_comes_from: str, /) -> str:
125127
"""
128+
Convert a ``comes_from`` path to a relative posix path.
129+
126130
This is the rewrite rule used when ``-r`` or ``-c`` appears in
127131
``comes_from`` data with an absolute path.
128132
129-
The ``-r`` or ``-c`` qualifier is retained, and the path is relativized
130-
with respect to the CWD.
133+
The ``-r`` or ``-c`` qualifier is retained, the path is relativized
134+
with respect to the CWD, and the path is converted to posix style.
131135
"""
132136
# require `-r` or `-c` as the source
133137
if not original_comes_from.startswith(("-r ", "-c ")):
134138
return original_comes_from
135139

136140
# split on the space
137-
prefix, _, suffix = original_comes_from.partition(" ")
141+
prefix, space_sep, suffix = original_comes_from.partition(" ")
138142

139143
file_path = pathlib.Path(suffix)
140144

@@ -144,26 +148,30 @@ def _relativize_comes_from_location(original_comes_from: str, /) -> str:
144148

145149
# make it relative to the current working dir
146150
suffix = file_path.relative_to(pathlib.Path.cwd()).as_posix()
147-
return f"{prefix} {suffix}"
151+
return f"{prefix}{space_sep}{suffix}"
148152

149153

150154
def _normalize_comes_from_location(original_comes_from: str, /) -> str:
151155
"""
156+
Convert a ``comes_from`` path to a posix-style path.
157+
152158
This is the rewrite rule when ``-r`` or ``-c`` appears in ``comes_from``
153-
data and the input path was absolute, meaning we should not relativize the locations.
159+
data and the input path was absolute, meaning we should not relativize the
160+
locations.
154161
155-
Instead, we apply minimal normalization, ensuring that posix-style paths are used.
162+
The ``-r`` or ``-c`` qualifier is retained, and the path is converted to
163+
posix style.
156164
"""
157165
# require `-r` or `-c` as the source
158166
if not original_comes_from.startswith(("-r ", "-c ")):
159167
return original_comes_from
160168

161169
# split on the space
162-
prefix, _, suffix = original_comes_from.partition(" ")
170+
prefix, space_sep, suffix = original_comes_from.partition(" ")
163171

164172
# convert to a posix-style path
165173
suffix = pathlib.Path(suffix).as_posix()
166-
return f"{prefix} {suffix}"
174+
return f"{prefix}{space_sep}{suffix}"
167175

168176

169177
def create_wheel_cache(cache_dir: str, format_control: str | None = None) -> WheelCache:

tests/test_cli_compile.py

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ def populate(self, tmp_path: pathlib.Path) -> None:
8484
def get_path_to(self, filename: str) -> str:
8585
"""Given a filename, find the (first) path to that filename in the contents."""
8686
return next(
87-
k
88-
for k in self.contents.keys()
89-
if (k == filename) or k.endswith(f"/{filename}")
87+
stub_file_path
88+
for stub_file_path in self.contents
89+
if (stub_file_path == filename) or stub_file_path.endswith(f"/{filename}")
9090
)
9191

9292

@@ -3891,7 +3891,9 @@ def test_stdout_should_not_be_read_when_stdin_is_not_a_plain_file(
38913891
assert out.exit_code == 0, out
38923892

38933893

3894-
@pytest.mark.parametrize("input_path_absolute", (True, False))
3894+
@pytest.mark.parametrize(
3895+
"input_path_absolute", (True, False), ids=("absolute-input", "relative-input")
3896+
)
38953897
@pytest.mark.parametrize(
38963898
"test_files_collection",
38973899
(
@@ -3922,23 +3924,22 @@ def test_second_order_requirements_path_handling(
39223924
test_files_collection,
39233925
):
39243926
"""
3925-
Given nested requirements files, the internal requirements will be written
3926-
in the output, and it will be absolute or relative depending only on
3927-
whether or not the initial path was absolute or relative.
3927+
Test normalization of ``-r`` includes in output.
3928+
3929+
Given nested requirements files, the internal requirements file path will
3930+
be written in the output, and it will be absolute or relative depending
3931+
only on whether or not the initial path was absolute or relative.
39283932
"""
39293933
test_files_collection.populate(tmp_path)
39303934

39313935
# the input path is given on the CLI as absolute or relative
39323936
# and this determines the expected output path as well
3933-
if input_path_absolute:
3934-
input_path = (tmp_path / "requirements.in").as_posix()
3935-
output_path = (tmp_path / "requirements2.in").as_posix()
3936-
else:
3937-
input_path = "requirements.in"
3938-
output_path = "requirements2.in"
3937+
input_dir_path = tmp_path if input_path_absolute else pathlib.Path(".")
3938+
input_path = (input_dir_path / "requirements.in").as_posix()
3939+
output_path = (input_dir_path / "requirements2.in").as_posix()
39393940

3940-
with monkeypatch.context() as m:
3941-
m.chdir(tmp_path)
3941+
with monkeypatch.context() as revertable_ctx:
3942+
revertable_ctx.chdir(tmp_path)
39423943

39433944
out = runner.invoke(
39443945
cli,
@@ -3998,13 +3999,11 @@ def test_second_order_requirements_relative_path_in_separate_dir(
39983999
pip_produces_absolute_paths,
39994000
):
40004001
"""
4001-
As in the above test, nested requirements file path handling.
4002-
But in this case, the primary variable is the relative location of the two
4003-
requirements.
4002+
Test normalization of ``-r`` includes when the requirements files are in
4003+
distinct directories.
40044004
4005-
The expectation is that the output path will be relative to the current
4006-
working directory, *not* relative to the dir containing the initial
4007-
requirements file.
4005+
Confirm that the output path will be relative to the current working
4006+
directory.
40084007
"""
40094008
test_files_collection.populate(tmp_path)
40104009
# the input is the path to 'requirements.in' relative to the starting dir
@@ -4021,8 +4020,8 @@ def test_second_order_requirements_relative_path_in_separate_dir(
40214020
pathlib.Path(input_path).parent / ("../" * relative_segments) / output_path
40224021
).as_posix()
40234022

4024-
with monkeypatch.context() as m:
4025-
m.chdir(tmp_path)
4023+
with monkeypatch.context() as revertable_ctx:
4024+
revertable_ctx.chdir(tmp_path)
40264025
out = runner.invoke(
40274026
cli,
40284027
[

0 commit comments

Comments
 (0)