Standardize management tools:

[pwalsh]

• make for tasks (no deps, simple, works)
• pylint
• let users manage their own venv
• info.json available inside package so can be used, for example, in __version__ as well as setup
• follow common practice of not having requirements.txt for libs
• changes to various CLI config files
• prep travis for automating package deployment

@roll this is not for merging right at this moment: it is for your feedback. Sorry I'm using your codebase to test this out, but, you know why we are doing this :).

[pwalsh]

@amercader @brew @vitorbaptista @akariv @roll

The above setup is based on various efforts around standardizing how we do this, that includes input from pretty much all of us.

I'm integrating this setup into our coding standards. The goals are:

• common tooling on our libraries
• lack of surprise on new code bases
• always, continuous deployment (for apps and packages)

Feel free to comment here, or, after I add to https://github.com/okfn/coding-standards contest via PRs :).

[roll]

My feedback:

make for tasks (no deps, simple, works)

It looks like the best way for now. Weak place in python ecosystem =(

pylint

Don't have enough knowledge on the topic. Pylint looks like a standard.

let users manage their own venv

When packages will be widely public facing it's a good idea to not have helpers for env activation (could be used in internal packages with git hooks to ensure no broken builds pushed).

info.json available inside package so can be used, for example, in version as well as setup

The essential one to have version at runtime. Just didn't have time to do it)

follow common practice of not having requirements.txt for libs

Don't like the idea having different setup.py file for every package. I think we have to reduce files having unique content between the packages.

Our standard by default list could include:

• .coveragerc
• .gitignore
• .travis.yml
• LICENSE.md
• MANIFEST.in (using wildcards?)
• Makefile (getting PKG somehow?)
• pylintrc
• setup.cfg
• setup.py
• tox.ini

That way a actual data could be in:

• requirements (?)
• info.json

Many files to do not maintain for every project)

[roll]

Also:

• remove .editorconfig - useless for python
• install_requirements could be part of info.json

[vitorbaptista]

Instead of having a separate file for tests requirements, why not simply add the requirements here? Any advantages on having tests_require in setup.py?

[pwalsh]

I think it is good to be able to pip install the test reqs (see make install). As for having it in setup.py, it is friendly for anyone who uses python setup.py test

[roll]

to give contributor do pip install -r requirements.txt

[vitorbaptista]

As we're using tox, we should never need to install the test dependencies. python setup.py test simply calls tox, which handles that for us.

[pwalsh]

@vitorbaptista would you recommend adding an explicit pip install tox to make install, or, just telling the dev that they need to install tox? I don't mind changing pip install tests_require to pip install tox and letting tox handle test deps actually... it just still looks a bit foreign to me. In general, I think you are right that we just let tox handle it.

[pwalsh]

make install will bring deps + tox. But yes, you'd then have to run tox at least once in order to be able to run the linter or coverage, as they'd only be declared in tox with the suggested change. Probably not so good, to change, then.

[roll]

deps declared where?

I mean if remove extenal file.

[pwalsh]

in setup.py. pip install -e . uses setup.py

[roll]

But it also installs the package by itself to development environment?

[roll]

I'm asking because if we have installed our package (python setup.py install) to dev venv it could lead to magical bugs because of 2 copies of package in PYTHONPATH.

Again - i never did it that way - correct me if i wrong.

[roll]

Another suggestion:

