Stop abusing Blueprints._prepConstruction or make it public

[drewj-tp]

Blueprints._prepConstruction does a lot. Its docstring even says as much

armi/armi/reactor/blueprints/__init__.py

Lines 276 to 278 in f318392

	def _prepConstruction(self, cs):
	"""
	This method initializes a bunch of information within a Blueprints object such

Not only does it construct the design of each individual assembly type, it

1. Does processing on nuclides found in the blueprints.
2. Checks the area of assemblies.
3. Conditionally configures the axial snap mesh.
4. Conditionally performs initial thermal axial expansion on assemblies.
5. Calls the afterConstructionOfAssemblies plugin hook.

That's a lot! The value there is clear, as all those things must be done in order to make sure the assembly designs are ready to be used.

While I was working on the axial expansion work (#1843), I noticed that this private method is used directly. Which raises some hairs on the back of my neck because, 1) it points out this contradiction in its own docstring

armi/armi/reactor/blueprints/__init__.py

Line 288 in f318392

This method should not be called directly, but it is used in testing.

and 2) I am of the belief that non-public methods (single or double leading underscore) should not be called by anything that is not the owning class. Yes, everything in Python is public, somethings just harder to access. But this public/protected/private distinction also allows developers some wiggle room.

If a method or attribute was truly protected, I have more freedom to refactor and break it up or think around that method. Maybe the four things it does don't all need to happen at once can be moved around. But if it's functionally public, it's a heavier lift.

Resolution

I propose that we rethink Blueprints._prepConstruction and determine how to either

1. stop misusing (in my opinion) a protected method, or
2. what functionality callers are expecting and make that public.

I've collected the cases where we use this method. Thankfully the majority of them are in testing. I presume that most are needed because we want to build assemblies without spinning up entire reactors from blueprints, but the blueprints class requires the assembly to be processed in _prepConstruction.

Non-test modules

armi/armi/cases/suiteBuilder.py

Line 168 in f318392

case.bp._prepConstruction(case.cs)

armi/armi/reactor/reactors.py

Line 2471 in f318392

self.parent.blueprints._prepConstruction(cs)

Test modules

armi/armi/reactor/tests/test_blocks.py

Line 316 in f318392

bps._prepConstruction(cs)

armi/armi/reactor/tests/test_assemblies.py

Line 597 in f318392

self.r.blueprints._prepConstruction(self.cs)

armi/armi/reactor/tests/test_assemblies.py

Line 1442 in f318392

self.blueprints._prepConstruction(self.cs)

armi/armi/reactor/blueprints/tests/test_blueprints.py

Line 63 in f318392

cls.blueprints._prepConstruction(cls.cs)

armi/armi/reactor/blueprints/tests/test_blueprints.py

Line 306 in f318392

design._prepConstruction(cs)

armi/armi/materials/tests/test_materials.py

Line 2013 in f318392

design._prepConstruction(settings.Settings())

armi/armi/cases/inputModifiers/tests/test_pinTypeInputModifiers.py

Line 31 in f318392

self.bp._prepConstruction(settings.Settings())

armi/armi/reactor/blueprints/tests/test_materialModifications.py

Line 62 in f318392

design._prepConstruction(settings.Settings())

armi/armi/cases/inputModifiers/tests/test_inputModifiers.py

Line 131 in f318392

bp._prepConstruction(cs)

armi/armi/reactor/blueprints/tests/test_assemblyBlueprints.py

Line 182 in f318392

design._prepConstruction(settings.Settings())

armi/armi/reactor/blueprints/tests/test_blockBlueprints.py

Line 265 in f318392

self.blueprints._prepConstruction(self.cs)

[john-science]

I'm not worried about the times when this pops up in testing.

Unit tests should be able to test private methods. Just because logic is internal to a class doesn't mean it's not complicated enough, or important enough, to warrant testing.

So, from my perspective, this only pops up once in a non-test environment. Still, that IS interesting.

[drewj-tp]

Unit tests should be able to test private methods

Agree to a point. A unit test on the Blueprints object using and testing protected methods on that object> I can get behind that.

But at test for a materials object needing a private method from `Blueprints?

armi/armi/materials/tests/test_materials.py

Lines 2010 to 2014 in f318392

	def loadAssembly(self, materialModifications):
	yamlString = self.baseInput + "\n" + materialModifications
	design = blueprints.Blueprints.load(yamlString)
	design._prepConstruction(settings.Settings())
	return design.assemblies["fuel a"]

I think that's stretching the domain of applicability.

I think nearly all of these could be mitigated if we have a Blueprints.buildAssembly method because that's what the majority of these tests need. After some brief skimming at least.

this only pops up once in a non-test environment

Twice

armi/armi/cases/suiteBuilder.py

Line 168 in f318392

case.bp._prepConstruction(case.cs)

and

armi/armi/reactor/reactors.py

Line 2471 in f318392

self.parent.blueprints._prepConstruction(cs)

[john-science]

I agree, if this is needed to be public in a non-test setting, we should probably make it public. Sure.

[drewj-tp]

FWIW I have been able to have identical success replacing patterns of

bp._prepConstruction(cs)
assem = bp.assemblies[assem_name]

with

assem = bp.constructAssembly(cs, name=assem_name)

The tradeoff is bp.constructAssembly does a deepcopy of an assembly and calls Assembly.makeUnique prior to returning the new assembly. Those may or may not be important for testing but are certainly important for production analysis