Merge pull request #710 from idaholab/709-surfaces-cant-be-unset-from-is_reflecting

Fixed surfaces cant be unset from is reflecting

Author: MicahGale

doc/source/changelog.rst

@@ -37,6 +37,7 @@ MontePy Changelog
 * Fixed bug where setting a lattice would print as ``LAT=None``. Also switched ``CellModifier`` to print in the cell block by default (:issue:`699`). 
 * Fixed bug that wouldn't allow cloning most surfaces (:issue:`704`).
 * Fixed bug that crashed when some cells were not assigned to any universes (:issue:`705`).
+* Fixed bug where setting ``surf.is_reflecting`` to ``False`` did not always get exported properly (:issue:`709`).
 
 **Breaking Changes**
 

montepy/input_parser/syntax_node.py

@@ -1328,9 +1328,14 @@ def format(self):
         else:
             pad_str = ""
             extra_pad_str = ""
-        buffer = "{temp:<{value_length}}{padding}".format(
-            temp=temp, padding=pad_str, **self._formatter
-        )
+        if not self.never_pad:
+            buffer = "{temp:<{value_length}}{padding}".format(
+                temp=temp, padding=pad_str, **self._formatter
+            )
+        else:
+            buffer = "{temp}{padding}".format(
+                temp=temp, padding=pad_str, **self._formatter
+            )
         """
         If:
             1. expanded

montepy/mcnp_object.py

@@ -161,11 +161,16 @@ def __setattr__(self, key, value):
             )
 
     @staticmethod
-    def _generate_default_node(value_type: type, default, padding: str = " "):
+    def _generate_default_node(
+        value_type: type, default: str, padding: str = " ", never_pad: bool = False
+    ):
         """Generates a "default" or blank ValueNode.
 
         None is generally a safe default value to provide.
 
+        .. versionchanged:: 1.0.0
+            Added ``never_pad`` argument.
+
         Parameters
         ----------
         value_type : Class
@@ -176,6 +181,8 @@ def _generate_default_node(value_type: type, default, padding: str = " "):
         padding : str, None
             the string to provide to the PaddingNode. If None no
             PaddingNode will be added.
+        never_pad: bool
+            Whether to never add trailing padding. True means extra padding is suppressed.
 
         Returns
         -------
@@ -187,8 +194,8 @@ def _generate_default_node(value_type: type, default, padding: str = " "):
         else:
             padding_node = None
         if default is None or isinstance(default, montepy.input_parser.mcnp_input.Jump):
-            return ValueNode(default, value_type, padding_node)
-        return ValueNode(str(default), value_type, padding_node)
+            return ValueNode(default, value_type, padding_node, never_pad)
+        return ValueNode(str(default), value_type, padding_node, never_pad)
 
     @property
     def parameters(self) -> dict[str, str]:

montepy/surfaces/surface.py

