Responding to comments

Mostly, this commit reinstates the `restorePreviousGeometry` method, which
was remove for some reason. But also some changes were made to docstrings.

armi/reactor/converters/geometryConverters.py

@@ -1353,6 +1353,17 @@ def convert(self, r=None):
             geometry.DomainType.FULL_CORE, geometry.BoundaryType.NO_SYMMETRY
         )
 
+    def restorePreviousGeometry(self, cs, reactor):
+        """Undo the changes made by convert by going back to 1/3 core."""
+        # remove the assemblies that were added when the conversion happened.
+        if bool(self.getNewAssembliesAdded()):
+            for a in self.getNewAssembliesAdded():
+                reactor.core.removeAssembly(a, discharge=False)
+
+            reactor.core.symmetry = geometry.SymmetryType.fromAny(
+                self.EXPECTED_INPUT_SYMMETRY
+            )
+
 
 class EdgeAssemblyChanger(GeometryChanger):
     """

armi/reactor/converters/tests/test_geometryConverters.py

@@ -295,6 +295,15 @@ def test_growToFullCoreFromThirdCore(self):
         # Check the full core conversion is successful
         self.assertGreater(len(self.r.core.getBlocks()), initialNumBlocks)
         self.assertEqual(self.r.core.symmetry.domain, geometry.DomainType.FULL_CORE)
+        # Check that the geometry can be restored to a third core
+        changer.restorePreviousGeometry(self.o.cs, self.r)
+        self.assertEqual(initialNumBlocks, len(self.r.core.getBlocks()))
+        self.assertEqual(
+            self.r.core.symmetry,
+            geometry.SymmetryType(
+                geometry.DomainType.THIRD_CORE, geometry.BoundaryType.PERIODIC
+            ),
+        )
 
     def test_skipGrowToFullCoreWhenAlreadyFullCore(self):
         """Test that hex core is not modified when third core to full core changer is called on an already full core geometry."""
@@ -315,6 +324,9 @@ def test_skipGrowToFullCoreWhenAlreadyFullCore(self):
         changer.convert(self.r)
         self.assertEqual(self.r.core.symmetry.domain, geometry.DomainType.FULL_CORE)
         self.assertEqual(initialNumBlocks, len(self.r.core.getBlocks()))
+        changer.restorePreviousGeometry(self.o.cs, self.r)
+        self.assertEqual(initialNumBlocks, len(self.r.core.getBlocks()))
+        self.assertEqual(self.r.core.symmetry.domain, geometry.DomainType.FULL_CORE)
 
 
 if __name__ == "__main__":

armi/reactor/tests/test_reactors.py

@@ -528,6 +528,12 @@ def test_countAssemblies(self):
         nFuel = self.r.core.countAssemblies(Flags.FUEL, fullCore=True)
         self.assertEqual(6, nFuel)
 
+    def test_restoreReactor(self):
+        aListLength = len(self.r.core.getAssemblies())
+        converter = self.r.core.growToFullCore(self.o.cs)
+        converter.restorePreviousGeometry(self.o.cs, self.r)
+        self.assertEqual(aListLength, len(self.r.core.getAssemblies()))
+
     def test_differentNuclideModels(self):
         self.assertEqual(self.o.cs["xsKernel"], "MC2v3")
         _o2, r2 = loadTestReactor(customSettings={"xsKernel": "MC2v2"})

armi/settings/caseSettings.py

@@ -80,7 +80,7 @@ def __init__(self, fName=None):
         """
         self.path = ""
 
-        app = armi.getApp()  # TODO: Is this ideal?
+        app = armi.getApp()
         assert app is not None
         self.__settings = app.getSettings()
         if not Settings.instance:
@@ -91,7 +91,7 @@ def __init__(self, fName=None):
 
     @property
     def inputDirectory(self):
-        if not self.path:
+        if self.path is None:
             return os.getcwd()
         else:
             return os.path.dirname(self.path)
@@ -134,7 +134,11 @@ def __getitem__(self, key):
             raise NonexistentSetting(key)
 
     def get_setting(self, key):
-        """This helper method exists for when users need the entire setting object, not just the value"""
+        """
+        Return a copy of an actual Setting object, instead of just its value.
+
+        NOTE: This is used very rarely, try to organize your code to only need a Setting value.
+        """
         if key in self.__settings:
             return deepcopy(self.__settings[key])
         else:
@@ -158,8 +162,7 @@ def __setstate__(self, state):
         --------
         armi.settings.setting.Setting.__getstate__ : removes schema
         """
-        # TODO: This potentially allows for invisible settings mutations and should be removed.
-        self.__settings = armi.getApp().getSettings()  # TODO: Is this ideal?
+        self.__settings = armi.getApp().getSettings()
 
         # restore non-setting instance attrs
         for key, val in state.items():
@@ -329,9 +332,7 @@ def updateEnvironmentSettingsFrom(self, otherCs):
             self[replacement] = otherCs[replacement]
 
     def modified(self, caseTitle=None, newSettings=None):
-        """Helper method to return a new settings object, like this one
-        This method exists to help keep settings immutable.
-        """
+        """Return a new Settings object containing the provided modifications."""
         settings = self.duplicate()
 
         if caseTitle: