code review

don't get too hung up on symmetry for union and intersection
python-poetry · Nov 27, 2022 · 885d36e · 885d36e
1 parent 38231e2
commit 885d36e
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 17 deletions.
diff --git a/src/poetry/core/version/markers.py b/src/poetry/core/version/markers.py
@@ -435,9 +435,6 @@ def intersect(self, other: BaseMarker) -> BaseMarker:
         return dnf(multi)
 
     def union(self, other: BaseMarker) -> BaseMarker:
-        if isinstance(other, MarkerUnion):
-            return other.union(self)
-
         union = MarkerUnion(self, other)
         conjunction = cnf(union)
         if not isinstance(conjunction, MultiMarker):
@@ -633,9 +630,6 @@ def append(self, marker: BaseMarker) -> None:
         self._markers.append(marker)
 
     def intersect(self, other: BaseMarker) -> BaseMarker:
-        if isinstance(other, MultiMarker):
-            return other.intersect(self)
-
         multi = MultiMarker(self, other)
         return dnf(multi)
 

diff --git a/tests/version/test_markers.py b/tests/version/test_markers.py
@@ -565,14 +565,21 @@ def test_version_ranges_collapse_on_union(
 
 
 def test_multi_marker_union_with_union() -> None:
-    m = parse_marker('sys_platform == "darwin" and implementation_name == "cpython"')
+    m1 = parse_marker('sys_platform == "darwin" and implementation_name == "cpython"')
+    m2 = parse_marker('python_version >= "3.6" or os_name == "Windows"')
 
-    union = m.union(parse_marker('python_version >= "3.6" or os_name == "Windows"'))
-    assert (
-        str(union)
-        == 'python_version >= "3.6" or os_name == "Windows"'
-        ' or sys_platform == "darwin" and implementation_name == "cpython"'
+    # Union isn't _quite_ symmetrical.
+    expected1 = (
+        'sys_platform == "darwin" and implementation_name == "cpython" or'
+        ' python_version >= "3.6" or os_name == "Windows"'
     )
+    assert str(m1.union(m2)) == expected1
+
+    expected2 = (
+        'python_version >= "3.6" or os_name == "Windows" or'
+        ' sys_platform == "darwin" and implementation_name == "cpython"'
+    )
+    assert str(m2.union(m1)) == expected2
 
 
 def test_multi_marker_union_with_multi_union_is_single_marker() -> None:
@@ -676,17 +683,24 @@ def test_marker_union_intersect_multi_marker() -> None:
     m1 = parse_marker('sys_platform == "darwin" or python_version < "3.4"')
     m2 = parse_marker('implementation_name == "cpython" and os_name == "Windows"')
 
-    expected = (
+    # Intersection isn't _quite_ symmetrical.
+    expected1 = (
+        'sys_platform == "darwin" and implementation_name == "cpython" and os_name =='
+        ' "Windows" or python_version < "3.4" and implementation_name == "cpython" and'
+        ' os_name == "Windows"'
+    )
+
+    intersection = m1.intersect(m2)
+    assert str(intersection) == expected1
+
+    expected2 = (
         'implementation_name == "cpython" and os_name == "Windows" and sys_platform'
         ' == "darwin" or implementation_name == "cpython" and os_name == "Windows"'
         ' and python_version < "3.4"'
     )
 
-    intersection = m1.intersect(m2)
-    assert str(intersection) == expected
-
     intersection = m2.intersect(m1)
-    assert str(intersection) == expected
+    assert str(intersection) == expected2
 
 
 def test_marker_union_union_with_union() -> None: