Feature/management update

[roll]

Work based on #40 PR and discussion.

All things in PR above

Included - pylint, travis, no activate, no editorconfig, no requirements.txt etc

Make for tasks

As discussed in the PR topic tox is a great test-runner but make is better suitable for run tasks. Also one of the main thing - there is no need to install make to do initial dependency install.

All metadata in setup.py

As suggested by @vitorbaptista the most pythonic way is to have all package metadata (except readme and version) in setup.py - that's true almost all devs look into setup.py to read it or figure out where to find it. So no package/info.json, requirements*.txt. Version has to be parsed from package (TODO, very simple).

All tools available

After make install in venv contributor will be having all tools prepared to work. Environment variable has been removed from tox so all the tools py.test, pylint, tox etc could be used as usual without make call. Of course on high-level we provide make review and make test.

DRY

Unique files per project:

• .coveragerc (may be we'll find a workaround how to remove hardcoded package name from here)
• Makefile (may be we'll find a workaround how to remove hardcoded package name from here)
• README.md
• setup.py

Initially standard files per project:

• .gitignore
• .pylintrc
• .travis.yml
• CONTRIBUTING.md
• LICENSE.md
• MANIFEST.in
• setup.cfg
• tox.ini

Basic contribution guide (only tech aspects) for this proposal - https://github.com/datapackages/tabulator-py/blob/feature/management-update/CONTRIBUTING.md - deadly simple with no deps to start.

[pwalsh]

@roll cool. I've got a version that looks almost the same to push to a generic example repo, hopefully today.

[roll]

@pwalsh
Take a look on extras_require - It looks I've finally found a way how to friend venv deps installation and having no requirements files.

With this tweak @vitorbaptista's suggestion about setup.py starts to shine because all variable package metadata starting to live in setup.py.

[vitorbaptista]

AFAIK, the license here should be just MIT, BSD-3, GPL, etc., and not the actual license text.

[roll]

I had the same feeling when writing it. I don't know why we read it. It looks like the issue was introduced earlier [TOFIX]

[pwalsh]

that is correct, it shouldn't be here. usually people concat license text into readme for long description.

[vitorbaptista]

Great job, @roll! This is looking really good! I just have a couple suggestions.

Tox

Instead of using setup.py's test_requires, let tox handle them. The only requirement in test_requires would be tox itself. The non-test dependencies don't need to be there as well, as tox automatically installs our package (which should bring all of its dependencies). The tox.ini would look like:

[tox]
envlist=
  py27
  py33
  py34
  py35

[testenv]
deps=
  pylint
  mock
  pytest
  pytest-cov
  coverage
  coveralls
commands=
  py.test tests \
    --cov-config .coveragerc \
    --cov-report term-missing \
    {posargs}

Actually, linting in my opinion is also a type of test, so it also should be handled by tox. With this sugestion, the tox.ini would become:

[tox]
envlist=
  py27
  py33
  py34
  py35
  lint

[testenv]
passenv = TRAVIS TRAVIS_JOB_ID TRAVIS_BRANCH
deps=
  mock
  pytest
  pytest-cov
  coverage
  coveralls
commands=
  py.test \
    --cov tabulator \
    --cov-config .coveragerc \
    --cov-report term-missing \
    {posargs}

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

Note that I've also added passenv, for it to work with Travis, and added --cov tabulator, that I think is required by coverage. With this configuration, tox will run the tests on all python versions we support and also lint the code by default. If we'd rather not lint by default, we just need to remove lint from the envlist.

My reasoning for linting by default is that, when developing datapackage-py with tox, I used tox -e py27,py35 to make the tests run faster. Just when I was about to commit that I ran just tox, to test with all versions. I think most of us will follow this pattern to keep the tests quick when developing, so I would prefer tox to run everything instead of having to remember to call tox && tox -e lint before commiting.

I'm happy with whatever is decided, though.

Makefile

Considering that Tox handles testing and linting, there are two remaining rules in the Makefile: running coveralls and installing the packages.

We will only run coveralls inside Travis, so adding that logic inside .travis.yml or tox.ini (following the pattern in https://github.com/datapackages/datapackage-py/blob/master/tox.ini#L25) makes more sense to me. Actually, we might have to add that into Tox for it to work.

There's also nothing special in how to install Tabulator. Simply running pip install -e .[development], which is standard across python packages, should work. I don't think adding that to our Makefile makes it easier for the users.

Given that, I think a Makefile is unnecessary for tabulator. It would be useful for packages that use Sphinx for the docs or other build tasks, but most of our projects won't have any.

[pwalsh]

@vitorbaptista @roll great stuff.

Other stuff I have in a WIP here on machine:

• I moved only version number to a VERSION plain text file inside package: is read by setup.py and also init of package
• I added a release task to make, which:
  • reads the VERSION
  • creates a tag from master with the version number (fails if version exists as tag, which is good)
  • pushes to origin

Then the CI is configured to deploy to pypi on successful build of any tag, using Travis' integration for that.

These are the type of tasks that I want to keep a makfile around for.

[pwalsh]

Also, make lint: pylint $(PACKAGE) is much more convenient (faster) than the tox config for that. I want to be able to lint without tests. but I can just do pylint $(PACKAGE) directly, of course.

[vitorbaptista]

About the make release, good example. Makes sense 👍

Regarding pylint, I prefer using tox because it handles the dependencies (pylint and pytest-pylint), linting is part of the test suite (IMHO), and it seems to be the usual pattern (used including by both pylint and flake8).

[roll]

@vitorbaptista
If we use setup.py - test_require = ['tox'] and move other deps to tox.ini than:

$ git clone package
$ cd package
$ virtualenv venv
$ source venv/bin/activate
$ pip install -e .[development]
$ py.test/pylint etc

fails because in our venv only tox installed - not our test dependencies. They will be installed only in tox's venvs (if I correctly understand how tox works).

[pwalsh]

@roll that is right. I like the whole thing @vitorbaptista has brought in here with tox very very much, but I want to be able to run pylint etc in a frictionless way too. Because of this, I think we still use requirements.txt and requirements.dev.txt which @akariv also seemed to prefer.

Here is a slightly cleaner setup.py for that, based on what you had in the storages repo (expects text file VERSION in base path of package):

# -*- coding: utf-8 -*-
from __future__ import division
from __future__ import print_function
from __future__ import absolute_import

import os
import io
import json
from setuptools import setup, find_packages


def read(f, method='read'):
    """Read a text file at the given relative path."""
    return getattr(io.open(f, encoding='utf-8'), method)().strip()


def clean(deps):
    """Clean invalid lines for the deps list for setup.py."""
    invalid = ('#', 'git+', 'hg+', '-e', '-r', '--i', '--index-url')
    return [d for d in deps if not d.startswith(invalid)]

PACKAGE = 'opendata'
VERSION = read(os.path.join(PACKAGE, 'VERSION'), 'readline')
README = '{0}\n\n{1}'.format(read('README.md'), read('LICENSE.md'))
INSTALL_REQUIRES = clean(read('requirements.txt'), 'splitlines')
TESTS_REQUIRE = clean(read('requirements.dev.txt'), 'splitlines')


setup(
    name=PACKAGE,
    version=VERSION,
    description='We want the data raw, and we want the data now.',
    long_description=README,
    author='Open Knowledge International',
    author_email='info@okfn.org',
    url='https://github.com/okfn/coding-standards',
    license='MIT',
    include_package_data=True,
    packages=find_packages(exclude=['tests']),
    package_dir={PACKAGE: PACKAGE},
    install_requires=INSTALL_REQUIRES,
    tests_require=TESTS_REQUIRE,
    test_suite='tox',
    zip_safe=False,
    keywords='raw data',
    classifiers=[
        'Development Status :: 4 - Beta',
        'Environment :: Web Environment',
        'Intended Audience :: Developers',
        'License :: OSI Approved :: MIT License',
        'Operating System :: OS Independent',
        'Programming Language :: Python :: 2',
        'Programming Language :: Python :: 2.7',
        'Programming Language :: Python :: 3',
        'Programming Language :: Python :: 3.3',
        'Programming Language :: Python :: 3.4',
        'Programming Language :: Python :: 3.5',
        'Topic :: Internet :: WWW/HTTP :: Dynamic Content',
        'Topic :: Software Development :: Libraries :: Python Modules'
    ]
)

[roll]

Also making tox.ini different for examples for 10 GOOG integrations = +10 tox.ini to maintain :)

I think we have to be flexible here - if some big packages with many contributors like datapackage-py want to move test_require to tox.ini because of a solid reason - let be ok with it.

The main thing of unification could be maintaining off small similar packages. Let minimize here time to work with them! Like every developer should know only setup.py and <package>/VERSION store information about the package (+ requirements if you prefer that way)

[pwalsh]

@roll sure, everything is and should be flexible. But as you see, we have a wide range of coding styles across various packages, so while of course, every package will still have its own particulars, we're doing this bike shedding now to help ease the burden as new people come in, and so that there is more or less the same pattern for anyone who wants to contribute. It is a real issue right now across our Python and JS libs.

[roll]

I think we still use requirements.txt and requirements.dev.txt

It's a standard (some requirements.txt) - no doubts dev will open those files (we should add comments on top of that like it's install_requires for setup.py). Also it's just handy to have deps without quotes, commas etc in plain files. Also I've rembered why I didn't move package.json inside the package - because it's also a standard to check for any dev interested in metadata. setup.py is better for python community but the idea the same - mimimum time for side dev to have a clue about our package.

[vitorbaptista]

@roll that's why people use tox to handle linting as well. The process then becomes:

$ git clone package
$ cd package
$ virtualenv venv
$ source venv/bin/activate
$ pip install -e .[development]
$ tox

If I want to just lint, simply do tox -e lint. The package's users don't even need to know if we're using pylint or flake8 or whatever new tool we happen to use. I think this pattern will become the standard in Python, as it's already used by many high-profile packages (pylint, flake8, sqlalchemy, pip, linting by default in all except sqlalchemy).

You guys certainly know this post, but there's a difference between the purpose of requirements.txt and of setup.py. They're not two ways of doing the same thing. Reading the requirements.txt into setup.py is treating them as if they were. To do so, we have to force things to make them work (i.e. the clean() method).

[roll]

@vitorbaptista
Because our testing environment will be unified between all the packages I'm starting to feel tox.ini encapsulation could be a good pattern for us. It will not be a +10 tox.ini to maintain because we use the same tools to test and lint. Contributor could install pylama (I'd like more) by himself and use it in his venv. When we migrate from pylint to some_other_thing it will not break contributors workflow because it have been encapsulated in tox.

About the famous post. Yes, of course. But I suppose main thought there is to understand logical difference between lib vs app deps. Just a deal of technique how we name it. But I agree:

• requirements way (easier to use)
• setup.py way (more logically clean way)

[pwalsh]

@roll ( cc @vitorbaptista )

Evgeny, I pushed this basic repo but want to make sure that the final config is as you and Vitor agreed. Please go over what is there and make all changes based on last discussion on Tabulator. We've already discussed enough here, so just push directly onto master here and ping when done on #tech so all can see.

Don't worry about any small unclear issues - we'll definitely adjust over time as people bring different perspectives.

Then obviously close off these branches on tabulator, and make your final changes here so we can publish to pypi

[vitorbaptista]

I've done some small modifications to oki-py, but I don't have access to it (and the issues are disabled there). @pwalsh could you give me access?

[pwalsh]

@vitorbaptista issues disabled on purpose. Use coding-standards. Giving you ownership now.