Repository: terrapower/armi

Issue:
Bad Default: Assembly.getArea() is 1.0
Over on [this PR](https://github.com/terrapower/armi/pull/1670), I noted a strange point in the ARMI API.

When an `Assembly` has no children/`Blocks`, this method returns a "default area" of `1.0`.  Surely that should be an error? Or it should return `None`. Or it should return `0.0`. Returning a valid numerical value just seems crazy.

https://github.com/terrapower/armi/blob/f5fd79b02f71a99e544da957050cf62392c13509/armi/reactor/assemblies.py#L274-L278

Discussion:
@albeanth: I say make it `None`. Rip the band aid off!  @jakehader: This might be one of those "not all things in the composite pattern" should be implemented kind of thing. I'd be careful with areas though. Typically you make a composite and then add things to it. Not sure if it should be `None` either. Can you point to the code here?@albeanth: > This might be one of those "not all things in the composite pattern" should be implemented kind of thing. I'd be careful with areas though. Typically you make a composite and then add things to it. Not sure if it should be `None` either. Can you point to the code here?

https://github.com/terrapower/armi/blob/f5fd79b02f71a99e544da957050cf62392c13509/armi/reactor/assemblies.py#L274-L278

I'd say `None` with a `runLog.warning` would be appropriate. Or you could raise an error. That'd work too. If someone is creating an Assembly and then querying its area without a Block I think it would be a mistake and getting 1.0 would be unexpected. But maybe there's an application I'm not aware of where an Assembly can do "stuff" without a Block? 

Solution patch:
diff --git a/armi/reactor/assemblies.py b/armi/reactor/assemblies.py
index 0761cded..7aa000c9 100644
--- a/armi/reactor/assemblies.py
+++ b/armi/reactor/assemblies.py
@@ -262,15 +262,14 @@ class Assembly(composites.Composite):
         )
 
     def coords(self):
-        """Return the location of the assembly in the plane using cartesian global
-        coordinates.
+        """Return the location of the assembly in the plane using cartesian global coordinates.
 
         .. impl:: Assembly coordinates are retrievable.
             :id: I_ARMI_ASSEM_POSI1
             :implements: R_ARMI_ASSEM_POSI
 
-            In this method, the spatialLocator of an Assembly is leveraged to return
-            its physical (x,y) coordinates in cm.
+            In this method, the spatialLocator of an Assembly is leveraged to return its physical
+            (x,y) coordinates in cm.
         """
         x, y, _z = self.spatialLocator.getGlobalCoordinates()
         return (x, y)
@@ -285,10 +284,8 @@ class Assembly(composites.Composite):
         try:
             return self[0].getArea()
         except IndexError:
-            runLog.warning(
-                "{} has no blocks and therefore no area. Assuming 1.0".format(self)
-            )
-            return 1.0
+            runLog.warning(f"{self} has no blocks and therefore no area.")
+            return None
 
     def getVolume(self):
         """Calculate the total assembly volume in cm^3."""
@@ -300,11 +297,11 @@ class Assembly(composites.Composite):
 
         Notes
         -----
-        If there is no plenum blocks in the assembly, a plenum volume of 0.0 is returned
+        If there is no plenum blocks in the assembly, a plenum volume of 0.0 is returned.
 
         Warning
         -------
-        This is a bit design-specific for pinned assemblies
+        This is a bit design-specific for pinned assemblies.
         """
         plenumVolume = 0.0
         for b in self.iterChildrenWithFlags(Flags.PLENUM):
@@ -349,8 +346,8 @@ class Assembly(composites.Composite):
                     "".format(b, b.getHeight(), refB, refB.getHeight())
                 )
             else:
