Neat class for facultative imports?

[mnmelo]

So, the Style Guide mandates that any non-standard imports (anything beyond Python standard library + numpy, biopython, gridDataFormats, networkx) be imported only at function use level.
This way importing the whole of MDAnalysis or MDAnalysis.analysis won't fail if optional dependencies (e.g., scipy) aren't met.

However, code written like that gets ugly. And it's confusing for contributors that are not so versed in our rules (e.g. #574).

How about creating a module class to specifically handle such cases? So that we'd only have to type:
from MDAnalysis.lib.optional import scipy

######################
# In MDAnalysis.lib.optional
from importlib import import_module
class DummyImport(object):
    def __init__(self, modulename):
        self.modulename = modulename
    def __getattribute__(self, attr):
        raise ImportError("Module '{0}' needed for the requested functionality.".format(object.__getattribute__(self, "modulename")))

def optional_import(modulename):
    try:
        return import_module(modulename)
    except ImportError:
        return DummyImport(modulename)

scipy = optional_import("scipy")

We'd still have to specifically add each optional module to the MDAnalysis.lib.optional namespace, which prevents problems with typos such as:
from MDAnalysis.lib.optional import spicy

What do you guys think?

[jbarnoud]

I think it is neat. It allows modules to fail at the relevant time while being completely transparent.

[mnmelo]

(conflict-of-interest disclaimer: @jbarnoud now sits across the desk from me and I guess he feels morally obligated to concur with my opinions!)

[orbeckst]

Haha, thanks for the disclaimer.

But this seems an interesting solution. It also gives as a good overview of what our optional dependencies are. In particular, getting rid of per-method level imports would be a good step towards better code readability.

[kain88-de]

Do you think we can add a check on travis that lets it fail when an optional dependency is not imported like this?

[orbeckst]

On 10 Dec, 2015, at 11:05, kain88-de wrote:

Do you think we can add a check on travis that lets it fail when an optional dependency is not imported like this?

Or teach quantifiedcode to detect it?

[mnmelo]

On the technical aspect @jbarnoud and I hit some problems with the (seemingly neat) approach above, namely that the following doesn't work (fails at import-time, not runtime):

from MDAnalysis.lib.optional import scipy # No prob: a dummy module is returned if scipy unavailable
from scipy.optimize import curve_fit # Oh-oh...

I'm looking into having MDAnalysis.lib.optional set some import hooks to transparently pretend it has a tree of modules under itself. The result might be overly hacked. I'll post soon-ish (holiday season is here for me).

Teaching quantifiedcode to detect non-standard imports at module-level might be an alternative (but I imagine one will have to somehow whitelist all the standard library modules...)

[mnmelo]

On the other hand, isn't my idea moot if optional imports only happen in analysis/visualization modules? Those have to be specifically imported by the user (at least analysis ones), at which point it's safe to fail.

Note that I'm assuming that optional imports really only happen out of the main code. If it's not the case, disregard this post.

[richardjgowers]

Yeah I think that's the idea, analysis/vis have to be deliberately imported. If you're feeling bored over the holidays, I've cleaned up the Selection code a lot on the issue-363 branch, it might be less painful to work on now!

https://github.com/MDAnalysis/mdanalysis/blob/issue-363/package/MDAnalysis/core/selection.py

[orbeckst]

Maybe we can refactor all analysis/visualization code so that

• all imports are at the top level of the module (much cleaner than having some imports in methods)
• these modules are never imported by default so that missing optional dependencies do not break import MDAnalysis

and then update the Style Guide's section on importing modules. It would make the "neat approach" superfluous. (Side note: have a look at how the mock module fakes imports, in case you want to play around with the hackish approach to fake imports from packages.)

However, having a utility function such as optional_import would make it easier to add a meaningful error message. It would be nice to tell the user that they need to install module foo from bar.boing.org in order to import this analysis module.

[orbeckst]

@MDAnalysis/coredevs : The discussion on the issue of imports in the analysis package came a bit to a standstill. I propose that we amend the Style Guide on Importing Modules to say

• In MDAnalysis.analysis, all imports must be at the top level.
• No analysis module is imported automatically at the MDAnalysis.analysis level to avoid breaking the installation when optional dependencies are missing.

This applies to new code. Old code that is not compliant should be refactored (raise an issue). This solution will be simpler but create less work than the elegant approach suggested in this issue.

[richardjgowers]

So the imports should happen at the top if the module requires it. There's no point being able to import MDAnalysis.analysis.CoolThing if every function I call tells me I need scipy, just tell me at import instead.

But for example, I've got a persistence length thing which just uses scipy for a curve fitting. So 90% of the module works without scipy, and it's only if you want to fit to an exponential you need it. So the in-function import lets you have that 90%.

I thought we already did point 2 in @orbeckst 's list?