Fixing broken returns (#834)

Many of return statements have the potential bug where some if-then statement drops off the end, and it returns nothing instead of something. Presumably, these bugs are present but wildly rare, or we would be seeing them. Still, this change fixes all those potential bugs.

armi/bookkeeping/db/factory.py

@@ -41,7 +41,6 @@ def databaseFactory(dbName: str, permission: str, version: Optional[str] = None)
     interrogate the type of the returned object to figure out to do based on whatever it
     needs.
     """
-
     import h5py
 
     dbPath = pathlib.Path(dbName)
@@ -89,3 +88,5 @@ def databaseFactory(dbName: str, permission: str, version: Optional[str] = None)
             )
         if majorversion == "3":
             return Database3(dbPath, permission)
+
+    return None

armi/bookkeeping/newReportUtils.py

@@ -47,7 +47,6 @@ def insertGeneralReportContent(cs, r, report, stage):
         r : reactor
         report : ReportContents object
         blueprint : blueprint
-
     """
     # These items only happen once at BOL
     if stage == newReports.ReportStage.Begin:
@@ -447,6 +446,7 @@ def getPinDesignTable(core):
     """
     designInfo = collections.defaultdict(list)
 
+    tableRows = newReports.Table("Pin Design", header=None)
     try:
         for b in core.getBlocks(Flags.FUEL):
             fuel = b.getComponent(Flags.FUEL)
@@ -478,7 +478,6 @@ def getPinDesignTable(core):
 
         # assumption made that all lists contain only numerical data
         designInfo = {key: numpy.average(data) for key, data in designInfo.items()}
-        tableRows = newReports.Table("Pin Design", header=None)
         dimensionless = {"sd", "hot sd", "zrFrac", "nPins"}
         for key, average_value in designInfo.items():
             dim = "{0:10s}".format(key)
@@ -495,12 +494,12 @@ def getPinDesignTable(core):
         tableRows.addRow(
             ["Plenum Height (cm):", "{0:.2f}".format(a.getHeight(Flags.PLENUM))]
         )
-
-        return tableRows
     except Exception as error:  # pylint: disable=broad-except
         runLog.warning("Pin summarization failed to work")
         runLog.warning(error)
 
+    return tableRows
+
 
 def insertAreaFractionsReport(block, report):
     """

armi/bookkeeping/report/data.py

@@ -77,6 +77,7 @@ def __getitem__(self, group):
             runLog.warning(
                 "Cannot locate group {} in report {}".format(group.title, self.title)
             )
+            return None
 
     def writeHTML(self):
         """Renders this report as a standalone HTML file"""
@@ -92,7 +93,6 @@ def writeGroupsHTML(self, f):
         """A helper method to the writeHTML method process
 
         Composes the group html content, intended for use in the midst of the html document generation
-
         """
         Image.count = 0  # reset the count for this report's figure enumeration
 
@@ -178,6 +178,8 @@ def __getitem__(self, name):
                 "Given name {} not present in report group {}".format(name, self.title)
             )
 
+        return None
+
     def __setitem__(self, name, value):
         self.data[name] = value
 

armi/bookkeeping/report/tests/test_report.py

@@ -60,6 +60,19 @@ def test_setData(self):
         self.assertEqual(filled_instance["banana_2"], ["sundae", "vanilla"])
         self.assertEqual(filled_instance["banana_3"], ["sundae", "chocolate"])
 
+    def test_getData(self):
+        # test the null case
+        self.assertIsNone(self.test_group["fake"])
+
+        # insert some data
+        self.test_group["banana_1"] = ["sundae", "plain"]
+
+        # validate we can pull that data back out again
+        data = self.test_group["banana_1"]
+        self.assertEqual(len(data), 2)
+        self.assertIn("sundae", data)
+        self.assertIn("plain", data)
+
     def test_reactorSpecificReporting(self):
         """Test a number of reporting utils that require reactor/core information"""
         o, r = loadTestReactor()

armi/cli/database.py

@@ -134,7 +134,7 @@ def invoke(self):
                 runLog.error("`{}` already exists. Aborting.".format(path))
                 bail = True
         if bail:
-            return -1
+            return
 
         for path, data, inp in [
             (settingsPath, settings, "settings"),
@@ -184,7 +184,7 @@ def invoke(self):
                 "No settings, blueprints, or geometry files specified; "
                 "nothing to do."
             )
-            return -1
+            return
 
         bp = None
         settings = None

