Skip to content

Commit

Permalink
perf: add support for AtomicMultiMarker and AtomicMarkerUnion for ext…
Browse files Browse the repository at this point in the history 
…ra markers
  • Loading branch information
@radoering
radoering committed Jan 18, 2025
1 parent 6f376eb commit 5eb4a64
Show file tree
Hide file tree
Showing 4 changed files with 230 additions and 21 deletions.
9 changes: 6 additions & 3 deletions src/poetry/core/packages/dependency.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,12 @@ def marker(self, marker: str | BaseMarker) -> None:
for op, extra in or_:
if op == "==":
new_in_extras.append(canonicalize_name(extra))
elif op == "" and "||" in extra:
for _extra in extra.split(" || "):
new_in_extras.append(canonicalize_name(_extra))
elif op == "" and ("||" in extra or "," in extra):
sep = "||" if "||" in extra else ","
extra_values = [e.strip() for e in extra.split(sep)]
for _extra in extra_values:
if not _extra.startswith("!="):
new_in_extras.append(canonicalize_name(_extra))
self._in_extras = [*self._in_extras, *new_in_extras]

# Recalculate python versions.
Expand Down
50 changes: 32 additions & 18 deletions src/poetry/core/version/markers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,7 @@ def __init__(self, name: str, constraint: SingleMarkerConstraint) -> None:
from poetry.core.constraints.generic import (
parse_constraint as parse_generic_constraint,
)
from poetry.core.constraints.generic import parse_extra_constraint
from poetry.core.constraints.version import parse_marker_version_constraint

self._name = ALIASES.get(name, name)
Expand All @@ -242,6 +243,8 @@ def __init__(self, name: str, constraint: SingleMarkerConstraint) -> None:
self._parser = functools.partial(
parse_marker_version_constraint, pep440=name != "platform_release"
)
elif name == "extra":
self._parser = parse_extra_constraint
else:
self._parser = parse_generic_constraint

