suny-downstate-medical-center / netpyne

A Python package to facilitate the development, parallel simulation, optimization and analysis of multiscale biological neuronal networks in NEURON.
http://www.netpyne.org
MIT License
142 stars 134 forks source link

fix(imports): only import matplotlib where needed and if gui is enabled #788

Closed sanjayankur31 closed 8 months ago

sanjayankur31 commented 8 months ago

Fixes #786

Same as https://github.com/suny-downstate-medical-center/netpyne/pull/787, but cherry-picked to apply against development

sanjayankur31 commented 8 months ago

A simple test script to check matplotlib imports is here (maybe this should be added as a test in CI?)

"""
Example python script to run on NSGR

File: example.py

Copyright 2023 OSB contributors
"""

from netpyne import specs
from netpyne import sim
from netpyne import __version__ as version

import sys
import pkg_resources

if __name__ == "__main__":
    print("Running example script to list Python packages...")

    with open("output.txt", "w") as f:
        print(f"Python version is: {sys.version}", file=f)
        print("Installed packages on NSG:", file=f)
        dists = [str(d).replace(" ", "==") for d in pkg_resources.working_set]
        for i in dists:
            print(i, file=f)

    for k in sys.modules.keys():
        if "matplotlib" in k:
            print(f"matplotlib still loaded: {k}")

    print(f"Args are: {sys.argv}")
    if sys.argv.count("-nogui") > 0:
        print("nogui found")
    print("Done!")
vvbragin commented 8 months ago

@sanjayankur31 @jchen6727 thank you guys, looks good to me as well. For those plotting functions highlighted by James, where plt is needed, I would move the import inside the functions themselves, so the importing won't get triggered unless user explicitly wants to call that func. (Conceptually, I wouldn't connect the plotting functions to -nogui option, because the latter is not about plotting but specifically about GUI as far as I understand). What do you think?

Anyways, I'd merge this one PR, and the rest of modifications I would make in separate one (it's just easier for me then dealing with forked repo's PRs)

sanjayankur31 commented 8 months ago

Hello,

Yes, moving the imports into the functions is probably the best thing to do---or if the main netpyne __init__ imports the necessary matplotlib bits, then the other sub-modules could just use it from there, so you don't need to re-import things again and again (like the __gui__ variable). In general, it's probably a good idea to keep the analysis bits separate from the modelling/simulation bits, unless one can only use the analysis bits if they've first created a model and run a simulation..