Imports inside functions

[JuliaKukulies]

I realized that we are currently quite inconsistent with importing modules in functions vs. importing modules at the top of the tobac modules. An example is that in some modules, we import numpy at the top whereas in other modules it is imported in every single function.

It is not the most important, but at some point, we should maybe discuss and agree on a rule for this and go through all files to remove redundant imports. Keeping most imports at the top of the file would have a few advantages:

• PEP8 compliant
• easier to keep track of which imports are needed for which module
• probably more efficient (especially when functions are called frequently, e.g. for every timestep/threshold/feature)
• potential ImportErrors are raised immediately before any code is run

Any thoughts?

[freemansw1]

I think switching to imports at the top of functions, in general, makes sense. The two situations where I see where importing inside functions as advantageous are:

1. If a feature is optional, clearly the import should be inside a function.
2. If an import is of a common name (e.g., imports of the form from x import y, where y is some common name). We should probably avoid these in general, but if we can't/don't, these should be as limited in scope as possible.

I'd be happy to see a refactor here, but given our code coverage is ~30-40%, we will have to be very careful that we don't break anything.

[JuliaKukulies]

Thanks for your thoughts @freemansw1! I agree with the two situations you mention where it makes sense to keep the imports inside functions instead of at the top of modules.

Good point about the test coverage. So maybe a way to go would be to only refactor functions that are covered and functions that are added to the modules from now (but this could also result in a mess and we would have redundant imports during some time) or we simply see this as a more long-term goal after we have increased coverage, which is certainly more important than this.

[freemansw1]

So while our overall code coverage number is pretty poor, our code coverage on the most important modules (feature detection, segmentation, tracking) is actually pretty good. I wouldn't be opposed to switching those functions to module-level imports, and then we can worry about the other modules later, especially as I think we plan on deprecating a lot of those functions.

[JuliaKukulies]

OK, that sounds good. I can have a look into that.