[MRG+1] Remove all nose imports from test scripts. by kdexd · Pull Request #422 · joblib/joblib

kdexd · 2016-11-03T05:18:06Z

Starter PR on #411

To initiate the migration form nose to py.test, this PR first of all removes all the direct usages of "nose" in all test scripts. It wraps the following utilities of nose / unittest modules in joblib/testing.py

assert_true, assert_false
assert_equal, assert_not_equal
assert_raises
with_setup, SkipTest

All the imports in scripts housed under directory joblib/tests such as:

from nose.tools import assert_equal
from nose import with_setup
import nose
# blah blah blah

nose.SkipTest()
nose.tools.assert_raises(*args, **kwargs)

have been replaced in a uniform fashion:

from joblib.testing import (assert_equal, assert_raises,
                            SkipTest, with_setup) # preserve pep8 length
# no nose imports now
# blah blah blah

SkipTest()
assert_raises(*args, **kwargs)

No unused imports from joblib.testing
Everything from joblib.testing is imported as from x import y, z

This can ease up the migration and is the first PR iniiating the process.

- These include assert_true, assert_false, assert_equal and assert_not_equal.

- Four more files in joblib/tests directory

kdexd · 2016-11-03T05:52:47Z

Only one job of travis fails, I don't know why. I can't reproduce the error 😕

GaelVaroquaux · 2016-11-03T06:02:11Z

joblib/testing.py

+assert_false = _dummy.assertFalse
+assert_equal = _dummy.assertEqual
+assert_not_equal = _dummy.assertNotEqual
+with_setup = nose.tools.with_setup


That sill needs nose, no?

Current framework is still dependent on nose, still its usage is only limited to joblib.testing module. Once this is done, we can easily move to pytest fixtures. I am not sure whether unittest module has something similar to with_setup.

aabadie · 2016-11-03T08:19:33Z

@karandesai-96, thanks a lot for providing this PR. This is definitely something useful to prepare the switch to pytest.

The travis error is indeed not related to your changes. There's an issue already opened about that #413. I restarted the build.

aabadie

Except a few alignment issues and cosmit changes/nitpicks, the update in the test modules is ok.
I added comments in the testing module because there's something weird with the assert_raises import.

aabadie · 2016-11-03T08:22:42Z

joblib/test/test_memory.py

        current_accumulator = len(accumulator)
        gg(1)
-        yield nose.tools.assert_equal, len(accumulator), \
+        yield assert_equal, len(accumulator), \


Splitting this line is now not required (see line 144 above)

aabadie · 2016-11-03T08:24:29Z

joblib/test/test_func_inspect.py


 def test_filter_args():
-    yield nose.tools.assert_equal, filter_args(f, [], (1, )),\
+    yield assert_equal, filter_args(f, [], (1, )),\


Several of the lines below don't need to be splitted.

aabadie · 2016-11-03T08:26:29Z

joblib/test/test_numpy_pickle.py

