Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle optional dependencies. #86

Merged
merged 1 commit into from
Oct 8, 2015

Conversation

HubLot
Copy link
Collaborator

@HubLot HubLot commented Oct 8, 2015

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.

If an optional package is not present (matplotlib, weblogo, mdanalysis),
the import will not fail and the functions which use them won't be
exposed to the API. This is done with the manipulation of the __all__ keyword.
@jbarnoud
Copy link
Collaborator

jbarnoud commented Oct 8, 2015

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
Copy link
Owner

pierrepo commented Oct 8, 2015

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

pierrepo added a commit that referenced this pull request Oct 8, 2015
@pierrepo pierrepo merged commit d036b3f into pierrepo:master Oct 8, 2015
@HubLot
Copy link
Collaborator Author

HubLot commented Oct 8, 2015

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
Copy link
Collaborator

jbarnoud commented Oct 8, 2015

@HubLot True.

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

@jbarnoud
Copy link
Collaborator

jbarnoud commented Oct 8, 2015

And what about

import pbxplore
pbxplore.analysis.vizualisation.plot_neq()

?

@HubLot
Copy link
Collaborator Author

HubLot commented Oct 8, 2015

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
Copy link
Collaborator

jbarnoud commented Oct 8, 2015

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

@HubLot HubLot deleted the conditional_import branch October 8, 2015 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants