Repository: CORE-GATECH-GROUP/serpent-tools
Pull request #393 (2020-04-01 02:34:38)

API/DEP Deprecate Depletion metadata for new attributes
The `metadata` dictionaries on `DepletionReader` and `DepletionSampler`  have been deprecated in favor of new attributes. These are more explicit and easier to access. The following example code serves to demonstrate the changes:
```python
>>> dep = serpentTools.read(depfile)
>>> dep.names is dep.metadata["names"]
True
>>> dep.zais is dep.metadata["zais"]
True
>>> dep.days is dep.metadata["days"]
True
>>> dep.burnup is dep.metadata["burnup"]
True
```

The metadata dictionary is never stored on the object, but is a property, generated at each call. Therefore any additions will not be permanent. However, the new attribute-based is a much better approach in my opinion. 

These data are still share with the depleted materials created during the read. This was done to keep this a back-compatible change if anyone is creating materials on their own.

There is one potential sticking point I would be happy to discuss. The reader has a `zais` attribute, while the materials have a `zai` attribute with the same data. I made the change to `zais` because these are really plural ZAI identifier**S** for each isotope, rather than a single isotope. I'm not sure how we want to reconcile the difference which would understandably be confusing. Changing `zai` :arrow_right: `zais` on the material objects would be a breaking change that I don't want to introduce in the next minor release. I support that change for 0.10.0 though.

Tests have been updated, separate of the API changes to make sure this was back-compatible. When introducing the properties in e896569 and a4c358b, the only change to the testing was one comparison test that added a key to the metadata dictionary. Since this is no longer permanent, the test is modified to compare directly against attributes.


Patch:
diff --git a/docs/changelog.rst b/docs/changelog.rst
index 15510d7..4d5e62c 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -6,11 +6,27 @@
 Changelog
 =========
 
-.. _v0.9.2:
+.. _v0.9.3:
 
-0.9.2
+0.9.3
 =====
 
+
+.. _v0.9.3-dep:
+
+Pending Deprecations
+--------------------
+
+* :attr:`serpentTools.DepletionReader.metadata`. Prefer attributes
+  like :attr:`~serpentTools.DepletionReader.zais`
+* :attr:`serpentTools.samplers.DepletionSampler.metadata`. Prefer
+  attributes like :attr:`~serpentTools.samplers.DepletionSampler.zais`
+
+.. _v0.9.2:
+
+:release-tag:`0.9.2`
+====================
+
 * Officially support installing under Python 3.8
 * Support for passing threshold values to hexagonal detector plots
   - :issue:`351`
diff --git a/serpentTools/parsers/base.py b/serpentTools/parsers/base.py
index 2542ef5..2847aa3 100644
--- a/serpentTools/parsers/base.py
+++ b/serpentTools/parsers/base.py
@@ -29,7 +29,6 @@ class BaseReader(ABC, BaseObject):
 
     def __init__(self, filePath, readerSettingsLevel):
         self.filePath = filePath
-        self.metadata = {}
         if isinstance(readerSettingsLevel, str):
             self.settings = rc.getReaderSettings(readerSettingsLevel)
         else:
@@ -81,7 +80,7 @@ class MaterialReader(BaseReader):
     """Parent class for files that store materials."""
 
     def __init__(self, filePath, readerSettingsLevel):
-        BaseReader.__init__(self, filePath, readerSettingsLevel)
+        super().__init__(filePath, readerSettingsLevel)
         self.materials = {}
 
 
@@ -89,7 +88,7 @@ class XSReader(BaseReader):
     """Parent class for the branching and results reader"""
 
     def __init__(self, filePath, readerSettingsLevel):
-        BaseReader.__init__(self, filePath, ['xs', readerSettingsLevel])
+        super().__init__(filePath, ['xs', readerSettingsLevel])
         self.settings['variables'] = rc.expandVariables()
         self.settings.pop('variableGroups')
         self.settings.pop('variableExtras')
