Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reducing the instances of bare "import armi". #347

Merged
merged 1 commit into from
Jul 28, 2021

Conversation

john-science
Copy link
Member

There are several places in the code where a bare import armi is done from within the armi module. Python is really good about lazily importing large swaths of code to prevent circular imports. But this sort of thing can still cause opaque runtime errors.

There are more places in the code where import armi is used, but those will all require some code refactoring to provide different pathways for the desired usage. This PR only covers the simplest cases where we can just remove the bare import and replace it without a more specific one.

There are several places in the code where a bare "import armi" is done
from within the armi module. Python is really good about lazily importing
large swaths of code to prevent circular imports. But this sort of thing
can very easily cause opaque runtime errors.

There are more places in the code where "import armi" is used, but those
will all require some code refactoring to provide different pathways for
the desired usage. This PR only covers the simplest cases where we can
just remove the bare import and replace it without a more specific one.
@john-science
Copy link
Member Author

There are still 23 instances left in ARMI with bare import armi statements:

File What is used?
__main__.py configure(), isConfigured(), MPI_RANK, MPI_NODENAME, MPI_COMM
bookkeeping/db/database3.py MPI_RANK, version, USER, ROOT, START_TIME, getApp()
bookkeeping/memoryProfiler.py MPI_NODENAME
bookkeeping/report/html.py RES, USER
bookkeeping/report/reportingUtils.py USER, ROOT, MPI_SIZE, START_TIME, MPI_NODENAME, MPI_RANK
cases/case.py getPluginManager(), RES, MPI_RANK, MPI_SIZE, MPI_COMM, CURRENT_MODE, Mode.INTERACTIVE, context.APP_NAME
cli/__init__.py getPluginManager(), context.APP_NAME, version, getApp(), Mode.setMode(), context.MPI_RANK
cli/database.py Mode.BATCH
localization/__init__.py MPI_RANK
mpiActions.py MPI_SIZE, MPI_RANK, MPI_COMM, MPI_DISTRIBUTABLE, MPI_NODENAMES
nucDirectory/nuclideBases.py context.RES
operators/__init__.py MPI_SIZE, getPluginManagerOrFail()
operators/operator.py MPI_RANK
operators/operatorMPI.py MPI_COMM, MPI_RANK, MPI_SIZE, getPluginManager()
operators/settingsValidation.py getPluginManagerOrFail(), MPI_RANK
physics/neutronics/crossSectionGroupManager.py MPI_RANK
physics/neutronics/macroXSGenerationInterface.py MPI_RANK, MPI_COMM, MPI_SIZE
reactor/blueprints/reactorBlueprint.py MPI_RANK
reactor/composites.py MPI_SIZE, MPI_COMM
settings/caseSettings.py getApp(), MPI_RANK
utils/__init__.py __path__, __name__
utils/directoryChangers.py context.getFastPath(), MPI_SIZE
doc/conf.py configure(), _ignoreConfigures, __version__

We are also using bare import armi statements in unit test files. While that is less important, these tests are currently being packaged with ARMI and are importable at run time:

Test File What is used?
localization/tests/test_localization.py MPI_RANK
operators/tests/test_operators.py MPI_COMM, MPI_RANK, MPI_SIZE
reactor/tests/test_parameters.py MPI_RANK, MPI_COMM, MPI_SIZE
settings/tests/test_settings.py _app, getPluginManagerOrFail()
settings/tests/test_settingsIO.py Mode, CURRENT_MODE
tests/__init__.py ROOT
tests/test_apps.py _app, getApp(), getDefaultPluginManager(), configure()
tests/test_mpiActions.py MPI_COMM, MPI_RANK, MPI_SIZE

@youngmit
Copy link
Contributor

Love it! We should probably pull your tables out into an issue, so they don't die in here.

@youngmit youngmit merged commit 27358cf into terrapower:master Jul 28, 2021
@john-science
Copy link
Member Author

Love it! We should probably pull your tables out into an issue, so they don't die in here.

Done: #349

@john-science john-science deleted the reduce_plain_imports branch September 10, 2021 21:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants