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

Add support for tox and use flake8 #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pabelanger
Copy link

Where is a simple patch to enable tox support and use flake8. Flake8 is still not passing at 100%, mostly because I don't understand the application at this point.

Nice work BTW!

This adds support for tox to create a virtual environment for all
development.  You can invoke it by making sure tox is installed:

   $ pip install tox

Then running:

   $ tox

from the local directory.

Signed-off-by: Paul Belanger <[email protected]>
Signed-off-by: Paul Belanger <[email protected]>
@leedm777
Copy link
Member

So my Python-fu is much weaker than yours, but doesn't this break the setup.py script? It no longer has dependencies listed, so the ./setup.py {install,develop} doesn't install them, and they don't get installed when you install the egg.

And nose will fail if the build directory doesn't exist. Please either get nose to not fail, or put it back :-)

@pabelanger
Copy link
Author

You'd use tox to handle the creation of eggs now:

$ tox -evenv -- python setup.py install
$ tox -evenv -- python setup.py develop

This way, you don't have any libraries on your local box.

@leedm777
Copy link
Member

Normally I manage my own virtualenv's for avoid local installs. Tox looks cool, though. But it shouldn't break the setup.py.

python setup.py develop should still setup for development, python setup.py nosetests should run the tests.

Regardless, the egg doesn't have dependencies in it, which seems very wrong.

$ git clean -xdff
$ tox -evenv -- python setup.py bdist_egg
# <snip/>
___________________________________ summary ____________________________________
  venv: commands succeeded
  congratulations :)
$ virtualenv tmp
New python executable in tmp/bin/python
# <snip/>
done.
$ . tmp/bin/activate
(tmp)$ easy_install dist/swaggerpy-0.0.1-py2.7.egg 
Processing swaggerpy-0.0.1-py2.7.egg
Copying swaggerpy-0.0.1-py2.7.egg to /Users/dlee/prj/swagger/swagger-py/tmp/lib/python2.7/site-packages
Adding swaggerpy 0.0.1 to easy-install.pth file
Installing swagger-codegen script to /Users/dlee/prj/swagger/swagger-py/tmp/bin

Installed /Users/dlee/prj/swagger/swagger-py/tmp/lib/python2.7/site-packages/swaggerpy-0.0.1-py2.7.egg
Processing dependencies for swaggerpy==0.0.1
Finished processing dependencies for swaggerpy==0.0.1
(tmp)$ pip freeze
swaggerpy==0.0.1
wsgiref==0.1.2

And tox fails due to missing {{./build}} directory.

$ git clean -xdff
(tmp)[dlee@dlee-mac swagger-py (tox)]$ tox
GLOB sdist-make: /Users/dlee/prj/swagger/swagger-py/setup.py
py27 create: /Users/dlee/prj/swagger/swagger-py/.tox/py27
py27 installdeps: -r/Users/dlee/prj/swagger/swagger-py/requirements.txt, -r/Users/dlee/prj/swagger/swagger-py/test-requirements.txt
py27 inst: /Users/dlee/prj/swagger/swagger-py/.tox/dist/swaggerpy-0.0.1.zip
py27 runtests: commands[0] | nosetests
# <snip/>
IOError: [Errno 2] No such file or directory: 'build/nosetests.xml'
ERROR: InvocationError: '/Users/dlee/prj/swagger/swagger-py/.tox/py27/bin/nosetests'

@pabelanger
Copy link
Author

Nose is fixed.

Also, here is the preferred way to handle dependencies in python these days: http://www.pip-installer.org/en/latest/cookbook.html

@leedm777
Copy link
Member

I had the separate nose.cfg so that IntelliJ/PyCharm can run the tests.

I still don't like not having meta-data in the package for dependencies. That page reads more like it's talking about managing application installation, not library dependencies.

Tell you what:

  • Put nose.cfg back
  • Add the metadata back to setup.py.
    setup_requires=['nose'],
    tests_require=open('test-requirements.txt').readlines(),
    install_requires=open('requirements.txt').readlines(),
  • Update the README

I think that would cover the last of my complaints.

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

Successfully merging this pull request may close these issues.

2 participants