• to have dedicated CONTRIBUTING.md file for lib contributors - standard by default for all our packages with make test etc described stuff (also it's nicely supported by github)
• to have clean README.md for lib users because it's 99% of readers.

https://github.com/okfn/datapackage-storage-py (section How to contribute with link needed).

[vitorbaptista]

I think moving to a Makefile instead of run.py is much better, but if we follow a few common patterns, we won't need it at all.

Installing a Python package should work the same: python setup.py install. The other rules in our makefile are test, lint and coverage. Test and coverage are already covered by our current tox.ini. For linting, the pattern used by (tox itself)[https://bitbucket.org/hpk42/tox/src/2d5d0e7584cc4cc35cc7e0519ce9610dd52b7a62/tox.ini?fileviewer=file-view-default] is to have an extra environment to run pylint.

With these few changes, the only thing the user needs to know is that we use tox. Everything works as expected.

[vitorbaptista]

Adding to my last comment, if we configure the test_suite in setup.py to run tox, the user doesn't need to know we're using it at all. He simply uses the standard python setup.py install and python setup.py test (considering he has tox installed).

[akariv]

We should probably get the name from the info.json file instead of repeating.

[pwalsh]

I guess, good idea.

[roll]

@pwalsh
we're all almost agreed on all the points (make etc) except having the long discussion on requirements.

But taking another look on external files soultion:

requirements.txt
requirements.dev.txt (including -r requirements.txt)

what is technical objections against it? It's:

• DRY
• declarative
• I suppose easy to find and intuitive (packages deps and dev requirements)

Now we have many tech problems to substitute it but initial problem was like it's bad naming? May be rename it to dependencies.txt (install_requires) and requirements.txt (like for apps) or other simple tweak?

[vitorbaptista]

To give a better example on what I think is the best approach, check https://github.com/datapackages/datapackage-py.

Everything related to the metadata about the package is in setup.py, except for the README. This keeps that file quite straightforward. The version is only in that setup.py, so the user can't find it programatically. There are a few ways of solving this, as we're talking in #40 (diff), but to be honest I would simply duplicate the version number in setup.py and datapackage/__init__.py. Although we want our code to be DRY, we have to balance that with keeping the code simple. This is the pattern followed by nosetests (see setup.py and nose/init.py).

If you want to run tests, you'd have to have tox installed in the first place. Then you can just run tox and everything will work.

I'm not linting the code, but to do so I would just add a new environment to my tox.ini, and nothing would change for the user. Simply calling tox will still work.

The main advantage of this structure is that we're not adding any extra abstractions on top of what Python already offers. There's no extra Makefile or JSON files to maintain. The user can simply clone the code and call tox, the same as with any other codebase that uses it. The same goes for people that want to contribute code: there's no pattern they need to learn that are specific to it, everything follows tox's and setup.py's conventions.

[pwalsh]

@vitorbaptista of course, a lot of this is based on that package. But the problem I see there is that you are turning tox into a task runner, which may or may not be ok, but I don't think it is as transparent as you make out. Definitely, I think it is important to be able to run lint and coverage reports as distinct from a normal test run (especially lint).

[roll]

@vitorbaptista
I'm not so familiar with tox (I suppose as the biggest part of our contributors) - what I should do in datapackage-py if I want to start ipython to play with package while I'm changing the source code?

[vitorbaptista]

Although I think that linting should be part of the default test suite, we can easily set lint to run only when calling tox -e lint, and not by default. The required changes in https://github.com/datapackages/datapackage-py/blob/master/tox.ini are simply adding at the end:

[testenv:lint]
deps=
  {[testenv]deps}
  pytest-pylint
commands=
  py.test \
    --pylint

If we don't specify lint into the envlist, it's not called by default. Then we just need to make sure Travis calls both tox and tox -e lint.

[vitorbaptista]

@roll I'm not that familiar with ipython, but tox isn't needed unless you want to run the tests. You meant how to run the tests from inside ipython? If so, I don't know, but given that ipython itself uses tox, I would guess there would be no issues between the two libraries.

All in all, I think tox solves a ton of issues with python and testing and is becoming the de facto standard (if it hasn't yet). For this reason, I think we're safer exposing it to the end users than trying to hide it behind some abstraction.

[pwalsh]

@vitorbaptista I agree tox solves a ton of issues, but that example above of trying to use tox as a runner for a linter is a vastly different experience to just running pylint $(PKG) periodically, or via a git hook before committing.

[roll]

I checked Paul's pip install -e . - it looks like it's how to have my own virtual environment with installed dependencies (no real installing of our package is happening - sorry I have never used this pip option).

Now I almost don't see blockers to make it tox centric as @vitorbaptista suggests.

[roll]

About version. It's a small thing but a real important thing. It's a real trap. I thinks we have to avoid to have it in 2 places - even we may be have to add test before release to check declared version matches git tag version (just check current tabulator git tag and declared version =( ).

Solution 1

Parse it in setup.py.
We're reading readme and licence - no big deal to read __init__.py and use regexp to get it

Solution 2 (?)

Have someone experience of getting version at runtime?

>>> import pkg_resources
>>> pkg_resources.get_distribution("blogofile").version
'0.7.1'

I suppose it's a bad idea because some collisions could be introduced etc? But I haven't tried it.

[roll]

tox -e lint sounds a little bit hucky but passing tox's parameters thru make will be also kinda a hack. Doesn't it? Also cons against make - tabs and other things why other tools was born.

[vitorbaptista]

Considering that we keep our test dependencies into tox.ini, there's no way to run pylint without duplicating (or extracting) them. Both pip and tox itself run not only lint, but also test for failures in their documentation using tox.

To be clear, I'm not saying that we should follow this pattern just because others are using it. There're plenty of ways to do the same thing. I'm just giving these examples as indications that we're not overusing Tox for something it was not intended to.

[pwalsh]

This is great @vitorbaptista keep it coming. I guess I'm seeing two distinct use cases here: running on CI, and some helper tools during development.

[roll]

Ohh.. but it looks like pip install -e . doesn't install test_requires so it's still not enough to prepare contributor's venv to work with.

So I think this https://github.com/datapackages/datapackage-py/blob/master/setup.py + version extraction + requirements.txt (with -e . and tox included - for dev env).

Than on getting started to contributors we just need to write:

pip install -Ur requirements.txt

All others (test, review) will be tox calls.
Anyway python packaging is broken so many time for so simple things =(

[vitorbaptista]

@pwalsh One of the specific goals for tox is to make testing on CI and testing locally the same:

What is Tox?
Tox is a generic virtualenv management and test command line tool you can use for:

• checking your package installs correctly with different Python versions and interpreters
• running your tests in each of the environments, configuring your test tool of choice
• acting as a frontend to Continuous Integration servers, greatly reducing boilerplate and merging CI and shell-based testing.

https://testrun.org/tox/latest/

@roll In Tox's FAQ, there's a suggestion on how to integrate tox with setuptools test commands, so python setup.py test would work.

I think it's a bit overkill, though. If there're issues with test_requires, I would suggest us to simply document that we're using tox and ask the user to install it herself. We could also add it to a requirements.dev.txt or something similar, I have no preference either way.

[roll]

@vitorbaptista
About test_require I've meant the most common workflow to contribute:

git clone project
cd project
virtualenv venv (here could be some venv wrappers and tools etc)
source venv/bin/activate
# i want to install dev (install+test) deps into venv

Then contributor needs to install install_requires and tests_require to this venv. So I'm just thinking how to do it having all requirements listed in setup.py like in datapackage-py.

[pwalsh]

@pwalsh One of the specific goals for tox is to make testing on CI and testing locally the same

Yes, I get that. The whole point of the Makefile is an interface to run tasks, not only tests.

[roll]

@amercader @brew @vitorbaptista @akariv @pwalsh

Please take a look - #41