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

Building from source failed tests that import random #32

Closed
logust79 opened this issue Jan 4, 2018 · 6 comments
Closed

Building from source failed tests that import random #32

logust79 opened this issue Jan 4, 2018 · 6 comments

Comments

@logust79
Copy link

logust79 commented Jan 4, 2018

On Ubuntu 16.04 TLS, I built it with Python 2.7 and tensorflow 1.4.1. It failed all tests that import random (e.g. deepvariant/deepvariant/core/cloud_utils_test.py). It turned out that in random.py, it imports math, and it mistakingly imports the math.py in /deepvariant/deepvariant/core/, instead of from the standard library. Is there a fix?

@depristo
Copy link

depristo commented Jan 4, 2018

To be 100% clear, are you saying you booted a clean ubuntu 16 instance and it failed to build there? Or is this on an already customized machine?

@logust79
Copy link
Author

logust79 commented Jan 4, 2018

It is customised. And Python 2.7 was installed using Anaconda.

@logust79
Copy link
Author

logust79 commented Jan 5, 2018

To make it clearer, I put the path structure here.

/deepvariant/core/
    cloud_utils_test.py
    math.py
    ...

And in cloud_utils_test.py:

"""Tests for deepvariant .core.cloud_utils."""

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

import httplib
...

Through httplib, it imports mimetools, which imports tempfile, which imports ramdom, which imports math.
But since there is a math.py in the same path, it shadows the math module in python's standard library, causing an error.

To test the hypothesis, simply importing httplib in the same path caused the following error:

>>> import httplib
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "xx/anaconda/envs/Python27/lib/python2.7/httplib.py", line 80, in <module>
    import mimetools
  File "xx/anaconda/envs/Python27/lib/python2.7/mimetools.py", line 6, in <module>
    import tempfile
  File "xx/anaconda/envs/Python27/lib/python2.7/tempfile.py", line 35, in <module>
    from random import Random as _Random
  File "xx/anaconda/envs/Python27/lib/python2.7/random.py", line 45, in <module>
    from math import log as _log, exp as _exp, pi as _pi, e as _e, ceil as _ceil
  ***File "math.py", line 79, in <module>***
    import numpy as np
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/__init__.py", line 142, in <module>
    from . import add_newdocs
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/add_newdocs.py", line 13, in <module>
    from numpy.lib import add_newdoc
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/lib/__init__.py", line 8, in <module>
    from .type_check import *
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/lib/type_check.py", line 11, in <module>
    import numpy.core.numeric as _nx
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/core/__init__.py", line 74, in <module>
    from numpy.testing.nosetester import _numpy_tester
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/testing/__init__.py", line 12, in <module>
    from . import decorators as dec
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/testing/decorators.py", line 20, in <module>
    from .utils import SkipTest, assert_warns
  File "xx/anaconda/envs/Python27/lib/python2.7/site-packages/numpy/testing/utils.py", line 15, in <module>
    from tempfile import mkdtemp, mkstemp
ImportError: cannot import name mkdtemp
>>> 

As you can see, it attempts to load the local math.py, shadowing the standard math module.
On the other hand, cd to another path and import httplib does not have any problem.

@depristo
Copy link

depristo commented Jan 5, 2018

I agree this is a problem. We have an internal bug tracking this. Probably we will just rename deepvariant/core/math.py to core_math.py or equivalent. I'll update this bug when the change is in internally and it'll show up in the next push of deepvariant to github. Note you can workaround this issue just like https://github.com/notoraptor/deepvariant/commit/15c2deb211672a8ba32c1cbe609d81e1a2b0fb74

@logust79
Copy link
Author

logust79 commented Jan 5, 2018

Many thanks for pointing me to the workaround! Looking forward to the next push!

@depristo
Copy link

depristo commented Jan 8, 2018

A fix is in in google, renaming math.py to genomics_math.py, which should fix the problem. The next major push of functional changes to DeepVariant will include this update.

@depristo depristo closed this as completed Jan 8, 2018
pichuan pushed a commit that referenced this issue Jan 30, 2018
The previous name (math.py) can shadow Python's standard math module in some open-source uses, so we rename it to be both clearer (it's genomics oriented math functions) and to avoid the shadowing. As part of this we remove one of our shadows (import math as py_math), which is nice.

Addresses #32 and the workaround seen in https://github.com/notoraptor/deepvariant/commit/15c2deb211672a8ba32c1cbe609d81e1a2b0fb74.

PiperOrigin-RevId: 181175216
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

2 participants