The templating changes all look good to me. I am not an expert on distutils or Appveyor, but I agree that the proposed changes seem to have reduced total LOC substantially which may ease future maintenance.

Disabling binaries is necessary for the Python-2.6/numpy-1.7 build. Otherwise matplotlib fails with an error that it was linked against the wrong numpy version. However, this prevents caching of the wheel which makes the build take much longer.

I think this means either the test against old numpy versions, or the matplotlib doctests need to go.

EDIT: Nevermind. pip doesn't order installations by dependency, so specifying numpy before matplotlib manually fixed it.

Everything is passing now, but the caching on appveyor seems to be broken. It's working on my own CI environment, just not the PyWavelets one.

EDIT: According to http://help.appveyor.com/discussions/problems/4048-build-cache-not-restored build caches are not kept on PR builds (but PR builds will use a cache from master).

@grlee77 I've been using ./setup.py nosetests, which runs in the source directory.

This is admittedly less nice than it being contained in build/testenv, I think the issue test discovery as tests are not installed with the rest of the package.

I've sort of got out of tree tests working:

nosetests --where build/lib.linux-x86_64-2.7/ ../../pywt/tests/

This requires including the pywt.data package and files in the install - we may want to do this anyway?

It also doesn't seem to play nicely with doctest or coverage, but in-tree builds seem to be the norm for python testing as far as I can tell.

Removal of .c.src templating is quite nice. When I introduced it it was a 1:1 translation of some homegrown templating method - with almost zero test coverage I didn't dare do anything more than that. But should be pretty risk-free now.

This requires including the pywt.data package and files in the install - we may want to do this anyway?

Yes. Tests pass now for installed pywt, so this must be done already.

I don't quite see why you'd need the --where argument to nosetests? pywt is either on sys.path or it isn't I'd say.

I think this commit is going to give subtle problems. IIRC Cython and setuptools incompatibly monkeypatch Extension, and using cython as a script is much more robust.

Do you mean setuptools and Cython.cythonize not working together? I hadn't heard of any problems from this, and it's one less script to maintain. Cython.cythonize didn't work with the numpy distutils, but I think that was more from it not knowing how to deal with the Configuration object.

Also, it looks like this requires Cython as a build-time dependency for released builds, which is not the case now. Or maybe that's fixed in a later commit (I'm reviewing commits one by one)?

No, but it should be straightforward to fix: http://docs.cython.org/src/reference/compilation.html#distributing-cython-modules

And the hashing to prevent rebuilds as well?

Cython.cythonize does this internally. It also picks up if the installed version of Cython has changed and rebuilds (just noticed this now, neat).

./setup.py build
kai@laptop ~/D/c/pywt> ./setup.py build
Compiling pywt/_extensions/_pywt.pyx because it changed.
Compiling pywt/_extensions/_dwt.pyx because it changed.
Compiling pywt/_extensions/_swt.pyx because it changed.
[1/3] Cythonizing pywt/_extensions/_dwt.pyx
[2/3] Cythonizing pywt/_extensions/_pywt.pyx
[3/3] Cythonizing pywt/_extensions/_swt.pyx
running build
running build_py
<LOTS MORE COMPILING>
kai@laptop ~/D/c/pywt> ./setup.py build
running build
running build_py
copying pywt/version.py -> build/lib.linux-x86_64-3.4/pywt
running build_ext
kai@laptop ~/D/c/pywt> 

I don't quite see why you'd need the --where argument to nosetests? pywt is either on sys.path or it isn't I'd say.

If I'm in the working directory (next to setup.py), and I run ./setup.py build, nosetests fails because the compiled extensions aren't in the working directory (and build/lib-linux-whatever isn't on sys.path). I need to run ./setup.py build_ext --inplace, but it's a little irritating because it puts a bunch of .so files in my working directory. Though I guess Cython already does this with .c files so it's not a big deal. The above was an attempt to get nosetests to run in the build/lib-linux-... directory so the source tree stays as clean as possible.

The fact that tests were passing without the data files is a problem. Let me know if anybody has a solution.

On that note, it should be possible to trigger CI on tags right? So the sdist build, test and upload can be done in an automated fashion?

CI now fails without the data files: https://travis-ci.org/kwohlfahrt/pywt/builds/113160773, as does tox.

@kwohlfahrt
I am +1 on merging this

It has been a while since I looked at this, but I see you added the ability to install without Cython present.

I checked out this branch and verified that a local development installation via
pip install -e .
is working fine. I also tried an alternative way of running tests via py.test pywt and that also works.

Is there anything else still in progress here or do you think it is ready?

Been a while since I have looked as well, but I think I left it fine. It also lets us merge my other PR for code coverage (yay!) after a rebase.

@rgommers @aaren
Do either of you have any objections or other feedback related to this build/CI refactoring?

It's been a while, I'll have a look now.

With the doctesting added, a missing matplotlib will result in 4 test failures. I'm not sure if there's a good way to mark those tests as skipped though - doctest tends to be inflexible. Any preferences on how to handle that?

I propose to keep it simple for now and document that matplotlib is needed to run all tests. Then we can get this PR merged.

For reference, this is what scipy does: https://github.com/scipy/scipy/blob/master/tools/refguide_check.py. It checks that docstrings run, but doesn't actually do the full doctesting. That allows skipping tests (possibly conditionally), not having to add doctest markup crap, etc. Doctests really aren't usable on a larger project, too many limitations.

Can we use tests_require in setup.py, or is that deprecated? I can never work out what the current state of setuptools is.

Can we use tests_require in setup.py, or is that deprecated?

This is only used when you do python setup.py test, which is not a good way to run tests. I think adding the requirement to the docs as you did is fine. I suggest to remove the last commit, and then we can press the green button here.

I can never work out what the current state of setuptools is.

That makes two of us:)

OK, updated. Feel free to merge (and then pull in #181 as well).

Merging. Thanks Kai!