armi/context.py

@@ -142,7 +142,7 @@ def setMode(cls, mode):
         except OSError as e:
             pass
     if not os.path.isdir(APP_DATA):
-        raise e
+        raise OSError("Directory doesn't exist {0}".format(APP_DATA))
 
 if MPI_COMM is not None:
     MPI_COMM.barrier()  # make sure app data exists before workers proceed.

armi/materials/zr.py

@@ -130,11 +130,12 @@ def linearExpansionPercent(self, Tk=None, Tc=None):
         Tk = getTk(Tc, Tk)
         self.checkPropertyTempRange("linear expansion percent", Tk)
 
-        if Tk >= 293 and Tk < 1137:
+        # NOTE: checkPropertyTempRange takes care of lower/upper limits
+        if Tk < 1137:
             return (
                 -0.111 + (2.325e-4 * Tk) + (5.595e-7 * Tk ** 2) - (1.768e-10 * Tk ** 3)
             )
-        elif Tk >= 1137:
+        else:
             return (
                 -0.759 + (1.474e-3 * Tk) - (5.140e-7 * Tk ** 2) + (1.559e-10 * Tk ** 3)
             )

armi/mpiActions.py

@@ -637,7 +637,7 @@ def _distributeInterfaces(self):
                     iNew = self.broadcast(None)
                     runLog.debug("Received {0}".format(iNew))
                     if iNew == "quit":
-                        return True  # signal the operator to break.
+                        return
                     self.o.removeInterface(iOld)
                     self.o.addInterface(iNew)
                     iNew.interactDistributeState()  # pylint: disable=no-member

armi/nucDirectory/nucDir.py

@@ -478,7 +478,7 @@ def isHeavyMetal(name):
     try:
         return getNuclide(name).isHeavyMetal()
     except AttributeError:
