Skip to content

simplify python_version markers #851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 91 additions & 8 deletions src/poetry/core/version/markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
if TYPE_CHECKING:
from collections.abc import Callable
from collections.abc import Iterable
from collections.abc import Iterator
from collections.abc import Mapping
from collections.abc import Sequence

from lark import Tree

Expand Down Expand Up @@ -723,9 +725,21 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None:
- union between two multimarkers where there are some common markers
and the union of unique markers is a single marker
"""
from poetry.core.packages.utils.utils import get_python_constraint_from_marker

if other in self._markers:
return other

if isinstance(other, SingleMarker) and other.name in PYTHON_VERSION_MARKERS:
# Convert 'python_version >= "3.8" and sys_platform == "linux" or python_version > "3.6"'
# to 'python_version > "3.6"'
for m in self._markers:
if isinstance(m, SingleMarker) and m.name in PYTHON_VERSION_MARKERS:
constraint = get_python_constraint_from_marker(m)
other_constraint = get_python_constraint_from_marker(other)
if other_constraint.allows_all(constraint):
return other

if isinstance(other, MultiMarker):
our_markers = set(self.markers)
their_markers = set(other.markers)
Expand All @@ -746,7 +760,13 @@ def union_simplify(self, other: BaseMarker) -> BaseMarker | None:
unique_union = MultiMarker(*unique_markers).union(
MultiMarker(*other_unique_markers)
)
if isinstance(unique_union, (SingleMarkerLike, AnyMarker)):
if isinstance(unique_union, (SingleMarkerLike, AnyMarker)) or (
# Convert 'python_version >= "3.8" and python_version < "3.10"
# or python_version >= "3.10" and python_version < "3.12"'
# to 'python_version >= "3.6" and python_version < "3.12"'
isinstance(unique_union, MultiMarker)
and unique_union.complexity <= (2, 2)
):
common_markers = [
marker for marker in self.markers if marker in shared_markers
]
Expand Down Expand Up @@ -856,12 +876,19 @@ def of(cls, *markers: BaseMarker) -> BaseMarker:

# If we have a SingleMarker then with any luck after union it'll
# become another SingleMarker.
# Especially, for `python_version` markers a multi marker is also
# an improvement. E.g. the union of 'python_version == "3.6"' and
# 'python_version == "3.7" or python_version == "3.8"' is
# 'python_version >= "3.6" and python_version < "3.9"'.
if not is_one_multi and isinstance(mark, SingleMarkerLike):
new_marker = mark.union(marker)
if new_marker.is_any():
return AnyMarker()

if isinstance(new_marker, SingleMarkerLike):
if isinstance(new_marker, SingleMarkerLike) or (
isinstance(new_marker, MultiMarker)
and new_marker.complexity <= (2, 2)
):
new_markers[i] = new_marker
included = True
break
Expand Down Expand Up @@ -903,9 +930,21 @@ def intersect_simplify(self, other: BaseMarker) -> BaseMarker | None:
- intersection between two markerunions where there are some common markers
and the intersection of unique markers is not a single marker
"""
from poetry.core.packages.utils.utils import get_python_constraint_from_marker

if other in self._markers:
return other

if isinstance(other, SingleMarker) and other.name in PYTHON_VERSION_MARKERS:
# Convert '(python_version >= "3.6" or sys_platform == "linux") and python_version > "3.8"'
# to 'python_version > "3.8"'
for m in self._markers:
if isinstance(m, SingleMarker) and m.name in PYTHON_VERSION_MARKERS:
constraint = get_python_constraint_from_marker(m)
other_constraint = get_python_constraint_from_marker(other)
if constraint.allows_all(other_constraint):
return other

if isinstance(other, MarkerUnion):
our_markers = set(self.markers)
their_markers = set(other.markers)
Expand All @@ -926,7 +965,14 @@ def intersect_simplify(self, other: BaseMarker) -> BaseMarker | None:
unique_intersection = MarkerUnion(*unique_markers).intersect(
MarkerUnion(*other_unique_markers)
)
if isinstance(unique_intersection, (SingleMarkerLike, EmptyMarker)):
if isinstance(unique_intersection, (SingleMarkerLike, EmptyMarker)) or (
# Convert '(python_version == "3.6" or python_version >= "3.8)"
# and (python_version >= "3.6" and python_version < "3.8"
# or python_version == "3.9")'
# to 'python_version == "3.6" or python_version == "3.9"'
isinstance(unique_intersection, MarkerUnion)
and unique_intersection.complexity <= (2, 2)
):
common_markers = [
marker for marker in self.markers if marker in shared_markers
]
Expand Down Expand Up @@ -1081,7 +1127,7 @@ def cnf(marker: BaseMarker) -> BaseMarker:
m.markers if isinstance(m, MultiMarker) else [m] for m in cnf_markers
]
return MultiMarker.of(
*[MarkerUnion.of(*c) for c in itertools.product(*sub_marker_lists)]
*[MarkerUnion.of(*c) for c in _unique_product(*sub_marker_lists)]
)