Expand Down Expand Up @@ -358,6 +361,7 @@ def __init__(
from poetry.core.constraints.generic import (
parse_constraint as parse_generic_constraint,
)
from poetry.core.constraints.generic import parse_extra_constraint
from poetry.core.constraints.version import parse_marker_version_constraint

parsed_constraint: BaseConstraint | VersionConstraint
Expand All @@ -381,7 +385,7 @@ def __init__(
self._operator = "=="

self._value = m.group("value")
parser = parse_generic_constraint
parser = parse_extra_constraint if name == "extra" else parse_generic_constraint

if swapped_name_value and name not in PYTHON_VERSION_MARKERS:
# Something like `"tegra" in platform_release`
Expand Down Expand Up @@ -516,14 +520,21 @@ def __str__(self) -> str:

class AtomicMultiMarker(SingleMarkerLike[MultiConstraint]):
def __init__(self, name: str, constraint: MultiConstraint) -> None:
assert all(c.operator == "!=" for c in constraint.constraints)
assert all(
c.operator in ({"==", "!="} if name == "extra" else {"!="})
for c in constraint.constraints
)
super().__init__(name, constraint)
self._values: list[str] = []

@property
def complexity(self) -> tuple[int, int]:
return len(self._constraint.constraints), 1

def validate(self, environment: Mapping[str, Any] | None) -> bool:
if self._name == "extra":
return self.expand().validate(environment)
return super().validate(environment)

def invert(self) -> BaseMarker:
return AtomicMarkerUnion(self._name, self._constraint.invert())

Expand All @@ -534,14 +545,16 @@ def expand(self) -> MultiMarker:

def __str__(self) -> str:
return " and ".join(
f'{self._name} != "{c.value}"' for c in self._constraint.constraints
f'{self._name} {c.operator} "{c.value}"'
for c in self._constraint.constraints
)


class AtomicMarkerUnion(SingleMarkerLike[UnionConstraint]):
def __init__(self, name: str, constraint: UnionConstraint) -> None:
assert all(
isinstance(c, Constraint) and c.operator == "=="
isinstance(c, Constraint)
and c.operator in ({"==", "!="} if name == "extra" else {"=="})
for c in constraint.constraints
)
super().__init__(name, constraint)
Expand All @@ -550,6 +563,11 @@ def __init__(self, name: str, constraint: UnionConstraint) -> None:
def complexity(self) -> tuple[int, int]:
return len(self._constraint.constraints), 1

def validate(self, environment: Mapping[str, Any] | None) -> bool:
if self._name == "extra":
return self.expand().validate(environment)
return super().validate(environment)

def invert(self) -> BaseMarker:
return AtomicMultiMarker(self._name, self._constraint.invert())

Expand All @@ -563,7 +581,7 @@ def __str__(self) -> str:
# contains only elements of type Constraint (instead of BaseConstraint)
# but mypy can't see that.
return " or ".join(
f'{self._name} == "{c.value}"' # type: ignore[attr-defined]
f'{self._name} {c.operator} "{c.value}"' # type: ignore[attr-defined]
for c in self._constraint.constraints
)

Expand Down Expand Up @@ -755,7 +773,7 @@ def __hash__(self) -> int:
def __str__(self) -> str:
elements = []
for m in self._markers:
if isinstance(m, (SingleMarker, MultiMarker)):
if isinstance(m, (SingleMarker, MultiMarker, AtomicMultiMarker)):
elements.append(str(m))
else:
elements.append(f"({m})")
Expand Down Expand Up @@ -1009,7 +1027,9 @@ def _compact_markers(
groups.append([])

# Combine the groups.
sub_markers = [MultiMarker(*group) for group in groups]
sub_markers = [
group[0] if len(group) == 1 else MultiMarker(*group) for group in groups
]

# This function calls itself recursively. In the inner calls we don't perform any
# simplification, instead doing it all only when we have the complete marker.
Expand Down Expand Up @@ -1094,14 +1114,6 @@ def _merge_single_markers(
if marker1.name != marker2.name:
return None

# "extra" is special because it can have multiple values at the same time.
# That's why we can only merge two "extra" markers if they have the same value.
if marker1.name == "extra":
assert isinstance(marker1, SingleMarker)
assert isinstance(marker2, SingleMarker)
if marker1.value != marker2.value: # type: ignore[attr-defined]
return None

if merge_class == MultiMarker:
merge_method = marker1.constraint.intersect
else:
Expand All @@ -1125,12 +1137,14 @@ def _merge_single_markers(
):
result_marker = SingleMarker(marker1.name, result_constraint)
elif isinstance(result_constraint, UnionConstraint) and all(
isinstance(c, Constraint) and c.operator == "=="
isinstance(c, Constraint)
and c.operator in ({"==", "!="} if marker1.name == "extra" else {"=="})
for c in result_constraint.constraints
):
result_marker = AtomicMarkerUnion(marker1.name, result_constraint)
elif isinstance(result_constraint, MultiConstraint) and all(
c.operator == "!=" for c in result_constraint.constraints
c.operator in ({"==", "!="} if marker1.name == "extra" else {"!="})
for c in result_constraint.constraints
):
result_marker = AtomicMultiMarker(marker1.name, result_constraint)
return result_marker
Expand Down
10 changes: 10 additions & 0 deletions tests/packages/test_main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ def test_dependency_from_pep_508_with_extras() -> None:
assert str(dep.marker) == 'extra == "foo" or extra == "bar"'


def test_dependency_from_pep_508_with_extra_and_inverse_extra() -> None:
name = 'requests==2.18.0; extra != "foo" and extra == "bar"'
dep = Dependency.create_from_pep_508(name)

assert dep.name == "requests"
assert str(dep.constraint) == "2.18.0"
assert dep.in_extras == ["bar"]
assert str(dep.marker) == 'extra != "foo" and extra == "bar"'


def test_dependency_from_pep_508_with_python_version() -> None:
name = 'requests (==2.18.0); python_version == "2.7" or python_version == "2.6"'
dep = Dependency.create_from_pep_508(name)
Expand Down
182 changes: 182 additions & 0 deletions tests/version/test_markers.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,12 @@
'extra == "a" and extra != "b"',
'extra != "a" and extra == "b"',
'extra != "a" and extra != "b"',
'extra == "a" and extra == "b" and extra != "c" and extra != "d"',
'extra == "a" or extra == "b"',
'extra == "a" or extra != "b"',
'extra != "a" or extra == "b"',
'extra != "a" or extra != "b"',
'extra == "a" or extra == "b" or extra != "c" or extra != "d"',
# String comparison markers
'"tegra" in platform_release',
'"tegra" not in platform_release',
Expand Down Expand Up @@ -250,6 +252,88 @@ def test_single_marker_not_in_python_intersection() -> None:
('extra == "a"', 'extra != "b"', 'extra == "a" and extra != "b"'),
('extra != "a"', 'extra == "b"', 'extra != "a" and extra == "b"'),
('extra != "a"', 'extra != "b"', 'extra != "a" and extra != "b"'),
# AtomicMultiMarker
(
'extra == "a" and extra == "b"',
'extra == "c"',
'extra == "a" and extra == "b" and extra == "c"',
),
(
'extra != "a" and extra != "b"',
'extra != "c"',
'extra != "a" and extra != "b" and extra != "c"',
),
(
'extra != "a" and extra != "b"',
'extra == "c"',
'extra != "a" and extra != "b" and extra == "c"',
),
(
'extra == "a" and extra == "b"',
'extra != "c"',
'extra == "a" and extra == "b" and extra != "c"',
),
(
'extra == "a" and extra == "b"',
'extra == "a" and extra == "b"',
'extra == "a" and extra == "b"',
),
(
'extra == "a" and extra == "b"',
'extra == "c" and extra != "d"',
'extra == "a" and extra == "b" and extra == "c" and extra != "d"',
),
('extra != "a" and extra != "b"', 'extra == "a"', "<empty>"),
('extra != "a" and extra == "b"', 'extra == "a" and extra == "c"', "<empty>"),
(
'extra != "a" and extra != "b"',
'extra != "a"',
'extra != "a" and extra != "b"',
),
(
'extra == "a" and extra != "b"',
'extra == "a"',
'extra == "a" and extra != "b"',
),
# AtomicMarkerUnion
(
'extra == "a" or extra == "b"',
'extra == "c"',
'(extra == "a" or extra == "b") and extra == "c"',
),
(
'extra == "a" or extra == "b"',
'extra != "c"',
'(extra == "a" or extra == "b") and extra != "c"',
),
('extra == "a" or extra == "b"', 'extra == "a"', 'extra == "a"'),
('extra != "a" or extra == "b"', 'extra != "a"', 'extra != "a"'),
(
'extra == "a" or extra == "b"',
'extra != "a"',
'extra == "b" and extra != "a"',
),
(
'extra == "a" or extra == "b"',
'extra == "a" or extra == "b"',
'extra == "a" or extra == "b"',
),
(
'extra == "a" or extra == "b"',
'extra == "a" or extra != "c"',
'(extra == "a" or extra == "b") and (extra == "a" or extra != "c")',
),
# AtomicMultiMarker and AtomicMarkerUnion
(
'extra != "a" and extra != "b"',
'extra == "a" or extra == "b"',
"<empty>",
),
(
'extra != "a" and extra != "b"',
'extra == "a" or extra == "c"',
'extra == "c" and extra != "a" and extra != "b"',
),
],
)
def test_single_marker_intersect_extras(
Expand Down Expand Up @@ -443,6 +527,104 @@ def test_single_marker_union_with_inverse() -> None:
('extra == "a"', 'extra != "b"', 'extra == "a" or extra != "b"'),
('extra != "a"', 'extra == "b"', 'extra != "a" or extra == "b"'),
('extra != "a"', 'extra != "b"', 'extra != "a" or extra != "b"'),
# AtomicMultiMarker
(
'extra == "a" and extra == "b"',
'extra == "c"',
'extra == "a" and extra == "b" or extra == "c"',
),
(
'extra != "a" and extra != "b"',
'extra != "c"',
'extra != "a" and extra != "b" or extra != "c"',
),
(
'extra != "a" and extra != "b"',
'extra == "c"',
'extra != "a" and extra != "b" or extra == "c"',
),
(
'extra == "a" and extra == "b"',
'extra != "c"',
'extra == "a" and extra == "b" or extra != "c"',
),
(
'extra == "a" and extra == "b"',
'extra == "a" and extra == "b"',
'extra == "a" and extra == "b"',
),
(
'extra == "a" and extra == "b"',
'extra == "c" and extra != "d"',
'extra == "a" and extra == "b" or extra == "c" and extra != "d"',
),
(
'extra != "a" and extra != "b"',
'extra == "a"',
'extra != "b" or extra == "a"',
),
(
'extra != "a" and extra == "b"',
'extra == "a" and extra == "c"',
'extra != "a" and extra == "b" or extra == "a" and extra == "c"',
),
(
'extra != "a" and extra != "b"',
'extra != "a"',
'extra != "a"',
),
(
'extra == "a" and extra != "b"',
'extra == "a"',
'extra == "a"',
),
# AtomicMarkerUnion
(
'extra == "a" or extra == "b"',
'extra == "c"',
'extra == "a" or extra == "b" or extra == "c"',
),
(
'extra == "a" or extra == "b"',
'extra != "c"',
'extra == "a" or extra == "b" or extra != "c"',
),
(
'extra == "a" or extra == "b"',
'extra == "a"',
'extra == "a" or extra == "b"',
),
(
'extra != "a" or extra == "b"',
'extra != "a"',
'extra != "a" or extra == "b"',
),
(
'extra == "a" or extra == "b"',
'extra != "a"',
"",
),
(
'extra == "a" or extra == "b"',
'extra == "a" or extra == "b"',
'extra == "a" or extra == "b"',
),
(
'extra == "a" or extra == "b"',
'extra == "a" or extra != "c"',
'extra == "a" or extra == "b" or extra != "c"',
),
# AtomicMultiMarker and AtomicMarkerUnion
(
'extra != "a" and extra != "b"',
'extra == "a" or extra == "b"',
'extra != "a" and extra != "b" or extra == "a" or extra == "b"',
),
(
'extra != "a" and extra != "b"',
'extra == "a" or extra == "c"',
'extra != "a" and extra != "b" or extra == "a" or extra == "c"',
),
],
)
def test_single_marker_union_extras(marker1: str, marker2: str, expected: str) -> None:
Expand Down

0 comments on commit 5eb4a64

Please sign in to comment.