diff --git a/serpentTools/parsers/depletion.py b/serpentTools/parsers/depletion.py
index 2434e93..f306682 100644
--- a/serpentTools/parsers/depletion.py
+++ b/serpentTools/parsers/depletion.py
@@ -6,7 +6,7 @@ from matplotlib import pyplot
 
 from serpentTools.utils import (
     magicPlotDocDecorator, formatPlot, DEPLETION_PLOT_LABELS,
-    convertVariableName, str2vec,
+    str2vec,
 )
 from ._collections import DEPLETION_VARIABLES
 from serpentTools.engines import KeywordParser
@@ -26,7 +26,8 @@ from serpentTools.utils import (
 )
 from serpentTools.io import MatlabConverter
 
-METADATA_KEYS = {'ZAI', 'NAMES', 'BU', 'DAYS'}
+METADATA_KEYS = {
+    'ZAI': "zais", 'NAMES': "names", 'BU': "burnup", 'DAYS': "days"}
 
 
 class DepPlotMixin(object):
@@ -135,6 +136,13 @@ class DepletionReader(DepPlotMixin, MaterialReader):
     """
     Parser responsible for reading and working with depletion files.
 
+    .. note::
+
+        :attr:`metadata` is now a deprecated property, generated at
+        each call.  Additional keys added will not be retained.
+        Prefer using :attr:`names`, :attr:`burnup`,
+        :attr:`days`, and :attr:`zais`
+
     Parameters
     ----------
     filePath: str
@@ -142,10 +150,25 @@ class DepletionReader(DepPlotMixin, MaterialReader):
 
     Attributes
     ----------
-    {attrs:s}
+    days : numpy.ndarray or None
+        Vector of points in time
+    burnup : numpy.ndarray or None
+        Nominal burnup in MWd/kgU
+    names : list of str or None
+        Names of isotopes corresponding to each row in the 2D arrays
+    zais : list of int or None
+        ZZAAAI identifiers for isotopes
+    metadata : dict of str to list
+        Dictionary with zai and names
+
+        .. deprecated:: 0.9.3
+    materials : dict of str to serpentTools.objects.Material
+        Materials from the file. Keys will be the names as they appear
+        in the file
     settings: dict
         names and values of the settings used to control operations
         of this reader
+
     """
     docAttrs = """materials: dict
         Dictionary with material names as keys and the corresponding
@@ -154,7 +177,6 @@ class DepletionReader(DepPlotMixin, MaterialReader):
     metadata: dict
         Dictionary with file-wide data names as keys and the
         corresponding data, e.g. ``'zai'``: [list of zai numbers]"""
-    __doc__ = __doc__.format(attrs=docAttrs)
 
     def __init__(self, filePath):
         MaterialReader.__init__(self, filePath, 'depletion')
@@ -162,6 +184,10 @@ class DepletionReader(DepPlotMixin, MaterialReader):
         # match all materials if nothing given
         self._matPatterns = [re.compile(mat) for mat in patterns]
         DepPlotMixin.__init__(self)
+        self.names = None
+        self.zais = None
+        self.days = None
+        self.burnup = None
 
     def __getitem__(self, name):
         """Retrieve a material from :attr:`materials`."""
@@ -223,12 +249,12 @@ class DepletionReader(DepPlotMixin, MaterialReader):
                     self._checkAddData(chunk, name, variable)
                     continue
                 self._addMetadata(chunk)
-        if 'days' in self.metadata:
-            for mKey in self:
-                self.materials[mKey].days = self.metadata['days']
+        if self.days is not None:
+            for material in self.values():
+                material.days = self.days
 
     def _addMetadata(self, chunk):
-        for varName in METADATA_KEYS:
+        for varName, destination in METADATA_KEYS.items():
             if varName not in chunk[0]:
                 continue
             if varName in ['ZAI', 'NAMES']:
@@ -240,10 +266,12 @@ class DepletionReader(DepPlotMixin, MaterialReader):
             else:
                 line = self._cleanSingleLine(chunk)
                 values = str2vec(line)
-            self.metadata[convertVariableName(varName)] = values
-            return
-        warning("Unsure about how to process metadata chunk {}"
-                .format(chunk[0]))
+            setattr(self, destination, values)
+            break
+        else:
+            raise ValueError(
+                "Unsure about how to process metadata chunk "
+                "{}".format(chunk[0]))
 
     @staticmethod
     def _cleanSingleLine(chunk):
