Repository: idaholab/MontePy

Issue:
Consider implicit parentheses operating on existing regions
## Is your feature request related to a problem? Please describe.

Order of operations is important for Boolean logic. When operating on existing regions (combinations of halfspaces), treat them as being surrounded by implicit parentheses. The present behavior chains a new halfspace to the end.

## To Reproduce

<!-- A short code snippet of what you have ran. Please change or remove any specific values or anything that can't be public. For example: --> 

``` python
>>> import montepy
>>> c = montepy.Cell(number=10)
>>> c.geometry = +montepy.Surface(number=1) | +montepy.Surface(number=2)
>>> c.mcnp_str()
'10 0 1 : 2 '
>>> c.geometry &= -montepy.Surface(number=3)
>>> c.mcnp_str()
'10 0 1 : 2 -3 '
```


## Describe the solution you'd like

When the operation on an existing region would be different if it were surrounded by parentheses, assume that they exist.

```python
>>> c.geometry  = +montepy.Surface(number=1) | +montepy.Surface(number=2)
>>> c.geometry &= -montepy.Surface(number=3)
>>> c.mcnp_str()
'10 0 (1 : 2) -3 '
```

## Describe alternatives you've considered

Always add parens around regions. This would violate priority 4 (make it pretty) to ensure priority 1 (make it right). This is what OpenMC does:

```python
import openmc
>>> reg  = +openmc.XPlane(surface_id=1) | +openmc.YPlane(surface_id=2)
>>> reg &= -openmc.ZPlane(surface_id=3)
>>> print(reg)
((1 | 2) -3)
>>> 
```


## Additional context

The order of operations is:

1. Complement
2. Intersection
3. Union

Discussion:

Fix (patch):
diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst
index 3b805cbd..d98b7c4d 100644
--- a/doc/source/changelog.rst
+++ b/doc/source/changelog.rst
@@ -5,6 +5,13 @@ MontePy Changelog
 1.2 releases
 ============
 
+#Next_Version#
+--------------
+
+**Bugs Fixed**
+
+* Fixed a bug where ``&=`` and ``|=`` for geometry definitions (``HalfSpace`` ) were not handled operator precedence properly (:issue:`879`).
+
 1.2.0
 --------------
 
diff --git a/montepy/surfaces/half_space.py b/montepy/surfaces/half_space.py
index 9717b7eb..41524752 100644
--- a/montepy/surfaces/half_space.py
+++ b/montepy/surfaces/half_space.py
@@ -399,38 +399,24 @@ def __switch_operator(self, new_symbol):
     def __iand__(self, other):
         if not isinstance(other, HalfSpace):
             raise TypeError(f"Right hand side must be HalfSpace. {other} given.")
+        self._add_new_children_to_cell(other)
         if isinstance(self, UnitHalfSpace):
             return self & other
-        right_leaf = (
-            self.right is not None and isinstance(self.right, UnitHalfSpace)
-        ) or self.right is None
-        if right_leaf:
-            if self.right is not None:
-                self.right = self.right & other
-                return self
-            else:
-                return (~self.left) & other
-        self.right &= other
-        self._add_new_children_to_cell(other)
-        return self
+        if self.right is None:
+            # if this is a complement operator delete self
+            return (~self.left) & other
+        return self & other
 
     def __ior__(self, other):
         if not isinstance(other, HalfSpace):
             raise TypeError(f"Right hand side must be HalfSpace. {other} given.")
+        self._add_new_children_to_cell(other)
         if isinstance(self, UnitHalfSpace):
             return self | other
-        right_leaf = (
-            self.right is not None and isinstance(self.right, UnitHalfSpace)
-        ) or self.right is None
-        if right_leaf:
-            if self.right is not None:
-                self.right = self.right | other
-                return self
-            else:
-                return (~self.left) | other
-        self.right |= other
-        self._add_new_children_to_cell(other)
-        return self
+        if self.right is None:
+            # if this is a complement operator delete self
+            return (~self.left) | other
+        return self | other
 
     def __and__(self, other):
         if not isinstance(other, HalfSpace):


