search

networkx / networkx-metis

NetworkX Addon to allow graph partitioning with METIS
Other
79 stars 21 forks source link

Setup Travis-CI #16

Closed OrkoHunter closed 9 years ago

OrkoHunter commented 9 years ago

Fixes #12 .

NOTES

But it's not ready yet. Namespace packaging is done, but there seems to be a little problem while installing. setup.py is installing a strange file named _metis.py in the same location as _metis.so with the following content

def __bootstrap__():
   global __bootstrap__, __loader__, __file__
   import sys, pkg_resources, imp
   __file__ = pkg_resources.resource_filename(__name__,'_metis.so')
   __loader__ = None; del __bootstrap__, __loader__
   imp.load_dynamic(__name__,__file__)
__bootstrap__()

This file is causing trouble and interfering while importing networkx.addons.metis._metis. Renaming the .so file doesn't help either, the new file is given the same name except for having .py extension.

Related : http://askubuntu.com/questions/442357/environment-setup-for-importing-a-userdefined-module/442363#comment917656_442363

OrkoHunter commented 9 years ago

I'm creating a topic over the cython-users mailing list. Hope something good comes out of it.

chebee7i commented 9 years ago

Is your travis-test branch completely updated? E.g. with master, setup, travis-ci all merged in? I just tried python setup.py build and I complains:

error: Namespace package problem: networkx.addons.metis is a namespace package, but its __init__.py does not call declare_namespace()! Please fix it.

If it is not completely updated, can you push it up so that others can easily debug/test.

OrkoHunter commented 9 years ago

@chebee7i Thanks for looking into it. Please give me a moment.

OrkoHunter commented 9 years ago

Pushed the changes. It's building for me. Can you please check now?

chebee7i commented 9 years ago

Builds fine, but I cannot install via: pip install -e .. That is rather unfortunate. Am I doing anything wrong?

OrkoHunter commented 9 years ago

Does your error log contains these lines?

/usr/bin/ld: cannot find -lmetis

/usr/bin/ld: cannot find -lgklib

/usr/bin/ld: cannot find -lgklib

/usr/bin/ld: cannot find -lmetis

collect2: error: ld returned 1 exit status

error: command 'x86_64-linux-gnu-gcc' failed with exit status 1
OrkoHunter commented 9 years ago

Can you try deleting networkx/addons/metis/__init__.py if it's there? And also change the line in setup.py

packages = ['networkx.addons.metis', 'networkx.addons', 'networkx'],

to

packages = ['networkx.addons', 'networkx'],
chebee7i commented 9 years ago

The reason pip install -e . does not work is because it doesn't install it as a namespace package. If I install using pip install ., then it is fine. There is, however, another issue: There are circular imports. _metis.so imports types.py. But types.py needs to import _metis. Can you make any of those imports function level imports or create a third file which has the common stuff only?

chebee7i commented 9 years ago

Should I still try the change you just suggested? I don't think it is needed.

OrkoHunter commented 9 years ago

The circular imports cause problems in the case of

from _metis import *

Doing absolute module import and using it as module.name resolves the problem of circular import. We discussed it here.

http://stackoverflow.com/a/746067/4698026

chebee7i commented 9 years ago

I'm not doing absolute imports, I'm running the code in the branch.

chebee7i commented 9 years ago

The circular import issue is still legitimate. Both _metis.pyx and types.py require that the other be available when they are imported. This will cause a circular import every time.

chebee7i commented 9 years ago

The simplest solution is to move

class MetisError(Exception):
    """An error type generated from METIS"""
    def __init__(self, rstatus):
        super(MetisError, self).__init__(rstatus)

into its own file exceptions.py. Then _metis.pyx only needs to import that. types.py will still import _metis.pyx, but the circle is broken.

OrkoHunter commented 9 years ago

Did your installed networkx core package contains networkx.addons.__init__.py with 

__import__('pkg_resources').declare_namespace(__name__)

in it? If I understand correctly, networkx-metis won't work without modifying the networkx core package.

OrkoHunter commented 9 years ago

The idea of exceptions.py sounds reasonable if the problem of circular import is still there.

chebee7i commented 9 years ago

I used exceptions.py and it fixed the import issue.

My networkx.addons.__init__.py in the core NetworkX does not have the line you mentioned and it works just fine.

chebee7i commented 9 years ago

I sent you a PR...not sure if the travis-test branch is the correct place for that. Feel free to gut the changes and place elsewhere. But I am able to build without issues after that change. I am also using your "addons" branch for NetworkX (which has a blank networkx.addons.__init__.py), even though I think you are correct that it is supposed to have that line you mentioned.

OrkoHunter commented 9 years ago

It's so astonishing for me that the actual problem (at this stage) was still the circular import which I thought was resolved. Thanks @chebee7i.

chebee7i commented 9 years ago

Glad to hear. FYI, using absolute imports cannot, in principle, resolve circular imports since a circular import has to do with access at parse time. Whether you use a relative or absolute import only changes how you access it, but it does not change whether you need access to it in the first place. To resolve it, you need to not access it at the top-level, or find some other way to break the loop.

ysitu commented 9 years ago

