pierrepo / PBxplore

A suite of tools to explore protein structures with Protein Blocks :snake:
https://pbxplore.readthedocs.org/en/latest/
MIT License
28 stars 17 forks source link

Handle optional dependencies. #86

Closed HubLot closed 8 years ago

HubLot commented 8 years ago

Currently, if matplotlib is not present, the import of pbxplore will fail. On other hand, if weblogo or MDAnalysis is not present, the functions which use them are still present in the API and will cause errors if they are used.

This PR aims to solve this issue by using conditional imports and by manipulating the __all__ keyword. This keyword contains the public 'objects' of that module which will be exposed from a from module import *. By populate it regarding the presence/absence of the conditional imports, only the right functions will be exposed to the API. This is done silently.

For example, if matplotlib is not present, plot_neq() and plot_map() will not be accessible/visible through pbxplore.analysis.

Overall, the API will reflect the presence/absence of the optional dependencies.

jbarnoud commented 8 years ago

I guess that if somebody tries to call a function that depends on a non installed dependency, it will fail at run time when a unavailable function will be called. It would be nice if the call could fail as soon as possible and with a meaningful error.

I see two ways of doing it. The first one is to condition the function definition.

if IS_MDANALYSIS:
    def chains_from_trajectory():
        # stuff with mdanalysis
else:
    def chains_from_trajectory():
        raise RuntimeError('Requires MDAnalysis')

The second option I see is to create a fancy decorator.

def depends_on(funct, dep_name):
    check_const_name = 'IS_{}'.format(dep_name.upper())
    if check_const_name in __dict__:
        if __dict__[check_const_name]:
                funct
        else:
            def fail(*args, **kwargs):
                raise RuntimeError('Requires {}'.format(dep_name))
            return fail
    else:
        raise ValueError('Unkown conditionnal dependency {}.'.format(dep_name))

@depends_on('MDAnalysis')
def chains_from_trajectory():
    # stuff with MDAnalysis

All of this is untested and just comes on the top of my head. The decorator could work in another way by testing if the module name is in the namespace rather than testing for the IS_* variable.

pierrepo commented 8 years ago

This a brilliant idea @hsantuz! I merge the PR. @jbarnoud I much prefer your second implementation with the decorator.

HubLot commented 8 years ago

thx @pierrepo :)

Indeed, the decorator implementation is great but isn't too much? I mean, to call the function (which depends on a non-installed dependency), you have to really search for it. The only way it fails are these case:

# Matplotlib is not availabe and somebody want to use plot_neq()
import pbxplore  # plot_neq() not available
import pbxplore.analysis #plot_neq() not available

# Fail on
import pbxplore.analysis.vizualisation as vizu
vizu.plot_neq() # will fail
from pbxplore.analysis.vizualisation importe plot_neq()
plot_neq() # will fail

It has to be done on purpose meaning the user knows what he's doing. See my point?

jbarnoud commented 8 years ago

@HubLot True.

Other question, what appends if a user read the doc and wants to use pbx.chains_from_trajectory?

jbarnoud commented 8 years ago

And what about

import pbxplore
pbxplore.analysis.vizualisation.plot_neq()

?

HubLot commented 8 years ago

Other question, what appends if a user read the doc and wants to use pbx.chains_from_trajectory?

IMO, this should be explain really clearly in the docs that you need MDAnalysis (and others) for this function otherwise it wont be usable.

Indeed, pbxplore.analysis.vizualisation.plot_neq() works and will fail but it's "a lot" of trouble to load a function no? From my experience, I don't try to search functions in deep levels in numpy.

As long as we are clear in the docs, I think it's enough :)

jbarnoud commented 8 years ago

Good for me. I open an issue about that so we do not forget we have to make the doc clearer on that.