Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Make weave tests behave #3182

Merged
merged 12 commits into from

6 participants

@rgommers
Owner

Most importantly:

  • make them not leave behind any files in the dir that the tests are run from
  • do not print anything by default

Furthermore a lot of random cleanups and deletions of unused or commented out code.

@rgommers
Owner

I have verified that the number of tests run and total test time is unchanged.

@coveralls

Coverage Status

Coverage remained the same when pulling 696492c on rgommers:weave-tests into 16fc0af on scipy:master.

@WarrenWeckesser
Collaborator

First, a big "thank you" for doing this!

I just tried the PR, and got three weave errors:

Running unit tests for scipy
NumPy version 1.8.0
NumPy is installed in /home/warren/local_scipy/lib/python2.7/site-packages/numpy
SciPy version 0.14.0.dev-696492c
SciPy is installed in /home/warren/local_scipy/lib/python2.7/site-packages/scipy
Python version 2.7.6 |Anaconda 1.8.0 (64-bit)| (default, Nov 11 2013, 10:47:18) [GCC 4.1.2 20080704 (Red Hat 4.1.2-54)]
nose version 1.3.0

<snip>

======================================================================
FAIL: test_add_function_ordered (test_catalog.TestCatalog)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/warren/local_scipy/lib/python2.7/site-packages/scipy/weave/tests/test_catalog.py", line 521, in test_add_function_ordered
    assert_(funcs1[:2] == [string.lower,string.upper]),repr(funcs1)
  File "/home/warren/local_scipy/lib/python2.7/site-packages/numpy/testing/utils.py", line 44, in assert_
    raise AssertionError(msg)
AssertionError

======================================================================
FAIL: test_add_function_persistent1 (test_catalog.TestCatalog)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/warren/local_scipy/lib/python2.7/site-packages/scipy/weave/tests/test_catalog.py", line 473, in test_add_function_persistent1
    assert_(i in pfuncs)
  File "/home/warren/local_scipy/lib/python2.7/site-packages/numpy/testing/utils.py", line 44, in assert_
    raise AssertionError(msg)
AssertionError

======================================================================
FAIL: test_get_existing_files2 (test_catalog.TestCatalog)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/warren/local_scipy/lib/python2.7/site-packages/scipy/weave/tests/test_catalog.py", line 401, in test_get_existing_files2
    assert_(len(files) == 1)
  File "/home/warren/local_scipy/lib/python2.7/site-packages/numpy/testing/utils.py", line 44, in assert_
    raise AssertionError(msg)
AssertionError

----------------------------------------------------------------------
Ran 11334 tests in 709.137s

FAILED (KNOWNFAIL=171, SKIP=451, failures=4)

But there were no files left behind :smile:

@argriffing
Collaborator

works for me on a ubuntu derivative

runtests.py default mode

Running unit tests for scipy.weave
NumPy version 1.7.1
SciPy version 0.14.0.dev-696492c
Python version 2.7.5+ (default, Sep 19 2013, 13:48:49) [GCC 4.8.1]
Ran 9944 tests in 95.298s
OK (KNOWNFAIL=118, SKIP=447)

runtests.py --mode=full

Ran 11334 tests in 684.561s
OK (KNOWNFAIL=171, SKIP=462)
@rgommers
Owner

@WarrenWeckesser I can't reproduce this. Neither can TravisCI or Alex. So could you investigate? I only changed the use of globals for backup_dir in these tests, in f0e4e5a.

@WarrenWeckesser
Collaborator

So far all I know is that some catalog methods are returning empty lists when they shouldn't:

  • In test_get_existing_files2, q.get_existing_files() returns an empty list.
  • In test_add_function_persistent1, q.get_cataloged_functions('code') returns an empty list.
  • In test_add_function_ordered, each of the calls t.get_functions('f'), t.get_functions('ff'), and t.get_functions('fff') returns an empty list.

I'll try to take another look this weekend.

FWIW, I'm using Anaconda's python in Ubuntu 12.04 (64 bit).

@rgommers
Owner

I think it's a bug in add_function that somehow is triggered for you. If you add back import os and import string within the tests, are the failures gone?

@WarrenWeckesser
Collaborator

