test custom with scheduler fixture by kain88-de · Pull Request #61 · MDAnalysis/pmda

kain88-de · 2018-09-19T17:45:38Z

This allows to add new schedulers in the future and have pytest
automatically create the tests for us. The should also catch more
bugs.

Fixes #57

Changes made in this Pull Request:

make scheduler fixture more common to use.
add testing helper module

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

orbeckst · 2018-09-20T01:13:39Z

Not sure why the tests fail with

___________ ERROR at setup of test_AnalysisFromFunction[distributed] ___________
file /home/travis/build/MDAnalysis/pmda/pmda/test/test_custom.py, line 26
  def test_AnalysisFromFunction(scheduler):
file /home/travis/build/MDAnalysis/pmda/pmda/testing.py, line 26
  @pytest.fixture(scope='session', params=('distributed', 'multiprocessing'))
  def scheduler(request, client):
E       fixture 'client' not found
>       available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, cov, doctest_namespace, monkeypatch, no_cover, pytestconfig, record_property, record_xml_attribute, record_xml_property, recwarn, scheduler, tmpdir, tmpdir_factory
>       use 'pytest --fixtures [testpath]' for help on them.

Can we not stack parametrized fixtures?

richardjgowers · 2018-09-20T01:16:34Z

Stacking should work fine

…

On Wed, Sep 19, 2018 at 8:13 PM, Oliver Beckstein ***@***.***> wrote: Not sure why the tests fail with ___________ ERROR at setup of test_AnalysisFromFunction[distributed] ___________ file /home/travis/build/MDAnalysis/pmda/pmda/test/test_custom.py, line 26 def test_AnalysisFromFunction(scheduler): file /home/travis/build/MDAnalysis/pmda/pmda/testing.py, line 26 @pytest.fixture(scope='session', params=('distributed', 'multiprocessing')) def scheduler(request, client): E fixture 'client' not found > available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, cov, doctest_namespace, monkeypatch, no_cover, pytestconfig, record_property, record_xml_attribute, record_xml_property, recwarn, scheduler, tmpdir, tmpdir_factory > use 'pytest --fixtures [testpath]' for help on them. Can we not stack parametrized fixtures? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#61 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AI0jB4r2h5gfovJXyTHiX8E6-ksa6I3mks5ucuvEgaJpZM4Wwpsp> .

kain88-de · 2018-09-20T08:08:39Z

Maybe both fixtures need to be loaded. I'm trying the conftest.py configuration file now. Maybe that works better On Thu, Sep 20, 2018 at 3:16 AM Richard Gowers <notifications@github.com> wrote:

…

Stacking should work fine On Wed, Sep 19, 2018 at 8:13 PM, Oliver Beckstein < ***@***.***> wrote: > Not sure why the tests fail with > > ___________ ERROR at setup of test_AnalysisFromFunction[distributed] ___________ > file /home/travis/build/MDAnalysis/pmda/pmda/test/test_custom.py, line 26 > def test_AnalysisFromFunction(scheduler): > file /home/travis/build/MDAnalysis/pmda/pmda/testing.py, line 26 > @pytest.fixture(scope='session', params=('distributed', 'multiprocessing')) > def scheduler(request, client): > E fixture 'client' not found > > available fixtures: cache, capfd, capfdbinary, caplog, capsys, capsysbinary, cov, doctest_namespace, monkeypatch, no_cover, pytestconfig, record_property, record_xml_attribute, record_xml_property, recwarn, scheduler, tmpdir, tmpdir_factory > > use 'pytest --fixtures [testpath]' for help on them. > > Can we not stack parametrized fixtures? > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#61 (comment)>, or mute > the thread > < https://github.com/notifications/unsubscribe-auth/AI0jB4r2h5gfovJXyTHiX8E6-ksa6I3mks5ucuvEgaJpZM4Wwpsp > > . > — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#61 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEGnVktHmJ42kuc8seU3yGjZlxF57_SWks5ucuxzgaJpZM4Wwpsp> .

kain88-de · 2018-09-20T08:54:06Z

OK the fixtures work now. The tests fail because of actual bugs.

kain88-de · 2018-09-20T11:32:45Z

pmda/rdf.py

    def _reduce(res, result_single_frame):
        """ 'add' action for an accumulator"""
-        if res == []:
+        if isinstance(res, list) and len(res) == 0:


@VOD555 Can you have a look. Your old version triggered a deprecation warning when res is a numpy array.

codecov · 2018-09-20T11:37:33Z

Codecov Report

Merging #61 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master      #61   +/-   ##
=======================================
  Coverage   99.27%   99.27%           