-        AttributeError(
+        raise AttributeError(
             "The nuclide {0} is not found in the nuclide directory".format(name)
         )
 
@@ -487,7 +487,7 @@ def isFissile(name):
     try:
         return getNuclide(name).isFissile()
     except AttributeError:
-        AttributeError(
+        raise AttributeError(
             "The nuclide {0} is not found in the nuclide directory".format(name)
         )
 

armi/nucDirectory/nuclideBases.py

@@ -481,7 +481,7 @@ def __readRiplDecayData():
 
     riplPath = os.environ.get(_riplEnvironVariable, None)
     if riplPath is None:
-        return None
+        return
 
     path = pathlib.Path(riplPath)
     if not path.exists() or not path.is_dir():
@@ -817,12 +817,13 @@ def getDecay(self, decayType):
         Returns
         -------
         decay : :py:class:`DecayModes <armi.nucDirectory.transmutations.DecayMode>`
-
         """
         for d in self.decays:
             if d.type == decayType:
                 return d
 
+        return None
+
     def isFissile(self):
         r"""Determine if the nuclide is fissile.
 

armi/nucDirectory/tests/test_nuclideBases.py

@@ -273,6 +273,11 @@ def test_nucBases_isHeavyMetal(self):
         for nb in nuclideBases.where(lambda nn: nn.z > 89):
             self.assertTrue(nb.isHeavyMetal())
 
+    def test_getDecay(self):
+        nb = list(nuclideBases.where(lambda nn: nn.z == 89))[0]
+        # This test is a bit boring, because the test nuclide library is a bit boring.
+        self.assertIsNone(nb.getDecay("sf"))
+
     def test_getEndfMatNum(self):
         self.assertEqual(nuclideBases.byName["U235"].getEndfMatNum(), "9228")
         self.assertEqual(nuclideBases.byName["U238"].getEndfMatNum(), "9237")

armi/plugins.py

@@ -120,7 +120,7 @@
    user settings. This also predated the plugin infrastructure, and may one day be
    replaced with plugin-based fuel handler logic.
 """
-from typing import Dict, Union, Callable, TYPE_CHECKING
+from typing import Callable, Dict, List, Union, TYPE_CHECKING
 
 import pluggy
 
@@ -141,13 +141,12 @@
 class ArmiPlugin:
     """
     An ArmiPlugin provides a namespace to collect hook implementations provided by a
-    single "plugin". This API is incomplete, unstable, and expected to change
-    dramatically!
+    single "plugin". This API is incomplete, unstable, and expected to change.
     """
 
     @staticmethod
     @HOOKSPEC
-    def exposeInterfaces(cs):
+    def exposeInterfaces(cs) -> List:
         """
         Function for exposing interface(s) to other code.
 
@@ -167,7 +166,7 @@ def exposeInterfaces(cs):
 
     @staticmethod
     @HOOKSPEC
-    def defineParameters():
+    def defineParameters() -> Dict:
         """
         Function for defining additional parameters.
 
@@ -198,7 +197,7 @@ def defineParameters():
 
     @staticmethod
     @HOOKSPEC
-    def afterConstructionOfAssemblies(assemblies, cs):
+    def afterConstructionOfAssemblies(assemblies, cs) -> None:
         """
         Function to call after a set of assemblies are constructed.
 
@@ -214,12 +213,11 @@ def afterConstructionOfAssemblies(assemblies, cs):
         Returns
         -------
         None
-
         """
 
     @staticmethod
     @HOOKSPEC
-    def onProcessCoreLoading(core, cs):
+    def onProcessCoreLoading(core, cs) -> None:
         """
         Function to call whenever a Core object is newly built.
 
@@ -258,7 +256,7 @@ def defineFlags() -> Dict[str, Union[int, flags.auto]]:
 
     @staticmethod
     @HOOKSPEC
-    def defineBlockTypes():
+    def defineBlockTypes() -> List:
         """
         Function for providing novel Block types from a plugin.
 
@@ -267,12 +265,15 @@ def defineBlockTypes():
         corresponding ``Component`` type that should activate it. For instance a
         ``HexBlock`` would be created when the largest component is a ``Hexagon``::
 
-            return [(Hexagon, HexBlock)]
+        Returns
+        -------
+        list
+            [(Hexagon, HexBlock)]
         """
 
     @staticmethod
     @HOOKSPEC
-    def defineAssemblyTypes():
+    def defineAssemblyTypes() -> List:
         """
         Function for providing novel Assembly types from a plugin.
 
@@ -289,11 +290,24 @@ def defineAssemblyTypes():
             probably be better suited elsewhere. Moving this code around would let us
             eliminate the specialized Assembly subclasses altogether. In such a case,
             this API will be removed from the framework.
+
+        Example
+        -------
+        [
+            (HexBlock, HexAssembly),
+            (CartesianBlock, CartesianAssembly),
+            (ThRZBlock, ThRZAssembly),
+        ]
+
+        Returns
+        -------
+        list
+            List of new Block&Assembly types
         """
 
     @staticmethod
     @HOOKSPEC
-    def defineBlueprintsSections():
+    def defineBlueprintsSections() -> List:
         """
         Return new sections for the blueprints input method.
 
@@ -329,7 +343,7 @@ def defineBlueprintsSections():
 
     @staticmethod
     @HOOKSPEC
-    def defineEntryPoints():
+    def defineEntryPoints() -> List:
         """
         Return new entry points for the ARMI CLI
 
@@ -344,7 +358,7 @@ class objects which derive from the base EntryPoint class.
 
     @staticmethod
     @HOOKSPEC
-    def defineSettings():
+    def defineSettings() -> List:
         """
         Define configuration settings for this plugin.
 
@@ -375,7 +389,7 @@ def defineSettings():
 
     @staticmethod
     @HOOKSPEC
-    def defineSettingsValidators(inspector):
+    def defineSettingsValidators(inspector) -> List:
         """
         Define the high-level settings input validators by adding them to an inspector.
 
@@ -430,12 +444,11 @@ def defineCaseDependencies(case, suite):
             This should return a set containing ``Case`` objects that are considered
             dependencies of the passed ``case``. They should be members of the passed
             ``suite``.
-
         """
 
     @staticmethod
     @HOOKSPEC
-    def defineGuiWidgets():
+    def defineGuiWidgets() -> List:
         """
         Define which settings should go in the GUI.
 
@@ -476,7 +489,7 @@ def getOperatorClassFromRunType(runType: str):
 
     @staticmethod
     @HOOKSPEC
-    def defineParameterRenames():
+    def defineParameterRenames() -> Dict:
         """
         Return a mapping from old parameter names to new parameter names.
 
@@ -596,7 +609,6 @@ def defineSystemBuilders() -> Dict[str, Callable[[str], "Composite"]]:
         -----
         The default :class:`~armi.reactor.ReactorPlugin` defines a ``"core"`` lookup
         and a ``"sfp"`` lookup, triggered to run after all other hooks have been run.
-
         """