Add writer style option to SettingsWriter pt2 : Propagate to additional methods (#932)

* Add settingsWriteStyle arg to case and suite cloning CLI/modules

* WIP add unit tests

* Docstring fixes

* Fix for using clone with medium write style

The medium write style requires clone to pass in knowledge about the
"from" location, not just the "to" location, so that the original
settings can be read in. 

* Add a test for clone

* Adding a little more to writeInputs test

* Release notes

* Missed a commit in interactive rebase...this fixes what was missing

* Address reviewer comment: add comments to explain expected test results

* Address reviewer comments: More descriptive docstrings, and moving a line under the if statement it pertains to

Author: opotowsky

armi/cases/case.py

@@ -601,7 +601,13 @@ def buildCommand(self, python="python"):
 
         return command
 
-    def clone(self, additionalFiles=None, title=None, modifiedSettings=None):
+    def clone(
+        self,
+        additionalFiles=None,
+        title=None,
+        modifiedSettings=None,
+        writeStyle="short",
+    ):
         """
         Clone existing ARMI inputs to current directory with optional settings modifications.
 
@@ -616,6 +622,9 @@ def clone(self, additionalFiles=None, title=None, modifiedSettings=None):
             title of new case
         modifiedSettings : dict (optional)
             settings to set/modify before creating the cloned case
+        writeStyle : str (optional)
+            Writing style for which settings get written back to the settings files
+            (short, medium, or full).
 
         Raises
         ------
@@ -644,7 +653,7 @@ def clone(self, additionalFiles=None, title=None, modifiedSettings=None):
         clone.cs = newCs
 
         runLog.important("writing settings file {}".format(clone.cs.path))
-        clone.cs.writeToYamlFile(clone.cs.path)
+        clone.cs.writeToYamlFile(clone.cs.path, style=writeStyle, fromFile=self.cs.path)
         runLog.important("finished writing {}".format(clone.cs))
 
         fromPath = lambda fname: pathTools.armiAbsPath(self.cs.inputDirectory, fname)
@@ -732,7 +741,9 @@ def compare(
 
         return code
 
-    def writeInputs(self, sourceDir: Optional[str] = None):
+    def writeInputs(
+        self, sourceDir: Optional[str] = None, writeStyle: Optional[str] = "short"
+    ):
         """
         Write the inputs to disk.
 
@@ -742,9 +753,12 @@ def writeInputs(self, sourceDir: Optional[str] = None):
 
         Parameters
         ----------
-        sourceDir : str, optional
+        sourceDir : str (optional)
             The path to copy inputs from (if different from the cs.path). Needed
             in SuiteBuilder cases to find the baseline inputs from plugins (e.g. shuffleLogic)
+        writeStyle : str (optional)
+            Writing style for which settings get written back to the settings files
+            (short, medium, or full).
 
         Notes
         -----
@@ -789,7 +803,13 @@ def writeInputs(self, sourceDir: Optional[str] = None):
                 newSettings[settingName] = value
 
             self.cs = self.cs.modified(newSettings=newSettings)
-            self.cs.writeToYamlFile(self.title + ".yaml")
+            if sourceDir:
+                fromPath = os.path.join(sourceDir, self.title + ".yaml")
+            else:
+                fromPath = self.cs.path
+            self.cs.writeToYamlFile(
+                self.title + ".yaml", style=writeStyle, fromFile=fromPath
+            )
 
 
 def _copyInputsHelper(
@@ -807,13 +827,10 @@ def _copyInputsHelper(
     ----------
     fileDescription : str
         A file description for the copyOrWarn method
-
     sourcePath : str
         The absolute file path of the file to copy
-
     destPath : str
         The target directory to copy input files to
-
     origFile : str
         File path as defined in the original settings file
 
@@ -857,11 +874,9 @@ def copyInterfaceInputs(
     ----------
     cs : CaseSettings
         The source case settings to find input files
-
     destination : str
         The target directory to copy input files to
-
-    sourceDir : str, optional
+    sourceDir : str (optional)
         The directory from which to copy files. Defaults to cs.inputDirectory
 
     Returns

armi/cases/suite.py

@@ -146,7 +146,7 @@ def echoConfiguration(self):
             )
         )
 
-    def clone(self, oldRoot=None):
+    def clone(self, oldRoot=None, writeStyle="short"):
         """
         Clone a CaseSuite to a new place.
 
@@ -160,6 +160,9 @@ def clone(self, oldRoot=None):
         oldRoot : str (optional)
             root directory of original case suite used to help filter when a suite
             contains one or more cases with the same case title.
+        writeStyle : str (optional)
+            Writing style for which settings get written back to the settings files
+            (short, medium, or full).
 
         Notes
         -----
@@ -185,7 +188,9 @@ def clone(self, oldRoot=None):
             with directoryChangers.ForcedCreationDirectoryChanger(
                 newDir, dumpOnException=False
             ):
-                clone.add(case.clone(modifiedSettings=modifiedSettings))
+                clone.add(
+                    case.clone(modifiedSettings=modifiedSettings, writeStyle=writeStyle)
+                )
         return clone
 
     def run(self):
@@ -266,17 +271,21 @@ def compare(
 
         return nIssues
 
-    def writeInputs(self):
+    def writeInputs(self, writeStyle="short"):
         """
         Write inputs for all cases in the suite.
 
+        writeStyle : str (optional)
+            Writing style for which settings get written back to the settings files
+            (short, medium, or full).
+
         See Also
         --------
         clone
             Similar to this but doesn't let you write out new geometry or blueprints objects.
         """
         for case in self:
-            case.writeInputs(sourceDir=self.cs.inputDirectory)
+            case.writeInputs(sourceDir=self.cs.inputDirectory, writeStyle=writeStyle)
 
     @staticmethod
     def writeTable(tableResults):

armi/cases/tests/test_cases.py

@@ -212,6 +212,44 @@ def test_run(self):
                 self.assertIn("xsGroups", mock._outputStream)
                 self.assertIn("Completed EveryNode - cycle 0", mock._outputStream)
 
+    def test_clone(self):
+        testTitle = "CLONE_TEST"
+        # test the short write style
+        with directoryChangers.TemporaryDirectoryChanger():
+            cs = settings.Settings(ARMI_RUN_PATH)
+            setMasterCs(cs)
+            case = cases.Case(cs)
+            shortCase = case.clone(
+                additionalFiles=["ISOAA"],
+                title=testTitle,
+                modifiedSettings={"verbosity": "important"},
+            )
+            # Check additional files made it
+            self.assertTrue(os.path.exists("ISOAA"))
+            # Check title change made it
+            clonedYaml = testTitle + ".yaml"
+            self.assertTrue(os.path.exists(clonedYaml))
+            self.assertTrue(shortCase.title, testTitle)
+            # Check on some expected settings
+            # Availability factor is in the original settings file but since it is a
+            # default value, gets removed for the write-out
+            txt = open(clonedYaml, "r").read()
+            self.assertNotIn("availabilityFactor", txt)
+            self.assertIn("verbosity: important", txt)
+
+        # test the medium write style
+        with directoryChangers.TemporaryDirectoryChanger():
+            cs = settings.Settings(ARMI_RUN_PATH)
+            setMasterCs(cs)
+            case = cases.Case(cs)
+            case.clone(writeStyle="medium")
+            clonedYaml = "armiRun.yaml"
+            self.assertTrue(os.path.exists(clonedYaml))
+            # Availability factor is in the original settings file and it is a default
+            # value. While "short" (default writing style) removes, "medium" should not
+            txt = open(clonedYaml, "r").read()
+            self.assertIn("availabilityFactor", txt)
+
 
 class TestCaseSuiteDependencies(unittest.TestCase):
     """CaseSuite tests"""
@@ -370,6 +408,19 @@ def test_writeInput(self):
             case = baseCase.clone()
             case.writeInputs()
             self.assertTrue(os.path.exists(cs["shuffleLogic"]))
+            # Availability factor is in the original settings file but since it is a
+            # default value, gets removed for the write-out
+            txt = open("armiRun.yaml", "r").read()
+            self.assertNotIn("availabilityFactor", txt)
+            self.assertIn("armiRun-blueprints.yaml", txt)
+
+        with directoryChangers.TemporaryDirectoryChanger():
+            case = baseCase.clone(writeStyle="medium")
+            case.writeInputs(writeStyle="medium")
+            # Availability factor is in the original settings file and it is a default
+            # value. While "short" (default writing style) removes, "medium" should not
+            txt = open("armiRun.yaml", "r").read()
+            self.assertIn("availabilityFactor", txt)
 
 
 class MultiFilesInterfaces(interfaces.Interface):

armi/cli/clone.py

@@ -34,6 +34,13 @@ def addOptions(self):
             default=[],
             help="Additional files from the source directory to copy into the target directory",
         )
+        self.parser.add_argument(
+            "--settingsWriteStyle",
+            type=str,
+            default="short",
+            help="Writing style for which settings get written back to the settings files.",
+            choices=["short", "medium", "full"],
+        )
         # somehow running `armi clone-batch -h` on the command line requires this to
         # not be first?
         for settingName in self.cs.keys():
@@ -44,7 +51,10 @@ def invoke(self):
         from armi import cases
 
         inputCase = cases.Case(cs=self.cs)
-        inputCase.clone(additionalFiles=self.args.additional_files)
+        inputCase.clone(
+            additionalFiles=self.args.additional_files,
+            writeStyle=self.args.settingsWriteStyle,
+        )
 
 
 class CloneArmiRunCommandInteractive(CloneArmiRunCommandBatch):
@@ -95,6 +105,13 @@ def addOptions(self):
             default=False,
             help="Just list the settings files found, don't actually submit them.",
         )
+        self.parser.add_argument(
+            "--settingsWriteStyle",
+            type=str,
+            default="short",
+            help="Writing style for which settings get written back to the settings files.",
+            choices=["short", "medium", "full"],
+        )
 
     def invoke(self):
         from armi import cases
@@ -105,4 +122,6 @@ def invoke(self):
             rootDir=self.args.directory,
             ignorePatterns=self.args.ignore,
         )
-        suite.clone(oldRoot=self.args.directory)
+        suite.clone(
+            oldRoot=self.args.directory, writeStyle=self.args.settingsWriteStyle
+        )

armi/cli/tests/test_runEntryPoint.py

@@ -97,21 +97,65 @@ class TestCloneArmiRunCommandBatch(unittest.TestCase):
     def test_cloneArmiRunCommandBatchBasics(self):
         ca = CloneArmiRunCommandBatch()
         ca.addOptions()
-        ca.parse_args([ARMI_RUN_PATH, "--additional-files", "test"])
+        ca.parse_args(
+            [
+                ARMI_RUN_PATH,
+                "--additional-files",
+                "test",
+                "--settingsWriteStyle",
+                "full",
+            ]
+        )
 
         self.assertEqual(ca.name, "clone-batch")
         self.assertEqual(ca.settingsArgument, "required")
         self.assertEqual(ca.args.additional_files, ["test"])
+        self.assertEqual(ca.args.settingsWriteStyle, "full")
+
+    def test_cloneArmiRunCommandBatchInvokeShort(self):
+        # Test short write style
+        ca = CloneArmiRunCommandBatch()
+        ca.addOptions()
+        ca.parse_args([ARMI_RUN_PATH])
+
+        with TemporaryDirectoryChanger():
+            ca.invoke()
+
+            self.assertEqual(ca.settingsArgument, "required")
+            self.assertEqual(ca.args.settingsWriteStyle, "short")
+            clonedYaml = "armiRun.yaml"
+            self.assertTrue(os.path.exists(clonedYaml))
+            # validate a setting that has a default value was removed
+            txt = open(clonedYaml, "r").read()
+            self.assertNotIn("availabilityFactor", txt)
+
+    def test_cloneArmiRunCommandBatchInvokeMedium(self):
+        # Test medium write style
+        ca = CloneArmiRunCommandBatch()
+        ca.addOptions()
+        ca.parse_args([ARMI_RUN_PATH, "--settingsWriteStyle", "medium"])
+
+        with TemporaryDirectoryChanger():
+            ca.invoke()
+
+            self.assertEqual(ca.settingsArgument, "required")
+            self.assertEqual(ca.args.settingsWriteStyle, "medium")
+            clonedYaml = "armiRun.yaml"
+            self.assertTrue(os.path.exists(clonedYaml))
+            # validate a setting that has a  default value is still there
+            txt = open(clonedYaml, "r").read()
+            self.assertIn("availabilityFactor", txt)
 
 
 class TestCloneSuiteCommand(unittest.TestCase):
     def test_cloneSuiteCommandBasics(self):
         cs = CloneSuiteCommand()
         cs.addOptions()
-        cs.parse_args(["-d", "test"])
+        cs.parse_args(["-d", "test", "--settingsWriteStyle", "medium"])
 
         self.assertEqual(cs.name, "clone-suite")
         self.assertEqual(cs.args.directory, "test")
+        self.assertEqual(cs.args.settingsWriteStyle, "medium")
 
 
 class TestCompareCases(unittest.TestCase):

armi/settings/caseSettings.py

@@ -352,7 +352,7 @@ def initLogVerbosity(self):
 
         self.setModuleVerbosities(force=True)
 
-    def writeToYamlFile(self, fName, style="short"):
+    def writeToYamlFile(self, fName, style="short", fromFile=None):
         """
         Write settings to a yaml file.
 
@@ -366,11 +366,17 @@ def writeToYamlFile(self, fName, style="short"):
             the file to write to
         style : str (optional)
             the method of output to be used when creating the file for the current
-            state of settings
+            state of settings (short, medium, or full)
+        fromFile : str (optional)
+            if the source file and destination file are different (i.e. for cloning)
+            and the style argument is ``medium``, then this arg is used
         """
         self.path = pathTools.armiAbsPath(fName)
         if style == "medium":
-            settingsSetByUser = self.getSettingsSetByUser(self.path)
+            getSettingsPath = (
+                self.path if fromFile is None else pathTools.armiAbsPath(fromFile)
+            )
+            settingsSetByUser = self.getSettingsSetByUser(getSettingsPath)
         else:
             settingsSetByUser = []
         with open(self.path, "w") as stream:

doc/release/0.2.rst

@@ -11,7 +11,7 @@ What's new in ARMI
 
 #. TBD
 #. Cleanup of stale ``coveragerc`` file (`PR#923 <https://github.com/terrapower/armi/pull/923>`_)
-#. Added writer style option to ``SettingsWriter`` and added it as arg to modify CLI (`PR#924 <https://github.com/terrapower/armi/pull/924>`_)
+#. Added `medium` writer style option to ``SettingsWriter``. Added it as arg to modify CLI (`PR#924 <https://github.com/terrapower/armi/pull/924>`_), and to clone CLI (`PR#932 <https://github.com/terrapower/armi/pull/932>`_).
 #. Update the EntryPoint class to provide user feedback on required positional arguments (`PR#922 <https://github.com/terrapower/armi/pull/922>`_)
 
 Bug fixes