With #18, can networkx/addons/__init__.py be changed back into an empty file then?

OrkoHunter commented 9 years ago

With #18, can networkx/addons/init.py be changed back into an empty file then?

Things started working after #18. networkx/addons/__init__.py was not empty any time. I don't think keeping it empty will help.

OrkoHunter commented 9 years ago

Now,

In [1]: import networkx as nx

In [2]: nx
Out[2]: <module 'networkx' from '/usr/local/lib/python2.7/dist-packages/networkx-2.0.dev_20150616095129-py2.7.egg/networkx/__init__.pyc'>

In [3]: import networkx.addons

In [4]: networkx.addons
Out[4]: <module 'networkx.addons' from '/usr/local/lib/python2.7/dist-packages/networkx_metis-1.0-py2.7-linux-x86_64.egg/networkx/addons/__init__.pyc'>

After I emptied the networkx/addons/__init__.py file

In [1]: import networkx as nx

In [2]: nx
Out[2]: <module 'networkx' from '/usr/local/lib/python2.7/dist-packages/networkx-2.0.dev_20150616095129-py2.7.egg/networkx/__init__.pyc'>

In [3]: import networkx.addons

In [4]: networkx.addons
Out[4]: <module 'networkx.addons' from '/usr/local/lib/python2.7/dist-packages/networkx-2.0.dev_20150616095129-py2.7.egg/networkx/addons/__init__.py'>

In [5]: import networkx.addons.metis
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-5-44df29cab2f4> in <module>()
----> 1 import networkx.addons.metis

ImportError: No module named metis

We can see in both Out[4], the indentity of networkx.addons depends upon the content of networkx/addons/__init__.py.

ysitu commented 9 years ago

Fair enough. Let's get #18 done to unblock this.

OrkoHunter commented 9 years ago

It's strange that pylint gives positive result on my computer. Comments @ysitu ? You've tried it, right?

Latest Build https://travis-ci.org/OrkoHunter/networkx-metis/builds/68863016

ysitu commented 9 years ago

You should be able to get rid of many no-member errors by whitelisting networkx.addons.metis._metis for importing by pylint.

ysitu commented 9 years ago

If setting up pylint is much too tricky, how about trying out landscape.io?

OrkoHunter commented 9 years ago

Let me try it.

OrkoHunter commented 9 years ago

I would say we go with landscape.io. It also uses pylint, but on a moderate level. Can you set up the hook there?

ysitu commented 9 years ago

I am on an airplane right now.

OrkoHunter commented 9 years ago

Yeah you must be going. @chebee7i Ping.

chebee7i commented 9 years ago

Anything that requires admin level privileges will require action from @hagberg, @dschult, or @jtorrents .

hagberg commented 9 years ago

Let's add @chebee7i as owner - I can't do it right now @dschult, @jtorrents?

jtorrents commented 9 years ago

Done! @chebee7i is in the owner group now. I'm not sure how to set up the travis hook (still have to set the readthedocs for my fork). So I'll let @chebee7i do it.

chebee7i commented 9 years ago

Done for both metis and lemon. @OrkoHunter I need your readthedocs username as well.

OrkoHunter commented 9 years ago

My username for readthedocs is same as GitHub orkohunter. However, my fork of docs is here http://orkohunter-networkx-metis.readthedocs.org/en/doc/

chebee7i commented 9 years ago

It is case-sensitive...that is why I couldn't add you. :)

OrkoHunter commented 9 years ago

Oh.. My bad then. :)

OrkoHunter commented 9 years ago

Triggered first build with python 2.7, 3.2+, pypy and pypy3. Let's wait for the results.

OrkoHunter commented 9 years ago

Here it is, ironically it fails for Python 2.7 and passes for Python 3.2+, pypy and pypy3. The problem lies with installation. Right now, it is

python setup.py install
python setup.py test

But if we skip the installation and rather only build and test

python setup.py build
python setup.py test

it passes for all.

OrkoHunter commented 9 years ago

Well it passed for build and test as I mentioned earlier.

ysitu commented 9 years ago

With python setup.py test you are not testing the installed code.

OrkoHunter commented 9 years ago

To test the installed code, do we need to run a python script which would do some imports and run some tests within it?

ysitu commented 9 years ago

cd into some unrelated directory and run nosetests?

OrkoHunter commented 9 years ago

nosetests searches recursively for all the tests within a directory. Won't making a new unrelated directory will run 0 tests? And running nosetests from the top level will also be same as python setup.py test, and both are generating the same OS error. I'm still surprised the variation of running the test between Python 2 and 3.

ysitu commented 9 years ago

You can supply a module name to nosetests?

OrkoHunter commented 9 years ago

Yes, like nosetests only_this_test.py.

chebee7i commented 9 years ago

Look at what we do in networkx.

OrkoHunter commented 9 years ago

pypy needs special installation http://pypy.readthedocs.org/en/latest/faq.html#module-xyz-does-not-work-with-pypy-importerror

OrkoHunter commented 9 years ago

Green tick feels. Well,

chebee7i commented 9 years ago

Note, to guarantee that the (all) tests are using the installed networkx rather than the repository one, you can move directories...but also, each test file must have from __future__ import absolute_import.