MDAnalysis / pmda

Parallel algorithms for MDAnalysis
https://www.mdanalysis.org/pmda/
Other
31 stars 22 forks source link

test custom with scheduler fixture #61

Closed kain88-de closed 6 years ago

kain88-de commented 6 years ago

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:

PR Checklist

orbeckst commented 6 years ago

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 commented 6 years ago

Stacking should work fine On Wed, Sep 19, 2018 at 8:13 PM, Oliver Beckstein notifications@github.com 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 https://github.com/MDAnalysis/pmda/pull/61#issuecomment-423007250, or mute the thread https://github.com/notifications/unsubscribe-auth/AI0jB4r2h5gfovJXyTHiX8E6-ksa6I3mks5ucuvEgaJpZM4Wwpsp .

kain88-de commented 6 years ago

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 < notifications@github.com> 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 https://github.com/MDAnalysis/pmda/pull/61#issuecomment-423007250, 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 https://github.com/MDAnalysis/pmda/pull/61#issuecomment-423007700, or mute the thread https://github.com/notifications/unsubscribe-auth/AEGnVktHmJ42kuc8seU3yGjZlxF57_SWks5ucuxzgaJpZM4Wwpsp .

kain88-de commented 6 years ago

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

codecov[bot] commented 6 years ago

Codecov Report

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

Impacted file tree graph

@@           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%> (ø) :arrow_up:

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.

kain88-de commented 6 years ago

Ready for review

VOD555 commented 6 years ago

@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 commented 6 years ago

See #66 for a fix of the warning

On Thu 20. Sep 2018 at 22:32, VOD555 notifications@github.com 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 https://github.com/MDAnalysis/pmda/pull/61#issuecomment-423323226, or mute the thread https://github.com/notifications/unsubscribe-auth/AEGnVnuxRLeazuSBJICQdLXZYhE04ERAks5uc_tYgaJpZM4Wwpsp .