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

Relative imports issue #18

Open
jchodera opened this issue May 5, 2015 · 8 comments
Open

Relative imports issue #18

jchodera opened this issue May 5, 2015 · 8 comments

Comments

@jchodera
Copy link
Contributor

jchodera commented May 5, 2015

I can't get the latest version to actually run any tests at all.

I think the issue has to do with relative imports. Instead of from bhmm import ..., everything uses relative pathnames inside of the bhmm package---especially api.py, e.g.:
https://github.com/bhmm/bhmm/blob/master/bhmm/api.py#L5-L8

from util import types as _types
from hmm.generic_hmm import HMM as _HMM
from estimators.maximum_likelihood import MaximumLikelihoodEstimator as _MaximumLikelihoodEstimator
from estimators.bayesian_sampling import BayesianHMMSampler as _BHMM

Are you testing this from within the bhmm github repo, rather than outside of the repo? If so, your python interpreter would be handling relative imports OK, but the installed version would not work if you were in a different directory (like one of the examples directories).

The error I get when running nosetests from a directory that is not the github repo is:

[LSKI1497:~] choderaj% nosetests bhmm
E
======================================================================
ERROR: Failure: ImportError (No module named hmm.generic_hmm)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/choderaj/anaconda/lib/python2.7/site-packages/nose/loader.py", line 409, in loadTestsFromName
    module = resolve_name(addr.module)
  File "/Users/choderaj/anaconda/lib/python2.7/site-packages/nose/util.py", line 312, in resolve_name
    module = __import__('.'.join(parts_copy))
  File "/Users/choderaj/anaconda/lib/python2.7/site-packages/bhmm-0.1.0-py2.7-macosx-10.5-x86_64.egg/bhmm/__init__.py", line 13, in <module>
    from api import *
  File "/Users/choderaj/anaconda/lib/python2.7/site-packages/bhmm-0.1.0-py2.7-macosx-10.5-x86_64.egg/bhmm/api.py", line 6, in <module>
    from hmm.generic_hmm import HMM as _HMM
ImportError: No module named hmm.generic_hmm

----------------------------------------------------------------------
Ran 1 test in 0.001s

There is more info on relative imports in Python here, though I'm afraid relative imports are still quite confusing to me. Absolute imports seem relatively safe.

@franknoe
Copy link
Contributor

franknoe commented May 5, 2015

Hi John,

I generally prefer absolute imports but unfortunately they are not safe.
BHMM uses pyEMMA, and we plan the next EMMA version to use BHMM. Using
absolute imports may create loops that python cannot resolve and would
produce an error.

I'm sure we can work with relative imports, because we are using them a
lot in EMMA. I'll ask Martin to have a look at it.

Thanks
Frank

Am 05/05/15 um 03:49 schrieb John Chodera:

I can't get the latest version to actually run any tests at all.

I think the issue has to do with relative imports. Instead of |from
bhmm import ...|, everything uses relative pathnames inside of the
|bhmm| package---especially |api.py|, e.g.:
https://github.com/bhmm/bhmm/blob/master/bhmm/api.py#L5-L8

|from util import types as _types
from hmm.generic_hmm import HMM as _HMM
from estimators.maximum_likelihood import MaximumLikelihoodEstimator as _MaximumLikelihoodEstimator
from estimators.bayesian_sampling import BayesianHMMSampler as _BHMM
|

Are you testing this from within the |bhmm| github repo, rather than
outside of the repo? If so, your python interpreter would be handling
relative imports OK, but the installed version would not work if you
were in a different directory (like one of the examples directories).

The error I get when running nosetests from a directory that is /not/
the github repo is:

|[LSKI1497:~] choderaj% nosetests bhmm

E

ERROR: Failure: ImportError (No module named hmm.generic_hmm)

Traceback (most recent call last):
File "/Users/choderaj/anaconda/lib/python2.7/site-packages/nose/loader.py", line 409, in loadTestsFromName
module = resolve_name(addr.module)
File "/Users/choderaj/anaconda/lib/python2.7/site-packages/nose/util.py", line 312, in resolve_name
module = import('.'.join(parts_copy))
File "/Users/choderaj/anaconda/lib/python2.7/site-packages/bhmm-0.1.0-py2.7-macosx-10.5-x86_64.egg/bhmm/init.py", line 13, in
from api import *
File "/Users/choderaj/anaconda/lib/python2.7/site-packages/bhmm-0.1.0-py2.7-macosx-10.5-x86_64.egg/bhmm/api.py", line 6, in
from hmm.generic_hmm import HMM as _HMM
ImportError: No module named hmm.generic_hmm