Test changes:
diff --git a/tests/test_geom_integration.py b/tests/test_geom_integration.py
index 655458e3..87b52114 100644
--- a/tests/test_geom_integration.py
+++ b/tests/test_geom_integration.py
@@ -53,3 +53,16 @@ def test_geom_invalid():
         +surf
     with pytest.raises(montepy.exceptions.IllegalState):
         ~montepy.Cell()
+
+
+def test_geom_recursive_iand_formatting():
+    cell = montepy.Cell(number=1)
+    surf = montepy.AxisPlane(number=1)
+    cell.geometry = +surf | -surf
+    cell.geometry &= +surf
+    assert cell.mcnp_str() == "1 0 (1 : -1) 1 "
+    cell = montepy.Cell(number=1)
+    surf = montepy.AxisPlane(number=1)
+    cell.geometry = +surf & -surf
+    cell.geometry |= +surf
+    assert cell.mcnp_str() == "1 0 1 -1 : 1 "
diff --git a/tests/test_geometry.py b/tests/test_geometry.py
index 21c30a10..0d90d477 100644
--- a/tests/test_geometry.py
+++ b/tests/test_geometry.py
@@ -247,10 +247,12 @@ def test_iand_recursion():
     half_space = ~cell1 & ~cell2
     cell3.geometry = half_space
     half_space &= ~cell1
-    assert half_space.left == ~cell1
+    # lazy tree enforcement
+    assert str(half_space) == "((#1*#2)*#1)"
+    assert half_space.right == ~cell1
     assert not isinstance(half_space.right, UnitHalfSpace)
-    assert half_space.right.left == ~cell2
-    assert half_space.right.right == ~cell1
+    assert half_space.left.left == ~cell1
+    assert half_space.left.right == ~cell2
     assert half_space.operator == Operator.INTERSECTION
     assert len(half_space) == 3
     for i in range(1, 100):
@@ -265,10 +267,13 @@ def test_iand_recursion():
     half_space = -surf
     half_space &= +surf
     assert len(half_space) == 2
+    assert str(half_space) == "(-5*+5)"
     # test with actual tree
     half_space = -surf | +surf
     half_space &= +surf
     assert len(half_space) == 3
+    assert half_space.operator == Operator.INTERSECTION
+    assert str(half_space) == "((-5:+5)*+5)"
 
 
 def test_ior_recursion():
@@ -280,10 +285,11 @@ def test_ior_recursion():
     cell2 = ~cell2
     half_space = cell1 | cell2
     half_space |= cell1
-    assert half_space.left is cell1
-    assert not isinstance(half_space.right, UnitHalfSpace)
-    assert half_space.right.left == cell2
-    assert half_space.right.right == cell1
+    assert str(half_space) == "((#1:#2):#1)"
+    assert half_space.right is cell1
+    assert not isinstance(half_space.left, UnitHalfSpace)
+    assert half_space.left.right == cell2
+    assert half_space.left.left == cell1
     assert half_space.operator == Operator.UNION
     assert len(half_space) == 3
     for i in range(1, 100):
@@ -297,9 +303,17 @@ def test_ior_recursion():
     half_space = -surf
     half_space |= +surf
     assert len(half_space) == 2
+    assert str(half_space) == "(-5:+5)"
     half_space = -surf | +surf
     half_space |= +surf
     assert len(half_space) == 3
+    assert str(half_space) == "((-5:+5):+5)"
+    # test with complex tree
+    half_space = -surf & +surf
+    half_space |= +surf
+    assert len(half_space) == 3
+    assert half_space.operator == Operator.UNION
+    assert str(half_space) == "((-5*+5):+5)"
 
 
 def test_parse_input_tree():