=======================================
  Files           7        7           
  Lines         274      274           
  Branches       26       26           
=======================================
  Hits          272      272           
  Misses          1        1           
  Partials        1        1

Impacted Files	Coverage Δ
pmda/rdf.py	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d56fd7e...58b1281. Read the comment docs.

This allows to add new schedulers in the future and have pytest automatically create the tests for us. The should also catch more bugs.

The comparison is deprecated is `res` is a numpy array. This is more robust.

kain88-de · 2018-09-20T14:04:12Z

Ready for review

orbeckst

lgtm

Can we now use this for other tests as well, not only for the custom analysis class?

VOD555 · 2018-09-20T20:32:24Z

@kain88-de I run the tests for rdf.py, rms.py and contacts.py, and they all returned the deprecation warning:

 /home/shujie/anaconda3/envs/py36/lib/python3.6/site-packages/dask/base.py:835: UserWarning: 
The get= keyword has been deprecated. Please use the scheduler= keyword instead with the 
name of the desired scheduler like 'threads' or 'processes'
    warnings.warn("The get= keyword has been deprecated. "

I think the cause is line775 in parallel.py

        scheduler_kwargs = {'get': scheduler.get}

where we use a get= keyword, whcih is now deprecated in the new version of dask.

kain88-de · 2018-09-21T05:46:44Z

See #66 for a fix of the warning

…

On Thu 20. Sep 2018 at 22:32, VOD555 ***@***.***> wrote: @kain88-de <https://github.com/kain88-de> I run the tests for rdf.py, rms.py and contacts.py, and they all returned the deprecation warning: /home/shujie/anaconda3/envs/py36/lib/python3.6/site-packages/dask/base.py:835: UserWarning: The get= keyword has been deprecated. Please use the scheduler= keyword instead with the name of the desired scheduler like 'threads' or 'processes' warnings.warn("The get= keyword has been deprecated. " — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#61 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEGnVnuxRLeazuSBJICQdLXZYhE04ERAks5uc_tYgaJpZM4Wwpsp> .

kain88-de force-pushed the test-with-scheduler-fixture branch from c0c8127 to 72375a1 Compare September 20, 2018 08:07

kain88-de force-pushed the test-with-scheduler-fixture branch from 927aeee to 1509fd0 Compare September 20, 2018 09:53

kain88-de commented Sep 20, 2018

View reviewed changes

kain88-de added 2 commits September 20, 2018 14:51

test custom with scheduler fixture

c248ca3

This allows to add new schedulers in the future and have pytest automatically create the tests for us. The should also catch more bugs.

fix up reduce in rdf

58b1281

The comparison is deprecated is `res` is a numpy array. This is more robust.

kain88-de force-pushed the test-with-scheduler-fixture branch 2 times, most recently from 9e83b15 to 58b1281 Compare September 20, 2018 13:13

orbeckst approved these changes Sep 20, 2018

View reviewed changes

kain88-de merged commit 3d78fc7 into master Sep 21, 2018

kain88-de deleted the test-with-scheduler-fixture branch September 21, 2018 06:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test custom with scheduler fixture#61

test custom with scheduler fixture#61
kain88-de merged 2 commits intomasterfrom
test-with-scheduler-fixture

kain88-de commented Sep 19, 2018 •

edited

Loading

Uh oh!

orbeckst commented Sep 20, 2018

Uh oh!

richardjgowers commented Sep 20, 2018 via email

Uh oh!

kain88-de commented Sep 20, 2018 via email

Uh oh!

kain88-de commented Sep 20, 2018

Uh oh!

kain88-de Sep 20, 2018

Uh oh!

codecov bot commented Sep 20, 2018 •

edited

Loading

Uh oh!

kain88-de commented Sep 20, 2018

Uh oh!

orbeckst left a comment

Uh oh!

VOD555 commented Sep 20, 2018 •

edited

Loading

Uh oh!

kain88-de commented Sep 21, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kain88-de commented Sep 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Uh oh!

orbeckst commented Sep 20, 2018

Uh oh!

richardjgowers commented Sep 20, 2018 via email

Uh oh!

kain88-de commented Sep 20, 2018 via email

Uh oh!

kain88-de commented Sep 20, 2018

Uh oh!

kain88-de Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kain88-de commented Sep 20, 2018

Uh oh!

orbeckst left a comment

Choose a reason for hiding this comment

Uh oh!

VOD555 commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kain88-de commented Sep 21, 2018 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kain88-de commented Sep 19, 2018 •

edited

Loading

codecov bot commented Sep 20, 2018 •

edited

Loading

VOD555 commented Sep 20, 2018 •

edited

Loading