Skip to content

Commit

Permalink
Remove PyYaml Dependency (#586)
Browse files Browse the repository at this point in the history
* Removing PyYaml dependency
* Removing and Fixing old TODOs
* Fail Build if code coverage under 80%
* Improving code coverage
  • Loading branch information
john-science authored Mar 2, 2022
1 parent 019ef64 commit 035711a
Show file tree
Hide file tree
Showing 12 changed files with 87 additions and 57 deletions.
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,6 @@ only use third-party Python libraries that have MIT or BSD licenses.
.. |Build Status| image:: https://github.com/terrapower/armi/actions/workflows/unittests.yaml/badge.svg?branch=master
:target: https://github.com/terrapower/armi/actions/workflows/unittests.yaml

.. |Code Coverage| image:: https://coveralls.io/repos/github/terrapower/armi/badge.svg?branch=master&kill_cache=1
.. |Code Coverage| image:: https://coveralls.io/repos/github/terrapower/armi/badge.svg?branch=master&kill_cache=2
:target: https://coveralls.io/github/terrapower/armi?branch=master

10 changes: 1 addition & 9 deletions armi/bookkeeping/db/database3.py
Original file line number Diff line number Diff line change
Expand Up @@ -1480,15 +1480,7 @@ def getHistoriesByLocation(
)

lLocation = layout.location
# filter for objects that live under the desired ancestor and at a desired
# location
# TODO: There might be a numpy way of doing this faster, were we to treat
# the locations as a numpy array. The elements are tuple of int, tuple of
# float, or sometimes even None, as determined by the pack/unpackLocations
# implementations, so it might not be possible, let alone trivial to do
# this. One approach could be to go back to the locations in their raw
# HDF5 form, then list index into that, along with locationType, and
# re-unpack them. 🤔
# filter for objects that live under the desired ancestor and at a desired location
objectIndicesInLayout = numpy.array(
[
i
Expand Down
4 changes: 2 additions & 2 deletions armi/cases/inputModifiers/tests/test_inputModifiers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import unittest
import os
import io
import yaml
from ruamel import yaml

from armi.utils import directoryChangers
from armi import cases
Expand Down Expand Up @@ -201,7 +201,7 @@ def SuiteNaming(index, _case, _mods):
f"case-suite-testBPBM/000{case_nbr}/armi-000{case_nbr}-blueprints.yaml",
"r",
)
bp_dict = yaml.safe_load(yamlfile)
bp_dict = yaml.load(yamlfile)
yamlfile.close()

self.assertEqual(bp_dict["blocks"]["fuel 1"]["clad"]["od"], 3.14)
Expand Down
7 changes: 4 additions & 3 deletions armi/nucDirectory/nuclideBases.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def getAttributes(nuc):
import pathlib
import glob

import yaml
from ruamel import yaml

import armi
from armi.nucDirectory import elements
Expand Down Expand Up @@ -308,7 +308,7 @@ def __readMc2Nuclides():
that have already been added from RIPL.
"""
with open(os.path.join(armi.context.RES, "mc2Nuclides.yaml"), "r") as mc2Nucs:
mc2Nuclides = yaml.load(mc2Nucs, Loader=yaml.FullLoader)
mc2Nuclides = yaml.load(mc2Nucs, yaml.RoundTripLoader)

# now add the mc2 specific nuclideBases, and correct the mc2Ids when a > 0 and state = 0
for name, data in mc2Nuclides.items():
Expand Down Expand Up @@ -400,7 +400,8 @@ def imposeBurnChain(burnChainStream):
runLog.warning("Burn chain already imposed. Skipping reimposition.")
return
_burnChainImposed = True
burnData = yaml.load(burnChainStream, Loader=yaml.FullLoader)
burnData = yaml.load(burnChainStream, yaml.RoundTripLoader)

for nucName, burnInfo in burnData.items():
nuclide = byName[nucName]
# think of this protected stuff as "module level protection" rather than class.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,30 +284,20 @@ def getLumpedFissionProductNames(self):
return self.keys()

def getAllFissionProductNames(self):
"""
Gets names of all fission products in this collection
TODO: This can use a set, continually updated from the keys() for each lfp, then converted
to a list. This will change the order of the fission products, though, so should be done
with care
"""
fpNames = []
"""Gets names of all fission products in this collection"""
fpNames = set()
for lfp in self.values():
for fp in lfp.keys():
if fp not in fpNames:
fpNames.append(fp.name)
return fpNames
fpNames.add(fp.name)
return sorted(fpNames)

def getAllFissionProductNuclideBases(self):
"""
Gets names of all fission products in this collection
"""
clideBases = []
"""Gets names of all fission products in this collection"""
clideBases = set()
for _lfpName, lfp in self.items():
for fp in lfp.keys():
if fp not in clideBases:
clideBases.append(fp)
return clideBases
clideBases.add(fp)
return sorted(clideBases)

def getNumberDensities(self, objectWithParentDensities=None, densFunc=None):
"""
Expand Down
1 change: 1 addition & 0 deletions armi/reactor/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ def __init__(self, name, height=1.0, location=None):
# TODO: what's causing these to have wrong values at BOL?
for problemParam in ["THcornTemp", "THedgeTemp"]:
self.p[problemParam] = []

for problemParam in [
"residence",
"bondRemoved",
Expand Down
34 changes: 14 additions & 20 deletions armi/reactor/reactors.py
Original file line number Diff line number Diff line change
Expand Up @@ -887,7 +887,7 @@ def getAssembliesInCircularRing(
)

# determine if the circularRingList has been generated
## TODO: make circularRingList a property that is generated on request
# TODO: make circularRingList a property that is generated on request
if not self.circularRingList:
self.circularRingList = self.buildCircularRingDictionary(
self._circularRingPitch
Expand Down Expand Up @@ -1230,9 +1230,8 @@ def regenAssemblyLists(self):
self._getAssembliesByName()
self._genBlocksByName()
runLog.important("Regenerating Core Zones")
self.buildZones(
settings.getMasterCs()
) # TODO: this call is questionable... the cs should correspond to analysis
# TODO: this call is questionable... the cs should correspond to analysis
self.buildZones(settings.getMasterCs())
self._genChildByLocationLookupTable()

def getAllXsSuffixes(self):
Expand Down Expand Up @@ -1571,7 +1570,6 @@ def findNeighbors(
showBlanks = True. This will have no effect if the model is full core since
asymmetric models could find many duplicates in the other thirds
Notes
-----
This only works for 1/3 or full core symmetry.
Expand All @@ -1580,7 +1578,6 @@ def findNeighbors(
standard (ring, pos) map. because neighbors have consistent indices this way. We
then convert over to (ring, pos) using the lookup table that a reactor has.
Returns
-------
neighbors : list of assembly objects
Expand All @@ -1597,7 +1594,6 @@ def findNeighbors(
assembly. It will only return "None" for an assembly when that assembly is
non-existing AND has no existing "symmetric identical".
See Also
--------
grids.Grid.getSymmetricEquivalents
Expand All @@ -1607,19 +1603,20 @@ def findNeighbors(
*a.spatialLocator.getCompleteIndices()
)

## TODO: where possible, move logic out of loops
dupReflectors = (
self.symmetry.domain == geometry.DomainType.THIRD_CORE
and self.symmetry.boundary == geometry.BoundaryType.PERIODIC
and duplicateAssembliesOnReflectiveBoundary
)

neighbors = []
for iN, jN, kN in neighborIndices:
neighborLoc = self.spatialGrid[iN, jN, kN]
neighbor = self.childrenByLocator.get(neighborLoc)
if neighbor is not None:
neighbors.append(neighbor)
elif showBlanks:
if (
self.symmetry.domain == geometry.DomainType.THIRD_CORE
and self.symmetry.boundary == geometry.BoundaryType.PERIODIC
and duplicateAssembliesOnReflectiveBoundary
):
if dupReflectors:
symmetricAssem = self._getReflectiveDuplicateAssembly(neighborLoc)
neighbors.append(symmetricAssem)
else:
Expand Down Expand Up @@ -2019,7 +2016,6 @@ def getMinimumPercentFluxInFuel(self, target=0.005):
targetRing, fraction of flux : tuple
targetRing is the ring with the fraction of flux that best meets the target.
"""

# get the total number of assembly rings
numRings = self.getNumRings()

Expand All @@ -2031,7 +2027,6 @@ def getMinimumPercentFluxInFuel(self, target=0.005):

# loop there all of the rings
for ringNumber in range(numRings, 0, -1):

# compare to outer most ring
# flatten list into one list of all blocks
blocksInRing = list(
Expand Down Expand Up @@ -2089,7 +2084,7 @@ def getAvgTemp(self, typeSpec, blockList=None, flux2Weight=False):
denom = 0.0
if not blockList:
blockList = list(self.getBlocks())
## TODO: this doesn't need to be a list

for b in blockList:
if flux2Weight:
weight = b.p.flux ** 2.0
Expand Down Expand Up @@ -2290,13 +2285,12 @@ def processLoading(self, cs):
for i, b in enumerate(refAssem):
if b.hasFlags(Flags.GRID_PLATE):
stationaryBlocks.append(i)
# TODO: remove hard-coded assumption of grid plates (T3019)
runLog.extra(
"Detected a grid plate {}. Adding to stationary blocks".format(b)
) # TODO: remove hard-coded assumption of grid plates (T3019)
)

cs[
"stationaryBlocks"
] = stationaryBlocks # TODO: DeprecationWarning - changing settings
cs["stationaryBlocks"] = stationaryBlocks

# Perform initial zoning task
self.buildZones(cs)
Expand Down
52 changes: 52 additions & 0 deletions armi/utils/tests/test_outputCache.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# Copyright 2022 TerraPower, LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
""" tests of the propereties class """
# pylint: disable=missing-function-docstring,missing-class-docstring,abstract-method,protected-access,invalid-name
import os
import time
import unittest

from armi.utils import directoryChangers
from armi.utils import outputCache


class TestOutputCache(unittest.TestCase):
def test_hashFiles(self):
with directoryChangers.TemporaryDirectoryChanger() as dc:
files = ["test_hashFiles1.txt", "test_hashFiles2.txt"]
for fileName in files:
with open(fileName, "w") as f:
f.write("hi")

hashed = outputCache._hashFiles(files)

self.assertEqual(hashed, "e9f5713dec55d727bb35392cec6190ce")

def test_deleteCache(self):
with directoryChangers.TemporaryDirectoryChanger() as dc:
outDir = "snapshotOutput_Cache"
self.assertFalse(os.path.exists(outDir))

os.mkdir(outDir)
with open(os.path.join(outDir, "test_deleteCache2.txt"), "w") as f:
f.write("hi there")

self.assertTrue(os.path.exists(outDir))
time.sleep(2)
outputCache.deleteCache(outDir)
self.assertFalse(os.path.exists(outDir))


if __name__ == "__main__":
unittest.main()
2 changes: 1 addition & 1 deletion armi/utils/textProcessors.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ def resolveMarkupInclusions(
-----
While the use of ``!include`` appears as though it would invoke some sort of special
custom YAML constructor code, this does not do that. Processing these inclusions as
part of the document parsing/composition that comes with pyyaml or ruamel.yaml could
part of the document parsing/composition that comes with ruamel.yaml could
work, but has a number of prohibitive drawbacks (or at least reasons why it might
not be worth doing). Using a custom constructor is more-or-less supported by
ruamel.yaml (which we do use, as it is what underpins the yamlize package), but it
Expand Down
1 change: 1 addition & 0 deletions doc/release/0.2.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ What's new in ARMI
#. Added neutronics settings: ``inners`` and ``outers`` for downstream support.
#. Removed unused Thermal Hydraulics settings.
#. Minor code re-org, moving math utilities into their own module.
#. Removed the ``PyYaml`` dependency.
#. TBD

Bug fixes
Expand Down
1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ def collectExtraFiles():
"pluggy",
"pyevtk",
"pympler",
"pyyaml>=5.1",
"scipy",
"tabulate",
"voluptuous",
Expand Down
4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ deps=
-r{toxinidir}/requirements.txt
-r{toxinidir}/requirements-testing.txt
commands =
pytest --cov-config=.coveragerc --cov=armi --ignore=armi/utils/tests/test_gridGui.py {posargs} armi
pytest --cov-config=.coveragerc --cov=armi --ignore=armi/utils/tests/test_gridGui.py --cov-fail-under=80 {posargs} armi

# NOTE: This only runs the MPI unit tests.
# NOTE: This will only work in POSIX/BASH Linux.
Expand All @@ -47,7 +47,7 @@ whitelist_externals =
commands =
pip install -r requirements.txt
pip install -r requirements-testing.txt
mpiexec -n 2 pytest armi/tests/test_mpiFeatures.py
mpiexec -n 2 --use-hwthread-cpus pytest armi/tests/test_mpiFeatures.py

[testenv:lint]
ignore_errors = true
Expand Down

0 comments on commit 035711a

Please sign in to comment.