-            nose.tools.assert_equal(warn.category, UserWarning)
-            nose.tools.assert_equal(warn.message.args[0],
+            assert_equal(warn.category, UserWarning)
+            assert_equal(warn.message.args[0],


other parameters alignment not updated

aabadie · 2016-11-03T08:26:42Z

joblib/test/test_numpy_pickle.py

            numpy_pickle.dump(a, filename, cache_size=cache_size)
            expected_nb_warnings = 1 if cache_size is not None else 0
-            nose.tools.assert_equal(len(caught_warnings),
+            assert_equal(len(caught_warnings),


single line

aabadie · 2016-11-03T08:26:51Z

joblib/test/test_numpy_pickle.py

-                nose.tools.assert_equal(warn.category, DeprecationWarning)
-                nose.tools.assert_equal(warn.message.args[0],
+                assert_equal(warn.category, DeprecationWarning)
+                assert_equal(warn.message.args[0],


parameters alignment

aabadie · 2016-11-03T08:29:27Z

joblib/test/test_parallel.py

-            10,
-            len(Parallel(n_jobs=2, backend=backend, timeout=10)
-                (delayed(sleep)(0.001) for x in range(10))))
+        assert_equal(10, len(Parallel(n_jobs=2, backend=backend, timeout=10)


for readability, I would split the lines here: one for the expected value, one (well 2) for the tested expression.

aabadie · 2016-11-03T08:32:00Z

joblib/testing.py

                                     expected_exception(expected_regexp))


+try:


Is this required ? Joblib is already tested against python 2.6 and it doesn't need this.

Maybe it should be a try on an import of assert_raises from unittest and if it fails (python 2.6), import assert_raises from nose.tools ?

just had a look at scikit-learn (who did the same recently) and there's nothing of this kind, just

from nose.tools import assert_raises

aabadie · 2016-11-03T08:33:49Z

joblib/test/test_parallel.py

    # Check that timeout properly fails when function is too slow
    for backend in ['multiprocessing', 'threading']:
-        nose.tools.assert_raises(
+        assert_raises(


For readability, I would move TimeuotError on the same line and update the other lines alignment.

kdexd · 2016-11-03T09:08:52Z

I'll address your changes but I was wondering this while making the changes:
If I have written like this:

callable_obj(long_arg1,
             long_arg2,
             **long_kwargs)

and I want to change the name "callable_obj" - it changes its length and 3 lines have to be changed manually. ( I faced this while doing nose.tools.assert_true => assert_true )

While here only one line changes:

callable_obj(
    long_arg1,
    long_arg2,
    **long_kwargs
)

Too much to change across the codebase so I won't break the uniformity in this PR but this can be thought upon.

aabadie · 2016-11-03T09:37:38Z

To me number of lines changed is less important than the whole related code statement readability and style consistency.
In your particular case:

nose.tools.assert_equal(long_param1,
                        long_param2)

should become:

assert_equal(long_param1,
             long_param2)

GaelVaroquaux · 2016-11-03T09:53:46Z

assert_true(param1, param2)

IMHO, PEP8 tells us that this should be written on one line, as it is well below 79 characters. The line returns are added for lines that are long.

aabadie · 2016-11-03T09:54:55Z

@GaelVaroquaux, indeed but this was just for the example.

aabadie · 2016-11-03T09:56:07Z

comment updated

- joblib is compatible with python 2.6+ and 3.3+ it need not handle exceptional cases if version is less than 2.6

kdexd · 2016-11-03T10:24:34Z

@aabadie I have addressed your review comments, and all CI services are happy now 💚 The spread out usage of nose across all the test scripts, as a dependency has been significantly reduced down to just one module and in subsequent PRs we can reduce it out from joblib.testing and fit in pytest while making minimal changes across the source code. Finally in the third phase, we can get creative with conftest, fixtures and parametrization of pytest and make the test suite optimized and more robust ! 😄

If there is anything left to add / modify in this PR, I'm here, thanks.

aabadie · 2016-11-03T12:41:51Z

@karandesai-96, I allow myself to push a small update on top of your recent changes (this is a new feature with GitHub). Hope this is fine.

By the way, thanks for updating the PR that fast !

LGTM, +1 for merge.

kdexd · 2016-11-03T12:49:21Z

@aabadie Sure, fine with me! Github did a nice thing adding this feature 😄

lesteve · 2016-11-04T11:05:46Z

Looks good, small nitpick still some nose imports in doc fixtures, excerpt from git grep -P 'from nose|import nose':

doc/parallel_numpy_fixture.py:from nose import SkipTest
doc/persistence_fixture.py:from nose import SkipTest

I guess you can replace that by from joblib.testing import SkipTest.

lesteve · 2016-11-04T12:01:23Z

and I want to change the name "callable_obj" - it changes its length and 3 lines have to be changed manually. ( I faced this while doing nose.tools.assert_true => assert_true )

My 2cents: when doing project-wide search and replace that breaks PEP8 similar to yours, I found running autopep8 may help. Obviously if your project has a lot of other PEP8 violations it is not going to help much because you'll need to undo a lot of things after the autopep8.

aabadie · 2016-11-07T08:34:15Z

We are good now, merging. Thanks @karandesai-96 and @lesteve !

GaelVaroquaux · 2016-11-07T09:20:59Z

Great! Thank you everyone.

* commit '0.10.2-55-g660fe5d': (52 commits) MAINT simplify way to skip doctests Do not print frames with IPython internals in format_stack BF(TST): compare for version in pickled filename ignoring full path, and looking for _ as well (joblib#445) [MRG] Remove nose dependency completely. (joblib#441) MAINT add Python 3.5 in setup.py Remove some Python 2.6 specific code [MRG] Make test execution on CI possible through py.test command. (joblib#433) Remove more python 2.6 related code (joblib#440) [MRG] Use commit range for diff in flake8_diff.sh (joblib#439) MAINT remove support for python 2.6 (joblib#437) Improve doc about caching methods (joblib#432) Get the tests passing with py.test MAINT update flake8_diff.sh COSMIT move stdlib imports where they belong [MRG] Replace assert_* methods with "assert" keyword statements. (joblib#430) Reraise when the exception is not a TransportableException (joblib#429) Make sure LICENSE.txt is included in the wheel Add LICENSE.txt Update doc/paralle.rst [MRG+1] Remove all nose imports from test scripts. (joblib#422) ...

* releases: (52 commits) MAINT simplify way to skip doctests Do not print frames with IPython internals in format_stack BF(TST): compare for version in pickled filename ignoring full path, and looking for _ as well (joblib#445) [MRG] Remove nose dependency completely. (joblib#441) MAINT add Python 3.5 in setup.py Remove some Python 2.6 specific code [MRG] Make test execution on CI possible through py.test command. (joblib#433) Remove more python 2.6 related code (joblib#440) [MRG] Use commit range for diff in flake8_diff.sh (joblib#439) MAINT remove support for python 2.6 (joblib#437) Improve doc about caching methods (joblib#432) Get the tests passing with py.test MAINT update flake8_diff.sh COSMIT move stdlib imports where they belong [MRG] Replace assert_* methods with "assert" keyword statements. (joblib#430) Reraise when the exception is not a TransportableException (joblib#429) Make sure LICENSE.txt is included in the wheel Add LICENSE.txt Update doc/paralle.rst [MRG+1] Remove all nose imports from test scripts. (joblib#422) ...

Karan Desai added 6 commits November 3, 2016 10:15

Wrap unittest assert helpers in joblib.testing

3fecc6d

- These include assert_true, assert_false, assert_equal and assert_not_equal.

Replace some nose.tools imports with joblib.testing imports

f6e1c80

More nose.tools imports with joblib.testing imports

dab49e1

- Four more files in joblib/tests directory

Replace all nose assert true, false, equal helpers in tests

a0dcd06

Replace nose.tools.assert_raises with joblib.testing.assert_raises

bf9cc09

Wrap SkipTest, with_setup of nose in joblib.testing

7dffa56

kdexd force-pushed the nose-imports-removal branch from 68aafd8 to 7dffa56 Compare November 3, 2016 05:30

GaelVaroquaux reviewed Nov 3, 2016

View reviewed changes

aabadie mentioned this pull request Nov 3, 2016

Nested parallel warnings test failing from time to time in python 3 #413

Closed

aabadie suggested changes Nov 3, 2016

View reviewed changes

kdexd force-pushed the nose-imports-removal branch from 6c9fdfa to c2c3498 Compare November 3, 2016 10:10

Rectify pep8 warns, remove try except from joblib.testing

c73bad0

- joblib is compatible with python 2.6+ and 3.3+ it need not handle exceptional cases if version is less than 2.6

kdexd force-pushed the nose-imports-removal branch 2 times, most recently from 2cba539 to c73bad0 Compare November 3, 2016 10:14

better parameters alignment

07dd20c

aabadie force-pushed the nose-imports-removal branch from eeb5afd to 07dd20c Compare November 3, 2016 12:37

aabadie approved these changes Nov 3, 2016

View reviewed changes

aabadie changed the title ~~[MRG] Remove all nose imports from test scripts.~~ [MRG+1] Remove all nose imports from test scripts. Nov 3, 2016

Remove nose imports from docs.

980a58e

aabadie merged commit 8c67f23 into joblib:master Nov 7, 2016

kdexd deleted the nose-imports-removal branch November 24, 2016 06:36

kdexd mentioned this pull request Nov 26, 2016

[MRG] Replace assert_* methods with "assert" keyword statements. #430

Merged

		expected_exception(expected_regexp))


		try:

Conversation

kdexd commented Nov 3, 2016

Starter PR on #411

All the imports in scripts housed under directory joblib/tests such as:

Uh oh!

kdexd commented Nov 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabadie commented Nov 3, 2016

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kdexd commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabadie commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GaelVaroquaux commented Nov 3, 2016 via email

Uh oh!

aabadie commented Nov 3, 2016

Uh oh!

aabadie commented Nov 3, 2016

Uh oh!

kdexd commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aabadie commented Nov 3, 2016

Uh oh!

kdexd commented Nov 3, 2016

Uh oh!

lesteve commented Nov 4, 2016

Uh oh!

lesteve commented Nov 4, 2016

Uh oh!

aabadie commented Nov 7, 2016

Uh oh!

GaelVaroquaux commented Nov 7, 2016 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kdexd commented Nov 3, 2016 •

edited

Loading

aabadie commented Nov 3, 2016 •

edited

Loading

kdexd commented Nov 3, 2016 •

edited

Loading