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

adding support for installing python wrapper with setuptools or PyPI #1012

Merged
merged 9 commits into from Jun 21, 2016

Conversation

gramhagen
Copy link
Contributor

General cleanup of state of the python wrapper. I see this as the first stage to providing support for python 2 and 3, by allowing us to improve python wrapper build based on the platform, python version and libraries installed.

Note, I have only been able to test this on MacOS. So it would be great if someone can try to run 'pip install -e .' with windows or linux.

I dumped most of the non-critical py files in examples, will need to walk through them and clean up anything old and transition anything test-related to the tests folder.

I have been testing this on the test PyPI server, I can upload it once merged to the real PyPI server and setup additional folks as authors so they manage the package there too.

@JohnLangford
Copy link
Member

@hal3 can you take a look at this?

@irustandi
Copy link

irustandi commented Jun 14, 2016

On Anaconda Python with Python 2.7.11 running on Ubuntu 15.10, I got the following error when running "pip install -e .":

Obtaining file:///home/indra/git_repos/vowpal_wabbit/python
Installing collected packages: vowpalwabbit
Running setup.py develop for vowpalwabbit
Complete output from command /home/indra/anaconda2/bin/python -c "import setuptools, tokenize;file='/home/indra/git_repos/vowpal_wabbit/python/setup.py';exec(compile(getattr(tokenize, 'open', open)(file).read().replace('\r\n', '\n'), file, 'exec'))" develop --no-deps:
running develop
running egg_info
writing vowpalwabbit.egg-info/PKG-INFO
writing top-level names to vowpalwabbit.egg-info/top_level.txt
writing dependency_links to vowpalwabbit.egg-info/dependency_links.txt
warning: manifest_maker: standard file '-c' not found
reading manifest file 'vowpalwabbit.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching '' under directory 'src'
warning: no previously-included files matching '
.o' found anywhere in distribution
warning: no previously-included files matching '.exe' found anywhere in distribution
warning: no previously-included files matching '
.pyc' found anywhere in distribution
writing manifest file 'vowpalwabbit.egg-info/SOURCES.txt'
running build_ext
error: [Errno 2] No such file or directory: '/home/indra/git_repos/vowpal_wabbit/python/src'

@gramhagen
Copy link
Contributor Author

ok thanks for checking that, can you try: da2b544 ?

@irustandi
Copy link

Yes, now I'm able to run "pip install -e ." to completion without any errors.

@irustandi
Copy link

irustandi commented Jun 15, 2016

Also, here's an example of setup.py that is able to find the appropriate boost-python library to link, at least for Linux and Mac:

https://github.com/casacore/python-casacore/blob/master/setup.py

@gramhagen
Copy link
Contributor Author

ok, borrowed some ideas from that project. i need to confirm this works for python3 on osx and linux and for both 2 and 3 under cygwin. Still not sure what's causing the travis ci failures, I've been trying to replicate it using a docker instance of travis, but it works there... =(

It's also fairly heavy to have to install numpy, scipy and sklearn. May want to look into changing the sklearn dependency to use a shell like they do here: https://github.com/dmlc/xgboost/tree/master/python-package

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 70.123% when pulling 263730e on gramhagen:pypi_prep into 043a03b on JohnLangford:master.

@gramhagen
Copy link
Contributor Author

brought in miniconda to help with travis, but unfortunately that breaks tox (sigh), so had to execute testing manually, this will likely get a little messier in the .travis.yml file when we need python3 support as well, but it should work.

@JohnLangford
Copy link
Member

Is this ready to go? Looking through, the only obvious thing that stood out to me is that it would be nice to have the version derived from the VW version rather than stated.

I'm going to drop a new version soon, so if it's ready, we should go.

@gramhagen
Copy link
Contributor Author

yeah, I can pull it from configure.ac if that's the best place?

@JohnLangford
Copy link
Member

it is, yes.

-John

On Mon, Jun 20, 2016 at 11:59 AM, Scott Graham notifications@github.com
wrote:

yeah, I can pull it from configure.ac if that's the best place?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1012 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAE25sOEvY8lglhGe2uk9k-oRAv2AFx9ks5qNrjagaJpZM4I0xf-
.

@gramhagen
Copy link
Contributor Author

ok, also it's not my intent to claim authorship in the package description, that was a placeholder. Should I put John Langford and jl@hunch.net as author/email?

@JohnLangford
Copy link
Member

@gramhagen
Copy link
Contributor Author

The build failed because the repo included a python notebook that is not in the current branch I'm working out of, and I was checking to make sure those were handled in the python package manifest. It's a bit mysterious to me why that would show up, but the manifest check is not really critical for build testing. So I've removed it to prevent it from breaking in the future. It is still in the python packaging instructions and tests.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 70.486% when pulling 4a4ac12 on gramhagen:pypi_prep into 043a03b on JohnLangford:master.

@JohnLangford JohnLangford merged commit 4a4ac12 into VowpalWabbit:master Jun 21, 2016
@JohnLangford
Copy link
Member

Merged, thanks.

@gramhagen gramhagen deleted the pypi_prep branch November 28, 2016 21:31
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.

None yet

4 participants