if isinstance(marker, MultiMarker):
Expand All @@ -1099,7 +1145,7 @@ def dnf(marker: BaseMarker) -> BaseMarker:
m.markers if isinstance(m, MarkerUnion) else [m] for m in dnf_markers
]
return MarkerUnion.of(
*[MultiMarker.of(*c) for c in itertools.product(*sub_marker_lists)]
*[MultiMarker.of(*c) for c in _unique_product(*sub_marker_lists)]
)

if isinstance(marker, MarkerUnion):
Expand Down Expand Up @@ -1181,6 +1227,21 @@ def union(*markers: BaseMarker) -> BaseMarker:
return min(*candidates, key=lambda x: x.complexity)


def _unique_product(
*sub_marker_lists: Sequence[BaseMarker],
) -> Iterator[Sequence[BaseMarker]]:
"""
Returns an itertools.product of the sub_marker_lists
without duplicates (and equivalents) removed while maintaining order.
"""
unique_sets = set()
for sub_marker_list in itertools.product(*sub_marker_lists):
sub_marker_set = frozenset(sub_marker_list)
if sub_marker_set not in unique_sets:
unique_sets.add(sub_marker_set)
yield sub_marker_list


@functools.cache
def _merge_single_markers(
marker1: SingleMarkerLike[SingleMarkerConstraint],
Expand Down Expand Up @@ -1247,16 +1308,38 @@ def _merge_single_markers(
result_marker = EmptyMarker()

elif isinstance(result_constraint, VersionUnion) and merge_class == MarkerUnion:
# Convert 'python_version == "3.8" or python_version >= "3.9"'
# to 'python_version >= "3.8"'.
# Convert 'python_version <= "3.8" or python_version >= "3.9"' to "any".
result_constraint = get_python_constraint_from_marker(marker1).union(
get_python_constraint_from_marker(marker2)
)
if result_constraint.is_any():
# Convert 'python_version <= "3.8" or python_version >= "3.9"' to "any".
result_marker = AnyMarker()
elif result_constraint.is_simple():
# Convert 'python_version == "3.8" or python_version >= "3.9"'
# to 'python_version >= "3.8"'.
result_marker = SingleMarker(marker1.name, result_constraint)
elif isinstance(result_constraint, VersionRange):
# Convert 'python_version' == "3.8" or python_version == "3.9"'
# to 'python_version >= "3.8" and python_version < "3.10"'.
# Although both markers have the same complexity, the latter behaves
# better if it is merged with 'python_version == "3.10' in a next step
# for example.
result_marker = MultiMarker(
SingleMarker(
marker1.name,
VersionRange(
min=result_constraint.min,
include_min=result_constraint.include_min,
),
),
SingleMarker(
marker1.name,
VersionRange(
max=result_constraint.max,
include_max=result_constraint.include_max,
),
),
)

return result_marker

Expand Down
4 changes: 2 additions & 2 deletions tests/packages/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ def test_dependency_from_pep_508_with_python_version() -> None:
assert dep.name == "requests"
assert str(dep.constraint) == "2.18.0"
assert dep.extras == frozenset()
assert dep.python_versions == "~2.7 || ~2.6"
assert str(dep.marker) == 'python_version == "2.7" or python_version == "2.6"'
assert dep.python_versions == ">=2.6 <2.8"
assert str(dep.marker) == 'python_version >= "2.6" and python_version < "2.8"'


def test_dependency_from_pep_508_with_single_python_version() -> None:
Expand Down
2 changes: 1 addition & 1 deletion tests/packages/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
),
(
'python_version == "2.7" or python_version == "2.6"',
{"python_version": [[("==", "2.7")], [("==", "2.6")]]},
{"python_version": [[(">=", "2.6"), ("<", "2.8")]]},
),
(
(
Expand Down
Loading