[MRG] Remove nose dependency completely from project. by kdexd · Pull Request #441 · joblib/joblib

kdexd · 2016-12-07T08:01:56Z

Checkpoint PR on #411 ( Succeeds PR #433 )

With this PR joblib finally drops nose as its own dependency.

kd@dragonstone:~/Documents/joblib$ git grep nose
CHANGES.rst:    py.test is used to run the tests instead of nosetests.
CHANGES.rst:    ``python setup.py test`` or ``python setup.py nosetests`` do not work

Replaced with_setup mechanism of nosetests with equivalent skipif markers of pytest.
I removed the examples of nose in some block comments too. (It is a small nitpick though let me know if I should rollback / modify it.)

NOTE: I have changed the mechanism of conditional setup of tests (based on availability of multiprocessing / shared memory) in test_hashing.py - that is, through a fixture mechanism. It is the "pytest design technique" and although one needs to specify same argument (as I have specified tmpdir_path) to many tests, looking at a specific method signature one can easily know that this test "requires" something prior its execution...

Similar thing can be exercised in other modules, but for this PR only the bare minimum changes needed to drop nose are included.

Further Roadmap:

Following this PR would be a few more, where each test module will be dealt with - yield based methods will be parametrized and overall revamping would be performed one by one to migrate from nose and adopt pytest's flavor.

…ers.

kdexd · 2016-12-07T08:06:13Z

@lesteve @GaelVaroquaux Please have a round of review, I'm ready to make amendments if any...

lesteve

Changes look good apart from the minor comments. The error on Travis is weird and consistent, it looks like the process was killed with a kill -9 ...

lesteve · 2016-12-07T08:44:05Z

doc/sphinxext/numpydoc/numpydoc.py

 def setup(app, get_doc_object_=get_doc_object):
    if not hasattr(app, 'add_config_value'):
-        return  # probably called by nose, better bail out
+        return


Do not change anything in doc/sphintext/numpydoc, this is a straight copy from numpydoc.

Does it cause any problem?

Nope it doesn't. I'll restore it back.

lesteve · 2016-12-07T08:46:31Z

joblib/test/test_hashing.py


-    def __init__(self):
-        mem = Memory(cachedir=test_memory_env['dir'])
+    def __init__(self, tmpdir_path):


Just curious, do you need tmpdir_path in the __init__ arguments, i.e. could you not just have mem = Memory(cachedir=tmpdir_path)?

Nope, tmpdir_path used that way would take in the global reference - the function itself. Pytest actually executes these functions marked with @fixture before test execution and automatically provides them as function arguments whenever specified. So this tmpdir_path will be passed in constructor from inside a test method.

Fair enough, although a little bit magical. We'll get used to it after some time.

lesteve · 2016-12-07T08:53:20Z

The error on Travis is weird and consistent, it looks like the process was killed with a kill -9

OK it passes this time. This flakiness with the tests that happens on Travis/AppVeyor is really weird, it is probably just a coincidence that this happens while moving towards py.test.

kdexd · 2016-12-07T09:45:20Z

@lesteve Even I haven't found any specific line to blame, I'm just optimistic about the fact that once I parametrize yield based tests, everything would become consistent.

kdexd · 2016-12-07T09:46:02Z

Although I should mention that such flakiness has never happened locally on my machine... strange.

lesteve · 2016-12-07T09:59:16Z

doc/sphinxext/numpydoc/numpydoc.py

 def setup(app, get_doc_object_=get_doc_object):
    if not hasattr(app, 'add_config_value'):
-        return  # probably called by nose, better bail out
+        return # probably called by nose, better bail out


Look at your diff ;-). I'll edit it inline quicker this way

My bad 😅

lesteve · 2016-12-07T10:29:25Z

joblib/test/test_hashing.py

-    def __init__(self):
-        mem = Memory(cachedir=test_memory_env['dir'])
+    def __init__(self, cachedir):
+        mem = Memory(cachedir=cachedir)


I changed the parameter name from tmpdir_path to cachedir. Less chance for confusion this way.

Okay that's great..

lesteve · 2016-12-07T12:27:09Z

LGTM, merging thanks a lot!

kdexd · 2016-12-07T14:32:53Z

Well... that was quick, I wanted this to happen! Now the subsequent steps will be painless, cheers ! 🎉

* 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) ...

kdexd changed the title ~~Remove nose dependency completely from project.~~ [MRG] Remove nose dependency completely from project. Dec 7, 2016

Karan Desai added 3 commits December 7, 2016 13:34

Convert check_multiprocessing and setup_if_has_dev_shm to skipif mark…

26a5d1c

…ers.

Remove usage of with_setup decorator in test_hashing.py

da61ce0

Remove nose dependency from the project completely.

78b6e4d

kdexd force-pushed the remove-nose-dependency branch from 6002dc2 to 78b6e4d Compare December 7, 2016 08:05

lesteve added 2 commits December 7, 2016 09:42

Fix typo

4bc2c40

Fix another typo

704a7c8

lesteve reviewed Dec 7, 2016

View reviewed changes

Restore a comment in numpydoc.

28f2fa8

lesteve reviewed Dec 7, 2016

View reviewed changes

lesteve added 2 commits December 7, 2016 10:59

Restored numpydoc file in its initial state

cab42e0

Better naming for class constructor parameter

7b0a4c0

lesteve reviewed Dec 7, 2016

View reviewed changes

lesteve merged commit 3e977b1 into joblib:master Dec 7, 2016

kdexd deleted the remove-nose-dependency branch December 7, 2016 14:31

kdexd mentioned this pull request Dec 7, 2016

[MRG] Refactor test_disk.py as per pytest design. #446

Merged

Conversation

kdexd commented Dec 7, 2016 • edited by lesteve Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checkpoint PR on #411 ( Succeeds PR #433 )

Further Roadmap:

Uh oh!

kdexd commented Dec 7, 2016

Uh oh!

lesteve 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!

lesteve commented Dec 7, 2016

Uh oh!

kdexd commented Dec 7, 2016

Uh oh!

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

Uh oh!

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!

lesteve commented Dec 7, 2016

Uh oh!

kdexd commented Dec 7, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kdexd commented Dec 7, 2016 •

edited by lesteve

Loading

kdexd commented Dec 7, 2016 •

edited

Loading