Ran 1 test in 0.001s
|

There is more info on relative imports in Python here
https://docs.python.org/2.5/whatsnew/pep-328.html, though I'm afraid
relative imports are still quite confusing to me. Absolute imports
seem relatively safe.


Reply to this email directly or view it on GitHub
bhmm/bhmm#18.


Prof. Dr. Frank Noe
Head of Computational Molecular Biology group
Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354
Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

@marscher
Copy link
Member

marscher commented May 5, 2015

I can not reproduce this:

 nosetests ~/sources/bhmm

does not raise the described ImportError

@marscher
Copy link
Member

marscher commented May 5, 2015

fixed by #20

@jchodera
Copy link
Contributor Author

jchodera commented May 5, 2015

I can not reproduce this:
nosetests ~/sources/bhmm

nosetests bhmm should test bhmm. This is the method we need to use for conda-distributed packages because the user will have no idea where the source files get installed, but there is still a great need to be able to test the packages after installation to make sure everything works correctly in the user's environment.

This works fine for tools like openmmtools, where nosetests openmmtools will test the suite once the user has installed it via conda.

If you want to use parts of pyemma in bhmm and use parts of bhmm in pyemma, this is going to require more than just using relative imports. We need to do some rearchitecting to make sure that no circular imports can occur. The simplest way to do this is to switch to lazy imports, where we import what we need (using absolute imports) only right before we need them. This has the potential of making debugging a bit more complicated if we forget an import and the failure only happens partway through a run, but would make it easiest to use whatever components of each tool we want without the potential for circular imports (as long as we're careful).

@franknoe
Copy link
Contributor

franknoe commented May 5, 2015

I am using exclusively lazy imports to pyemma in bhmm.

Am 05/05/15 um 19:48 schrieb John Chodera:

I can not reproduce this:
nosetests ~/sources/bhmm

|nosetests bhmm| should test |bhmm|. This is the method we need to use
for conda-distributed packages because the user will have no idea
where the source files get installed, but there is still a great need
to be able to test the packages after installation to make sure
everything works correctly in the user's environment.

This works fine for tools like |openmmtools|
https://github.com/choderalab/openmmtools, where |nosetests
openmmtools| will test the suite once the user has installed it via conda.

If you want to use parts of |pyemma| in |bhmm| and use parts of |bhmm|
in |pyemma|, this is going to require more than just using relative
imports. We need to do some rearchitecting to make sure that no
circular imports can occur. The simplest way to do this is to switch
to lazy imports, where we import what we need (using absolute imports)
only right before we need them. This has the potential of making
debugging a bit more complicated if we forget an import and the
failure only happens partway through a run, but would make it easiest
to use whatever components of each tool we want without the potential
for circular imports (as long as we're careful).


Reply to this email directly or view it on GitHub
bhmm/bhmm#18 (comment).


Prof. Dr. Frank Noe
Head of Computational Molecular Biology group
Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354
Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

@jchodera
Copy link
Contributor Author

jchodera commented May 5, 2015

I am using exclusively lazy imports to pyemma in bhmm.

So there shouldn't be any issues if we convert to absolute import paths, should there?

@franknoe
Copy link
Contributor

franknoe commented May 5, 2015

No that should be fine. I now get your point, although I don't
understand why my normal nosetests would work while it wouldn't work in
the conda test environment. I'll change to absolute imports and give you
a PR

Am 05/05/15 um 19:55 schrieb John Chodera:

I am using exclusively lazy imports to pyemma in bhmm.

So there shouldn't be any issues if we convert to absolute import
paths, should there?


Reply to this email directly or view it on GitHub
bhmm/bhmm#18 (comment).


Prof. Dr. Frank Noe
Head of Computational Molecular Biology group
Freie Universitaet Berlin

Phone: (+49) (0)30 838 75354
Web: research.franknoe.de

Mail: Arnimallee 6, 14195 Berlin, Germany

@jchodera
Copy link
Contributor Author

jchodera commented May 5, 2015

Sounds great, thanks!

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

No branches or pull requests

3 participants