Merged
Conversation
Transition to flat setuptools package. "setup.py test" works, "runtests.py" does not. Note "setup.py test" runs tests in source tree.
Try a minimal AppVeyor setup, and see what happens.
Tests now work fine with setup.py, so use that for travis. This currently doesn't use coverage or doctests, so those are TODO.
The install step is split into two stages, the first creates wheels for all requirements, and the second installs from those wheels. The wheels and downloaded sources are cached with AppVeyor to reduce build times.
Debian stretch has a (very) old pip, and what worked on my machine is no longer valid. Use new caching mechanims instead.
Cache pip install on travis-ci as well.
According to the article linked in the header, python3.3 and 3.4 need a special flag to make sure the build environment is set up correctly. This works together with util/appveyor/build.cmd.
This makes the build process a little more consistent (same packages updated and installed on both platforms). Also removes the dependency of the appveyor cache on appveyor.yml, which was preventing me from properly testing it. Caching on Travis doesn't seem to be working, but we'll see if this helps.
Numpy build was failing, this might be the reason.
This commit enables doctests and coverage reports. Also fixes the absurdly long logs (nosetests is more compact than test). Note matplotlib is required for data/_readers.py, use a non-interactive backend for travis.
Update tox.ini to install matplotlib for new doctests, and don't change out of source directory. Coverage isn't quite right, it includes lots of other modules not part of pywt (dateutil, etc)
No longer used.
Apparently setup.py nosetests already specifies --where, so using that gives a warning that it'll be deprecated in future. Use ignore-files instead. Install coverage on appveyor.
Shape returns e.g. (512L, 512L) on python2 appveyor. Check for equality instead.
Other packages (notably matplotlib) link against numpy, so they must be installed in the correct order.
Don't specify a numpy version so pip doesn't insist on upgrading it with pywt. Cython is treated as a compiler and not required after distribution, so don't require it.
This allows the import of Cython to fail, and attempts to use pre-built C sources if it does.
Tests before didn't pick up problems with the distribution (e.g. missing data files) that were present in the source tree. This should be fixed in CI now, developers should probably use: ./setup.py build nosetests build/lib.xyz/ --tests pywt/tests
nosetests wasn't added to $PATH on windows, so specify full location.
Old link was broken/the wrong link in the first place.
Member
Author
On Appveyor Python-3.5 builds only, both 64- and 32-bit. Going to see if it builds on the |
Member
Author
|
OK, looks good on top of #161. That should be good to merge now too. |
Member
|
Haven't seen this issue before, but then making a copy of a temporary memoryview isn't common. Fix looks fine to me. |
Doing a copy of a temporary memoryview resulted in a leak. Storing the input shape into an explicit memoryview object and then copying that does not cause a leak.
Member
|
Merged, thanks @kwohlfahrt |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #180. Something about copying a temporary memoryview (or is it a memoryviewslice?) causes Cython to not free something. Not entirely sure what's going on but this fixes it and looks reasonably clean to me. If somebody with more Cython experience can cast an eye over it that'd be great, otherwise I'm merging it tomorrow morning.
Also if anybody knows of a good way to test for memory leaks, that would be nice. I think py.test supports this, but haven't looked in a while.
Edit 2: Is this a bug that should be reported to upstream Cython?