Should we remove flags that represent part of the API?

[keckler]

There are currently flags for Core and Reactor being defined in our flags module:

armi/armi/reactor/flags.py

Lines 208 to 209 in 9dc6760

	CORE = auto()
	REACTOR = auto()

I would like to propose that we get rid of the two flags, as well as any other flags whose functionality is duplicated by other parts of the API (if any). My reasoning is the following:

1. These two flags do not serve any purpose that cannot also be fulfilled by something like isinstance(myCoreObject, Core)
2. Flags can lead to unintended behavior, expecially when we autoflag things (which we do)

A concrete example of this causing problems is documented in #1754 . Removing the Core and Reactor flags would be one way for us to safeguard against these types of problems, without us losing any actual functionality in the framework.

[john-science]

Flags should define what something is

I have not seen this in the code base, and if it exists I want to squash it immediately:

https://github.com/terrapower/armi/blame/9dc6760de7da3800cbe98e83066cf5f343e81a6a/armi/reactor/flags.py#L64-L66

I really hope that is a problem of the past.

CORE and REACTOR are bad flags

I agree with Chris above, that CORE and REACTOR are bad flags to exist. They are only used in a few places in ARMI:

armi/armi/reactor/reactors.py

Line 126 in 9dc6760

cores = self.getChildrenWithFlags(Flags.CORE)

armi/armi/bookkeeping/db/database3.py

Line 860 in 9dc6760

if comp.hasFlags(Flags.CORE):

armi/armi/bookkeeping/db/database3.py

Line 1422 in 9dc6760

r = comps[0].getAncestorWithFlags(Flags.REACTOR)

armi/armi/reactor/reactors.py

Line 101 in 9dc6760

self.p.flags |= Flags.REACTOR

These are easily replaced with type checks. (Also, I cannot find usages of these two flags downstream of ARMI.)

What other flags are unused or counter-useful?

I am immediately suspicious of these three flags, which are used nowhere but the docs:

armi/armi/reactor/flags.py

Lines 204 to 206 in 9dc6760

	HIGH = auto()
	MEDIUM = auto()
	LOW = auto()

[opotowsky]

#1835 Closes this

[opotowsky]

oops @john-science was that correct? I see you listed 3 more unused flags