@@ -64,7 +64,7 @@ def __init__(
                 elif "+" in self._number.token:
                     self._is_white_boundary = True
                     self._number._token = self._number.token.replace("+", "")
-                    self._modifier = self._generate_default_node(str, "+", None)
+                    self._modifier = self._generate_default_node(str, "+", None, True)
                     self._tree["surface_num"].nodes["modifier"] = self._modifier
             try:
                 assert self._number.value > 0
@@ -289,6 +289,8 @@ def _update_values(self):
             modifier.value = "*"
         elif self.is_white_boundary:
             modifier.value = "+"
+        else:
+            modifier.value = ""
         if self.transform is not None:
             self._old_transform_number.value = self.transform.number
             self._old_transform_number.is_negative = False

tests/test_surfaces.py

@@ -1,5 +1,7 @@
 # Copyright 2024-2025, Battelle Energy Alliance, LLC All Rights Reserved.
+import io
 from unittest import TestCase
+from pathlib import Path
 import pytest
 
 import montepy
@@ -129,26 +131,6 @@ def test_validator(self):
         with self.assertRaises(montepy.errors.IllegalState):
             surf.validate()
 
-    def test_surface_is_reflecting_setter(self):
-        in_str = "1 PZ 0.0"
-        card = Input([in_str], BlockType.SURFACE)
-        surf = Surface(card)
-        surf.is_reflecting = True
-        self.assertTrue(surf.is_reflecting)
-        self.verify_export(surf)
-        with self.assertRaises(TypeError):
-            surf.is_reflecting = 1
-
-    def test_surface_is_white_bound_setter(self):
-        in_str = "1 PZ 0.0"
-        card = Input([in_str], BlockType.SURFACE)
-        surf = Surface(card)
-        surf.is_white_boundary = True
-        self.assertTrue(surf.is_white_boundary)
-        self.verify_export(surf)
-        with self.assertRaises(TypeError):
-            surf.is_white_boundary = 1
-
     def test_surface_constants_setter(self):
         in_str = "1 PZ 0.0"
         card = Input([in_str], BlockType.SURFACE)
@@ -315,24 +297,36 @@ def test_cylinder_location_setter(self):
         with self.assertRaises(ValueError):
             surf.coordinates = [3, 4, 5]
 
-    def verify_export(_, surf):
-        output = surf.format_for_mcnp_input((6, 3, 0))
-        print("Surface output", output)
-        new_surf = Surface("\n".join(output))
-        assert surf.number == new_surf.number, "Material number not preserved."
-        assert len(surf.surface_constants) == len(
-            new_surf.surface_constants
-        ), "number of surface constants not kept."
-        for old_const, new_const in zip(
-            surf.surface_constants, new_surf.surface_constants
-        ):
-            assert old_const == pytest.approx(new_const)
-        assert surf.is_reflecting == new_surf.is_reflecting
-        assert surf.is_white_boundary == new_surf.is_white_boundary
-        if surf.old_periodic_surface:
-            assert surf.old_periodic_surface == new_surf.old_periodic_surface
-        if surf.old_transform_number:
-            assert surf.old_transform_number == new_surf._old_transform_number
+
+def verify_export(surf):
+    output = surf.format_for_mcnp_input((6, 3, 0))
+    print("Surface output", output)
+    new_surf = Surface("\n".join(output))
+    verify_equiv_surf(surf, new_surf)
+
+
+def verify_equiv_surf(surf, new_surf):
+    assert surf.number == new_surf.number, "Material number not preserved."
+    assert len(surf.surface_constants) == len(
+        new_surf.surface_constants
+    ), "number of surface constants not kept."
+    for old_const, new_const in zip(surf.surface_constants, new_surf.surface_constants):
+        assert old_const == pytest.approx(new_const)
+    assert surf.is_reflecting == new_surf.is_reflecting
+    assert surf.is_white_boundary == new_surf.is_white_boundary
+    if surf.old_periodic_surface:
+        assert surf.old_periodic_surface == new_surf.old_periodic_surface
+    if surf.old_transform_number:
+        assert surf.old_transform_number == new_surf._old_transform_number
+
+
+def verify_prob_export(problem, surf):
+    with io.StringIO() as fh:
+        problem.write_problem(fh)
+        fh.seek(0)
+        new_problem = montepy.read_input(fh)
+    new_surf = new_problem.surfaces[surf.number]
+    verify_equiv_surf(surf, new_surf)
 
 
 @pytest.mark.parametrize(
@@ -345,3 +339,41 @@ def test_surface_clone(surf_str):
     new_surf = surf.clone()
     assert surf.surface_type == new_surf.surface_type
     assert surf.surface_constants == new_surf.surface_constants
+
+
+@pytest.fixture
+def simple_problem(scope="module"):
+    return montepy.read_input(Path("tests") / "inputs" / "test.imcnp")
+
+
+@pytest.fixture
+def cp_simple_problem(simple_problem):
+    return simple_problem.clone()
+
+
+@pytest.mark.parametrize(
+    "in_str, expected",
+    [("1 PZ 0.0", True), ("*1 PZ 0.0", False), ("    *1 PZ 0.0", False)],
+)
+def test_surface_is_reflecting_setter(cp_simple_problem, in_str, expected):
+    surf = Surface(in_str)
+    surf.is_reflecting = expected
+    assert surf.is_reflecting == expected
+    cp_simple_problem.surfaces.append(surf)
+    verify_prob_export(cp_simple_problem, surf)
+    with pytest.raises(TypeError):
+        surf.is_reflecting = 1
+
+
+@pytest.mark.parametrize(
+    "in_str, expected",
+    [("1 PZ 0.0", True), ("+1 PZ 0.0", False), ("    +1 PZ 0.0", False)],
+)
+def test_surface_is_white_bound_setter(cp_simple_problem, in_str, expected):
+    surf = Surface(in_str)
+    surf.is_white_boundary = expected
+    assert surf.is_white_boundary == expected
+    cp_simple_problem.surfaces.append(surf)
+    verify_prob_export(cp_simple_problem, surf)
+    with pytest.raises(TypeError):
+        surf.is_white_boundary = 1