I haven't debugged the errors, but I have some more information. On my system, import dbhash (in catalog.py before this PR) fails. If I put import dbhash back to force the use of _dumb_shelve.py for the shelve module in catalog.py, the tests pass.

@rgommers
Owner

@WarrenWeckesser do you want me to just put import dbhash back (or you send me a PR to that effect)? Or do you want to debug this?

@WarrenWeckesser
Collaborator

Apparently the Anaconda python distribution from Continuum is missing the library _bsddb.so (at least in Linux; see https://groups.google.com/a/continuum.io/forum/#!topic/anaconda/mCQL6fVx55A). That's why import dbhash fails. When this import fails, the code in catalog.py uses the module _dumb_shelve.py instead of python's standard shelve module. When this module is used, the tests pass.

With this PR, import dbhash has been removed, and import shelve succeeds. I don't know why using the standard shelve module triggers the three test failures. It might be related to the missing _bsddb.so, or perhaps there is something else wrong with shelve in Anaconda.

TL;DR: This might be a problem with Anaconda's build of the shelve module, and not a scipy problem. (Pinging @ilanschnell, @teoliphant, @asmeurer in case any of them can take a look.)

@rgommers
Owner

OK now that rings a bell. bsddb is broken in various ways on various Pythons/platforms, not present at all in Python 3.x and is used in shelve under the hood. So I will revert removing what looked like an unused import and comment it properly. Then everything should be good to go; no need to investigate.

@rgommers rgommers referenced this pull request from a commit in rgommers/scipy
@rgommers rgommers BUG: put back dbhash import in weave/catalog.py, while unused it had …
…a purpose

Issue without this import observed by Warren with Anaconda on gh-3182.
See comments in commit for more details.
8f68821
@coveralls

Coverage Status

Coverage remained the same when pulling 8f68821 on rgommers:weave-tests into 9c7017e on scipy:master.

@rgommers
Owner

@WarrenWeckesser done, see last commit. Can you test again?

scipy/weave/blitz_tools.py
@@ -114,7 +116,6 @@ def test_function():
expr = "ex[:,1:,1:] = k + ca_x[:,1:,1:] * ex[:,1:,1:]" \
"+ cb_y_x[:,1:,1:] * (hz[:,1:,1:] - hz[:,:-1,1:])"\
"- cb_z_x[:,1:,1:] * (hy[:,1:,1:] - hy[:,1:,:-1])"
- #ast = parser.suite('a = (b + c) * sin(d)')
ast = parser.suite(expr)
@WarrenWeckesser Collaborator

This function (test_function) looks like a test. It assigns values to a bunch of local variables that are never used. Nothing calls it. It should be removed or moved to a file in the tests directory. But that doesn't have to happen in this PR, if you want to restrict the changes in this PR to the more obvious bits of code clean up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scipy/weave/tests/test_blitz_tools.py
((13 lines not shown))
- # this really should give more info...
- try:
- # this isn't very stringent. Need to tighten this up and
- # learn where failures are occurring.
- assert_(allclose(abs(actual.ravel()),abs(desired.ravel()),1e-4,1e-6))
- except:
- diff = actual-desired
- print(diff[:4,:4])
- print(diff[:4,-4:])
- print(diff[-4:,:4])
- print(diff[-4:,-4:])
- print(sum(abs(diff.ravel()),axis=0))
- raise AssertionError
- return standard,compiled
+ # TODO: this isn't very stringent. Need to tighten this up and
+ # learn where failures are occuring.
@WarrenWeckesser Collaborator

*occurring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scipy/weave/tests/test_ext_tools.py
((15 lines not shown))
c = 5
arg_list = ['a','b','c']
actual = ext_tools.assign_variable_types(arg_list,locals())
- # desired = {'a':(Float32,1),'b':(Float32,1),'i':(Int32,0)}
+ # desired = {'a':(float32,1),'b':(float32,1),'i':(Int32,0)}
@WarrenWeckesser Collaborator

Why not delete this line? The values assigned to ad, bd and cd make clear what the desired values are, so I don't think this is needed for documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@WarrenWeckesser
Collaborator

The tests now pass for me.

I made a few small suggestions as line notes. Other than those, I think the PR is ready to go.

@asmeurer

Just now seeing this. Is there still something you want me to look at in the Anaconda package? I looked at our scipy recipe and there's nothing too out of the ordinary going on. We do have a small patch to skip a failing test, and we do some stuff when we link against MKL, but other than that it's just setup.py install.

@rgommers
Owner

@asmeurer it's not your scipy but your Python install - that's lacking bsddb (stdlib module that sometimes doesn't get built, I don't remember exactly why). If you did that on purpose, no need to do anything. But you may want to fix that for Python 2.7 is you care about BSDDB support.

@asmeurer

Yep, I just checked, and it's not there on Mac OS X or Linux (but it is on Windows). We do have some code to support it, but only on armv6l. I'm not sure what the store is there. @ilanschnell would have to comment.

But I also noticed that bsddb is deprecated, and was removed from Python 3 (http://docs.python.org/2/library/bsddb.html#module-bsddb).

rgommers and others added some commits
@rgommers rgommers TST: weave tests: clean up ast/blitz/catalog tests.
Basic changes - no docstrings, imports, run_module_suite
dbb5928
@rgommers rgommers TST: weave tests: clean up build_tools/c_spec/ext_tools tests. a37fbce
@rgommers rgommers TST: weave tests: clean up tests for numpy_scalar/size_check/inline_t…
…ools
69b6ba7
@rgommers rgommers TST: weave tests: clean up weave_test_utils and slice_handler tests.
Also remove duplicate remove_whitespace function in a number of files.
a9d7026
@rgommers rgommers TST: weave tests: clean up scxx_* tests and make scxx_timings run again. 6901951
@rgommers rgommers TST: weave tests: silence a large amount of test noise. Add BlitzWarn…
…ing.
2d86fa0
@rgommers rgommers TST: weave tests: remove more test noise, fix use of global temp dirs. 0943476
@rgommers rgommers TST: weave tests: don't leave .so/cpp files in dir from which tests a…
…re run.
ce92896
@rgommers rgommers TST: weave tests: add context manager to create/clean tempdirs for bl…
…itz().

Closes gh-2092.
0370917
@rgommers rgommers BUG: put back dbhash import in weave/catalog.py, while unused it had …
…a purpose

Issue without this import observed by Warren with Anaconda on gh-3182.
See comments in commit for more details.
937c197
@juliantaylor juliantaylor TST: avoid use of unsafe mktemp and cleanup test_dir in test_c_spec 3f4419c
@WarrenWeckesser WarrenWeckesser MAINT: weave: A few tweaks:
* Remove an unused function ('test_function()') from blitz_tools.py
* Fix a typo in a comment in test_blitz_tools.py
* Remove a commented-out line of code from test_ext_tools.py
a78fbe5
@coveralls

Coverage Status

Coverage remained the same when pulling a78fbe5 on rgommers:weave-tests into df81077 on scipy:master.

@coveralls

Coverage Status

Coverage remained the same when pulling a78fbe5 on rgommers:weave-tests into df81077 on scipy:master.

@rgommers
Owner

OK this all works now and contains added commits from Warren and Julian that needed to go in, so merging.

@rgommers rgommers merged commit 2df405a into scipy:master
@rgommers rgommers deleted the rgommers:weave-tests branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 16, 2014
  1. @rgommers

    TST: weave tests: clean up ast/blitz/catalog tests.

    rgommers authored
    Basic changes - no docstrings, imports, run_module_suite
  2. @rgommers
  3. @rgommers
  4. @rgommers

    TST: weave tests: clean up weave_test_utils and slice_handler tests.

    rgommers authored
    Also remove duplicate remove_whitespace function in a number of files.
  5. @rgommers
  6. @rgommers
  7. @rgommers
  8. @rgommers
  9. @rgommers
  10. @rgommers

    BUG: put back dbhash import in weave/catalog.py, while unused it had …

    rgommers authored
    …a purpose
    
    Issue without this import observed by Warren with Anaconda on gh-3182.
    See comments in commit for more details.
  11. @juliantaylor @rgommers
  12. @WarrenWeckesser @rgommers

    MAINT: weave: A few tweaks:

    WarrenWeckesser authored rgommers committed
    * Remove an unused function ('test_function()') from blitz_tools.py
    * Fix a typo in a comment in test_blitz_tools.py
    * Remove a commented-out line of code from test_ext_tools.py
Something went wrong with that request. Please try again.