Do not run Python code in top-level of modules

[dangunter]

Modules like dispatches/models/fossil_case/thermal_oil/discharge_heat_exchanger.py will run code on import. This is generally accepted as bad practice, and can break many different things.

---

EDIT @lbianchi-lbl Jul 19: added list

List of files where this happens (feel free to add or check off items if they change):

• dispatches/models/fossil_case/thermal_oil/discharge_heat_exchanger.py

[dangunter]

https://docs.python.org/3/library/__main__.html

[ksbeattie]

https://github.com/IDAES/idaes-pse/blob/main/idaes/power_generation/flowsheets/subcritical_power_plant/steam_cycle_flowsheet.py is an example to follow.

[ksbeattie]

We should try to get a list of such files, possibly using pylint?

[ksbeattie]

@dangunter, says he'll look into trying pytest in collect mode or possibly measuring import time similar to what pyomo runs in their test suite.

[dangunter]

There is a test to do this in a PR in the idaes-pse (main IDAES) repo, although that one may take a while to get merged since there seem to be some opinions on how to do it: IDAES/idaes-pse#396

[ksbeattie]

@jghouse88 will look into, at least for the thermal_oil/discharge_heat_exchanger.py file, moving this into a separate test.

[klfrick2]

is this an issue. The file dispatches/models/fossil_case/thermal_oil/discharge_heat_exchanger.py doesn't exist in current branches or the main file tree. At least not that I see.

It points to an old branch for this model.

[lbianchi-lbl]

The original file dispatches/models/fossil_case/thermal_oil/discharge_heat_exchanger.py that prompted this issue has since been converted to a test, so that particular aspect of the issue has been addressed.

To ensure that the DISPATCHES codebase is checked to avoid similar issues going forward, we're finalizing a tool to systematically run and analyze module imports, which will be integrated with the pytest test suite and run as part of the CI checks.

[lbianchi-lbl]

Bumping the priority down until after the upcoming internal release. Until we have an an automatic way to check for this, we can perform this check "manually" when reviewing a PR to make sure that only expected code (imports, functions/classes/constants definition) is executed at import time.