@@ -299,8 +327,18 @@ class DepletionReader(DepPlotMixin, MaterialReader):
             )
         debug('  found {} materials'.format(len(self)))
 
-        if 'bu' in self.metadata:
-            self.metadata['burnup'] = self.metadata.pop('bu')
+    @property
+    def metadata(self):
+        out = {}
+        if self.names is not None:
+            out["names"] = self.names
+        if self.zais is not None:
+            out["zai"] = self.zais
+        if self.days is not None:
+            out["days"] = self.days
+        if self.burnup is not None:
+            out["burnup"] = self.burnup
+        return out
 
     def _compare(self, other, lower, upper, _sigma):
 
@@ -311,9 +349,13 @@ class DepletionReader(DepPlotMixin, MaterialReader):
         return similar
 
     def _comparePrecheckMetadata(self, other):
-        for key, myVec in self.metadata.items():
-            otherVec = other.metadata[key]
-            if len(myVec) != len(otherVec):
+        for key in ["names", "zais", "days", "burnup"]:
+            myVec = getattr(self, key)
+            otherVec = getattr(other, key, None)
+            if (
+                (otherVec is None and myVec is not None)
+                or len(myVec) != len(otherVec)
+            ):
                 error("Stopping comparison early due to mismatched {} vectors"
                       "\n\t>{}\n\t<{}".format(key, myVec, otherVec))
                 return False
@@ -394,18 +436,12 @@ class DepletionReader(DepPlotMixin, MaterialReader):
     def _compareMetadata(self, other, lower, upper, _sigma):
         """Private method for comparing metadata"""
 
-        similar = logDirectCompare(
-            self.metadata['names'], other.metadata['names'],
-            0, 0, 'names')
-        similar &= logDirectCompare(
-            self.metadata['zai'], other.metadata['zai'],
-            0, 0, 'zai')
+        similar = logDirectCompare(self.names, other.names, 0, 0, 'names')
+        similar &= logDirectCompare(self.zais, other.zais, 0, 0, 'zai')
         similar &= logDirectCompare(
-            self.metadata['days'], other.metadata['days'],
-            lower, upper, 'days')
+            self.days, other.days, lower, upper, 'days')
         similar &= logDirectCompare(
-            self.metadata['burnup'], other.metadata['burnup'],
-            lower, upper, 'burnup')
+            self.burnup, other.burnup, lower, upper, 'burnup')
         return similar
 
     @deprecated("toMatlab")
@@ -480,8 +516,13 @@ class DepletionReader(DepPlotMixin, MaterialReader):
         else:
             converter = prepToMatlab
 
-        data = {k.upper() if reconvert else k: v
-                for k, v in self.metadata.items()}
+        data = {}
+        for k in ["names", "zais", "burnup", "days"]:
+            value = getattr(self, k)
+            if k == "zais":
+                k = "zai"  # Back compatibility
+            if value is not None:
+                data[k.upper() if reconvert else k] = value
 
         for matName, material in self.items():
             for varName, varData in material.data.items():
diff --git a/serpentTools/parsers/results.py b/serpentTools/parsers/results.py
index c59daf9..c0876fb 100644
--- a/serpentTools/parsers/results.py
+++ b/serpentTools/parsers/results.py
@@ -202,8 +202,7 @@ class ResultsReader(XSReader):
     """Metadata keys that will not be compared."""
 
     def __init__(self, filePath):
-        XSReader.__init__(self, filePath, 'results')
-
+        super().__init__(filePath, 'results')
         self.metadata = {}
         self.resdata = {}
         self.universes = {}
diff --git a/serpentTools/samplers/depletion.py b/serpentTools/samplers/depletion.py
index 1a732c6..eb0d1d0 100644
--- a/serpentTools/samplers/depletion.py
+++ b/serpentTools/samplers/depletion.py
@@ -3,6 +3,7 @@ Class responsible for reading multiple depletion files
 and obtaining true uncertainties
 """
 from math import fabs
+import collections
 
 from numpy import zeros, zeros_like
 from matplotlib import pyplot
@@ -17,15 +18,9 @@ from serpentTools.samplers.base import (
     Sampler, SampledContainer,
 )
 
-CONSTANT_MDATA = ('names', 'zai')
-"""metadata that should be invariant throughout repeated runs"""
-VARIED_MDATA = ('days', 'burnup')
-"""metadata that could be varied throughout repeated runs"""
-
 
 class DepletionSampler(DepPlotMixin, Sampler):
-    __doc__ = """
-    Class that reads and stores data from multiple ``*dep.m`` files
+    """Class that reads and stores data from multiple ``*dep.m`` files
 
     The following checks are performed in order to ensure that depletion
     files are of similar form:
@@ -46,6 +41,14 @@ class DepletionSampler(DepPlotMixin, Sampler):
 
     Attributes
     ----------
+    days : numpy.ndarray or None
+        Vector of points in time
+    burnup : numpy.ndarray or None
+        Nominal burnup in MWd/kgU
+    names : list of str or None
+        Names of isotopes corresponding to each row in the 2D arrays
+    zais : list of int or None
+        ZZAAAI identifiers for isotopes
     materials: dict
         Dictionary with material names as keys and the corresponding
         :py:class:`~serpentTools.objects.materials.DepletedMaterial` class
@@ -53,9 +56,12 @@ class DepletionSampler(DepPlotMixin, Sampler):
     metadata: dict
         Dictionary with file-wide data names as keys and the
         corresponding data, e.g. ``'zai'``: [list of zai numbers]
+
+        .. deprecated:: 0.9.3
+            Use attributes like :attr:`days` directly
     metadataUncs: dict
-        Dictionary containing uncertainties in file-wide metadata,
-        such as burnup schedule
+        Dictionary containing uncertainties in day and burnup
+        vectors
     allMdata: dict
         Dictionary where key, value pairs are name of metadata and
         metadata arrays for all runs. Arrays with be of one greater dimension,
@@ -74,7 +80,10 @@ class DepletionSampler(DepPlotMixin, Sampler):
 
     def __init__(self, files):
         self.materials = {}
-        self.metadata = {}
+        self.names = None
+        self.zais = None
+        self.days = None
+        self.burnup = None
         self.metadataUncs = {}
         self.allMdata = {}
         Sampler.__init__(self, files, DepletionReader)
@@ -84,6 +93,19 @@ class DepletionSampler(DepPlotMixin, Sampler):
         """Retrieve a material from :attr:`materials`."""
         return self.materials[name]
 
+    @property
+    def metadata(self):
+        out = {}
+        if self.names is not None:
+            out["names"] = self.names
+        if self.zais is not None:
+            out["zai"] = self.zais
+        if self.days is not None:
+            out["days"] = self.days
+        if self.burnup is not None:
+            out["burnup"] = self.burnup
+        return out
+
     def _precheck(self):
         self._checkParserDictKeys('materials')
         self._checkParserDictKeys('metadata')
@@ -92,25 +114,28 @@ class DepletionSampler(DepPlotMixin, Sampler):
     def _checkMetadata(self):
         misMatch = {}
         for parser in self:
-            for key, value in parser.metadata.items():
-                valCheck = (tuple(value) if key in CONSTANT_MDATA
-                            else value.size)
+            for key in ["names", "zais", "days", "burnup"]:
+                value = getattr(parser, key)
+                valCheck = (
+                    tuple(value) if key in {"names", "zais"} else value.size
+                )
                 if key not in misMatch:
-                    misMatch[key] = {}
-                if valCheck not in misMatch[key]:
-                    misMatch[key][valCheck] = {parser.filePath}
-                else:
-                    misMatch[key][valCheck].add(parser.filePath)
+                    misMatch[key] = collections.defaultdict(set)
+                misMatch[key][valCheck].add(parser.filePath)
         for mKey, matches in misMatch.items():
             if len(matches) > 1:
                 self._raiseErrorMsgFromDict(matches, 'values',
                                             '{} metadata'.format(mKey))
 
     def _process(self):
+        # Not sure how / why this is a set...
+        parser = self.parsers.pop()
+        self._allocateMetadata(parser)
+        self.parsers.add(parser)
+
         for N, parser in enumerate(self.parsers):
-            if not self.metadata:
-                self.__allocateMetadata(parser.metadata)
-            self._copyMetadata(parser.metadata, N)
+            self.allMdata["days"][N] = parser.days
+            self.allMdata["burnup"][N] = parser.burnup
             for matName, material in parser.materials.items():
                 if matName in self.materials:
                     sampledMaterial = self.materials[matName]
@@ -123,28 +148,31 @@ class DepletionSampler(DepPlotMixin, Sampler):
         self._finalize()
 
     def _finalize(self):
-        for _matName, material in self.materials.items():
+        for material in self.materials.values():
             material.finalize()
-        for key in VARIED_MDATA:
+        for key in {"days", "burnup"}:
             allData = self.allMdata[key]
-            self.metadata[key] = allData.mean(axis=0)
+            setattr(self, key, allData.mean(axis=0))
             self.metadataUncs[key] = allData.std(axis=0)
 
-    def __allocateMetadata(self, parserMdata):
-        for key in CONSTANT_MDATA:
-            self.metadata[key] = parserMdata[key]
-        vectorShape = tuple([len(self.files)]
-                            + list(parserMdata['days'].shape))
-        for key in VARIED_MDATA:
-            self.allMdata[key] = zeros(vectorShape)
+    def _allocateMetadata(self, parser):
+        self.names = parser.names
+        self.zais = parser.zais
+        if parser.days is None:
+            if parser.burnup is None:
+                raise AttributeError("Neither burnup nor days stored")
+            shape = len(self.files), parser.burnup.size
+        else:
+            shape = len(self.files), parser.days.size
+        self.allMdata["days"] = zeros(shape)
+        self.allMdata["burnup"] = zeros(shape)
 
-    def _copyMetadata(self, parserMdata, N):
-        for key in VARIED_MDATA:
-            self.allMdata[key][N, ...] = parserMdata[key]
+        for key in {"days", "burnup"}:
+            self.allMdata[key] = zeros(shape)
 
     def _free(self):
         self.allMdata = {}
-        for _mName, material in self.materials.items():
+        for material in self.materials.values():
             material.free()
 
     def iterMaterials(self):


Test patch:
diff --git a/tests/compare/test_depletion.py b/tests/compare/test_depletion.py
index 2bcc287..cfe74a5 100644
--- a/tests/compare/test_depletion.py
+++ b/tests/compare/test_depletion.py
@@ -83,12 +83,9 @@ class DepletionReaderComparisonTester(DepletionCompareHelper):
         """
         Verify the comparison fails early for dissimilar metadata shapes.
         """
-        newVecKey = 'badMetadataKey'
-        self.refReader.metadata[newVecKey] = [0, 1, 2, 3]
-        self.otherReader.metadata[newVecKey] = [0, 1, 2]
+        self.otherReader.names = self.refReader.names[1:]
         self.assertFalse(self.compare(100, 100, 100))
-        self.assertMsgInLogs('ERROR', newVecKey, partial=True)
-        self.refReader.metadata.pop(newVecKey)
+        self.assertMsgInLogs('ERROR', "names", partial=True)
 
     def test_diffMaterialDicts(self):
         """Verify the test fails if different materials are stored."""
diff --git a/tests/test_depletion.py b/tests/test_depletion.py
index cd3bb83..5dc0061 100644
--- a/tests/test_depletion.py
+++ b/tests/test_depletion.py
@@ -84,25 +84,28 @@ class DepletionTester(_DepletionTestHelper):
 
     def test_metadata(self):
         """Test the metadata storage for the reader."""
-        expectedMetadata = {
-            'zai':
-                [621490, 541350, 922350, 942390, 50100, 666, 0],
-            'names':
-                ['Sm149', 'Xe135', 'U235', 'Pu239', 'B10', 'lost', 'total'],
-            'burnup': [0.00000E+00, 1.93360E-02, 3.86721E-02, 1.16016E-01,
-                       1.93360E-01, 2.90041E-01, 3.86721E-01, 6.76762E-01,
-                       9.66802E-01, 1.45020E+00, 1.93360E+00, 2.90041E+00,
-                       3.86721E+00, 4.83401E+00],
-            'days': [0.00000E+00, 5.00000E-01, 1.00000E+00, 3.00000E+00,
-                     5.00000E+00, 7.50000E+00, 1.00000E+01, 1.75000E+01,
-                     2.50000E+01, 3.75000E+01, 5.00000E+01, 7.50000E+01,
-                     1.00000E+02, 1.25000E+02]
-        }
-        expectedKeys = set(expectedMetadata)
-        actualKeys = set(self.reader.metadata.keys())
-        self.assertSetEqual(expectedKeys, actualKeys)
-        for key, expectedValue in expectedMetadata.items():
-            assert_equal(self.reader.metadata[key], expectedValue)
+        self.assertListEqual(
+            self.reader.zais,
+            [621490, 541350, 922350, 942390, 50100, 666, 0],
+        )
+        self.assertListEqual(
+            self.reader.names,
+            ['Sm149', 'Xe135', 'U235', 'Pu239', 'B10', 'lost', 'total'],
+        )
+        assert_equal(
+            self.reader.burnup,
+            [0.00000E+00, 1.93360E-02, 3.86721E-02, 1.16016E-01,
+             1.93360E-01, 2.90041E-01, 3.86721E-01, 6.76762E-01,
+             9.66802E-01, 1.45020E+00, 1.93360E+00, 2.90041E+00,
+             3.86721E+00, 4.83401E+00],
+        )
+        assert_equal(
+            self.reader.days,
+            [0.00000E+00, 5.00000E-01, 1.00000E+00, 3.00000E+00,
+             5.00000E+00, 7.50000E+00, 1.00000E+01, 1.75000E+01,
+             2.50000E+01, 3.75000E+01, 5.00000E+01, 7.50000E+01,
+             1.00000E+02, 1.25000E+02],
+        )
 
     def test_ReadMaterials(self):
         """Verify the reader stored the correct materials."""
@@ -218,8 +221,7 @@ class DepletedMaterialTester(_DepletionTestHelper):
              1.49595E+09, 1.66322E+09, 1.80206E+09, 1.79453E+09, 1.79100E+09,
              1.80188E+09, 1.75346E+09, 1.60021E+09, 1.89771E+09],
         ])
-        self.assertListEqual(self.material.zai,
-                             self.reader.metadata['zai'])
+        self.assertListEqual(self.material.zai, self.reader.zais)
         assert_equal(self.material.adens, expectedAdens)
         assert_equal(self.material['ingTox'], expectedIngTox)
 
@@ -377,10 +379,32 @@ class DepMatlabExportHelper(MatlabTesterHelper):
                     err_msg="{} {}".format(materialName, variableName))
 
     def checkMetadata(self, loaded):
-        for key, data in self.reader.metadata.items():
-            expected = key.upper() if self.RECONVERT else key
-            assert expected in loaded, (
-                "Missing {} from metadata".format(expected))
+        if self.RECONVERT:
+            zais = loaded["ZAI"]
+            names = loaded["NAMES"]
+            days = loaded["DAYS"]
+            burnup = loaded["BURNUP"]
+        else:
+            zais = loaded["zai"]
+            names = loaded["names"]
+            days = loaded["days"]
+            burnup = loaded["burnup"]
+
+        # String array has extra spacing around shorter words
+        # so names like "Xe135" are fine, but we also get "lost "
+        # with a space
+        self.assertListEqual([x.strip() for x in names], self.reader.names)
+
+        # Numeric arraysrrays are written as 1xN matrices
+        # Can't do a straight comparison first
+        for actual, attr in [
+            [zais, "zais"],
+            [days, "days"],
+            [burnup, "burnup"],
+        ]:
+            expected = getattr(self.reader, attr)
+            self.assertEqual(actual.shape, (1, len(expected)), msg=attr)
+            assert_equal(actual[0], expected, err_msg=attr)
 
     def constructExpectedVarName(self, material, variable):
         """