-                # b is larger than refB. Split b up by splitting it into several smaller
-                # blocks of refBs
+                # b is larger than refB. Split b up by splitting it into several smaller blocks of
+                # refBs
                 heightToChop = b.getHeight()
                 heightChopped = 0.0
                 while (
@@ -364,16 +361,12 @@ class Assembly(composites.Composite):
                     heightChopped += refB.getHeight()
                     newBlocks += 1
                     runLog.important(
-                        "Added a new block {0} of height {1}".format(
-                            newB, newB.getHeight()
-                        )
+                        f"Added a new block {newB} of height {newB.getHeight()}"
                     )
-                    runLog.important(
-                        "Chopped {0} of {1}".format(heightChopped, heightToChop)
-                    )
-                newBlocks -= (
-                    1  # subtract one because we eliminated the original b completely.
-                )
+                    runLog.important(f"Chopped {heightChopped} of {heightToChop}")
+
+                # subtract one because we eliminated the original b completely.
+                newBlocks -= 1
 
         self.removeAll()
         self.spatialGrid = grids.AxialGrid.fromNCells(len(newBlockStack))
@@ -388,20 +381,19 @@ class Assembly(composites.Composite):
         Parameters
         ----------
         centers : bool, optional
-            Return centers instead of tops. If centers and zeroesAtFuel the zero point
-            will be center of first fuel.
+            Return centers instead of tops. If centers and zeroesAtFuel the zero point will be
+            center of first fuel.
 
         zeroAtFuel : bool, optional
-            If true will make the (bottom or center depending on centers) of the
-            first fuel block be the zero point instead of the bottom of the first block.
+            If true will make the (bottom or center depending on centers) of the first fuel block be
+            the zero point instead of the bottom of the first block.
 
         See Also
         --------
-        armi.reactor.assemblies.Assembly.makeAxialSnapList : makes index-based lookup of
-        axial mesh
+        armi.reactor.assemblies.Assembly.makeAxialSnapList : makes index-based lookup of axial mesh
 
-        armi.reactor.reactors.Reactor.findAllAxialMeshPoints : gets a global list of all
-        of these, plus finer res.
+        armi.reactor.reactors.Reactor.findAllAxialMeshPoints : gets a global list of all of these,
+        plus finer res.
         """
         bottom = 0.0
         meshVals = []
diff --git a/doc/developer/documenting.rst b/doc/developer/documenting.rst
index 09d4b81b..f01d6c35 100644
--- a/doc/developer/documenting.rst
+++ b/doc/developer/documenting.rst
@@ -5,25 +5,29 @@ Documenting ARMI
 ****************
 
 ARMI uses the `Sphinx <https://www.sphinx-doc.org/en/master/>`_ documentation system to compile the 
-web-based documentation from in-code docstrings and hand-created 
+ARMI documentation into HTML and PDF from in-code docstrings and hand-created
 `ReStructedText files <https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html>`_.
 This provides several benefits:
  
-* We can revise and track the documentation in lock-step with the code itself, in the same 
-  source code repository
-* We can make use of hyperlinked cross-references that stay up to date as the code is expanded 
-  or refactored.
-* We can run specific code tests during documentation building to ensure the documentation 
-  examples remain valid
+* We can revise and track the documentation in lock-step with the code itself, in the same source
+  code repository
+* We can make use of hyperlinked cross-references that stay up to date as the code is expanded or
+  refactored.
+* We can run specific code tests during documentation building to ensure the documentation examples
+  remain valid
 * We can auto-generate class diagrams based on the latest status of the code
+* Every Pull Request (PR) generates HTML and PDF versions of the documentation for the PR Author and
+  Reviewer
 
 We use some special Sphinx plugins that run the tutorial jupyter notebooks during documentation
-build with the most up-to-date code. 
+build with the most up-to-date code.
 
-Building the documentation
+Building the Documentation
 ==========================
 Before building documentation, ensure that you have installed the documentation requirements into
-your ARMI virtual environment with::
+your ARMI virtual environment with:
+
+.. code-block:: bash
 
     pip install -e .[docs]
 
@@ -32,32 +36,30 @@ You also need to have the following utilities available in your PATH:
 * `Graphviz <https://graphviz.org/>`_
 * `Pandoc <https://pandoc.org/>`_
 
-If you want to build the documentation into a PDF using the Sphinx LaTeX
-builder, you also need:
+If you want to build the documentation into a PDF using the Sphinx LaTeX builder, you also need:
 
 * LaTeX (`MikTeX <https://miktex.org/>`_ on Windows)
 * `ImageMagick <https://imagemagick.org/>`_
 
-The documentation depends on at least one submodule as well, so you must be sure
-it is available in your source tree with::
+The documentation depends on at least one submodule as well, so you must be sure it is available in
+your source tree with:
+
+.. code-block:: bash
 
     git submodule update --init
 
 
-To build the ARMI documentation as html, go to the ``doc`` folder and run::
+To build the ARMI documentation as html, go to the ``doc`` folder and run:
+
+.. code-block:: bash
 
     make html
 
-This will invoke Sphinx and generate a series of html files in the
-``_build/html`` folder. Open up ``index.html`` to see the documentation from
-there. A copy of the documentation is hosted online at
+This will invoke Sphinx and generate a series of html files in the ``_build/html`` folder. Open up
+``index.html`` to see the documentation from there. A copy of the documentation is hosted online at
 https://terrapower.github.io/armi/.
 
-You can suggest a change to the built documentation by copying the ``_build``
-files to a clone of the `documentation repository
-<https://github.com/terrapower/terrapower.github.io>`_ with a command like::
-
-    rsync -ahv --delete _build/html/ path/to/terrapower.github.io/armi
+You can suggest a change to the documentation by opening an ARMI PR.
 
 Documentation for ARMI plugins
 ==============================
@@ -65,7 +67,10 @@ The following subsections apply to documentation for ARMI plugins.
 
 Linking to ARMI documentation from plugins
 ------------------------------------------
-ARMI plugin documentation can feature rich hyperlinks to the ARMI API documentation with the help of the `intersphinx Sphinx plugin <http://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html>`_. The ARMI plugin documentation config file should add ``"sphinx.ext.intersphinx",`` to its active Sphinx plugin list, and change the default config to read::
+ARMI plugin documentation can feature rich hyperlinks to the ARMI API documentation with the help
+of the `intersphinx Sphinx plugin <http://www.sphinx-doc.org/en/master/usage/extensions/intersphinx.html>`_.
+The ARMI plugin documentation config file should add ``"sphinx.ext.intersphinx",`` to its active
+Sphinx plugin list, and change the default config to read::
 
     intersphinx_mapping = {
         "python": ("https://docs.python.org/3", None),
@@ -80,7 +85,9 @@ Now you can link to the ARMI documentation with links like::
 
 Automatically building apidocs of namespace packages
 ----------------------------------------------------
-Activating the ``"sphinxcontrib.apidoc",`` `Sphinx plugin <https://github.com/sphinx-contrib/apidoc>`_ enables plugin API documentation to be built with the standard ``make html`` Sphinx workflow. If your ARMI plugin is a namespace package, the following extra config is required::
+Activating the ``"sphinxcontrib.apidoc",`` `Sphinx plugin <https://github.com/sphinx-contrib/apidoc>`_
+enables plugin API documentation to be built with the standard ``make html`` Sphinx workflow. If your
+ARMI plugin is a namespace package, the following extra config is required::
 
     apidoc_extra_args = ["--implicit-namespaces"]
 
@@ -88,19 +95,20 @@ Updating the Gallery
 ====================
 The `ARMI example gallery <https://terrapower.github.io/armi/gallery/index.html>`_ is a great way
 to quickly highlight neat features and uses of ARMI. To add a new item to the gallery, add your
-example code (including the required docstring) to the ``doc/gallery-src`` folder in the ARMI
-source tree. The example will be added to the gallery during the next documentation build.
+example code (including the required docstring) to the ``doc/gallery-src`` folder in the ARMI source
+tree. The example will be added to the gallery during the next documentation build.
 
-Using Jupyter notebooks
+Using Jupyter Notebooks
 =======================
-For interactive tutorials, it's convenient to build actual Jupyter notebooks and 
-commit them to the documentation to be rendered by Sphinx using the nbsphinx plugin.
-When this is done, notebooks without any output should be committed to the repository
-so that Sphinx actually executes the notebooks with the up-to-date code when the 
-documentation is built. To do this, you can clean the output with::
+For interactive tutorials, it's convenient to build actual Jupyter notebooks and commit them to the
+documentation to be rendered by Sphinx using the nbsphinx plugin. When this is done, notebooks
+without any output should be committed to the repository so that Sphinx actually executes the
+notebooks with the up-to-date code when the  documentation is built. To do this, you can clean the
+output with:
 
-    jupyter nbconvert --ClearOutputPreprocessor.enabled=True --inplace mynotebook.ipynb
+.. code-block:: bash
 
-This should clear the output and overwrite the file. If this doesn't work, you can clear
-all output cells in the notebook web interface itself before committing the file.
+    jupyter nbconvert --ClearOutputPreprocessor.enabled=True --inplace mynotebook.ipynb
 
+This should clear the output and overwrite the file. If this doesn't work, you can clear all output
+cells in the notebook web interface itself before committing the file.


Test patch:
diff --git a/armi/reactor/tests/test_assemblies.py b/armi/reactor/tests/test_assemblies.py
index 1a24f629..226fac16 100644
--- a/armi/reactor/tests/test_assemblies.py
+++ b/armi/reactor/tests/test_assemblies.py
@@ -399,7 +399,7 @@ class Assembly_TestCase(unittest.TestCase):
         """Tests area calculation for hex assembly."""
         # Default case: for assemblies with no blocks
         a = HexAssembly("TestAssem", assemNum=10)
-        self.assertEqual(a.getArea(), 1)
+        self.assertIsNone(a.getArea())
 
         # more realistic case: a hex block/assembly
         cur = self.assembly.getArea()