Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace unsupported get keyword with scheduler #48

Closed
orbeckst opened this issue Jun 26, 2018 · 5 comments
Closed

replace unsupported get keyword with scheduler #48

orbeckst opened this issue Jun 26, 2018 · 5 comments
Assignees
Labels

Comments

@orbeckst
Copy link
Member

orbeckst commented Jun 26, 2018

Expected behaviour

No warnings are raised when using dask.

PMDA should work with latest dask releases ≥ 0.18.0

Actual behaviour

Using the get= kwarg is deprecated removed (raises TypeError), instead use scheduler=

pmda/test/test_contacts.py::TestContacts::()::test_startframe
  /home/travis/miniconda/envs/test/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. "

This is now a TypeError.

Code to reproduce the behaviour

Currently version of MDAnalysis:

(run python -c "import MDAnalysis as mda; print(mda.__version__)")
(run python -c "import pmda; print(pmda.__version__)")
(run python -c "import dask; print(dask.__version__)")

@kain88-de
Copy link
Member

I think for a longer period we should support both keywords. This allows people to use also an older dask version and not break scripts with another release.

@orbeckst
Copy link
Member Author

orbeckst commented Jun 26, 2018 via email

@VOD555
Copy link
Collaborator

VOD555 commented Oct 29, 2018

It seems in the new version of dask (0.20.0) released on 10/26/2018, it raises an error on the use of the get= keyword and set_options (http://docs.dask.org/en/latest/changelog.html).

@orbeckst orbeckst added the bug label Oct 29, 2018
@orbeckst orbeckst changed the title replace deprecated get keyword with scheduler replace unsupported get keyword with scheduler Oct 29, 2018
@orbeckst
Copy link
Member Author

I think for a longer period we should support both keywords. This allows people to use also an older dask version and not break scripts with another release.

Would be nice to support old and new dask, but dask is moving so quickly, I don't think we have the developer time to do this. Instead we will have to require dask ≥ 0.18.0 and have users update. At least that is relatively painless for dask.

@kain88-de
Copy link
Member

I'm also OK saying this library is cutting edge. We explicitly claim the library is not stable in the readme.

@kain88-de kain88-de mentioned this issue Oct 30, 2018
4 tasks
orbeckst added a commit that referenced this issue Oct 31, 2018
- fix #48
- updated boiler-plate code in ParallelAnalysisBase.run and copied and pasted into
  leaflet.LeafletFinder.run() (TODO: makes this more DRY)
- dask.distributed added as dependency (it is recommended by dask for a single node anyway, and
  it avoids imports inside if statements... much cleaner code in PMDA)
- removed scheduler kwarg: use dask.config.set(scheduler=...)
- 'multiprocessing' and n_jobs=-1 are now only selected if nothing is set by dask;
  if one wants n_jobs=-1 to always grab all cores then you must set the multiprocessing
  scheduler
- default for n_jobs=1 (instead of -1), i.e., the single threaded scheduler
- updated tests
- removed unnecessary broken(?) test for "no deprecations" in parallel.ParallelAnalysisBase
- updated CHANGELOG
orbeckst added a commit that referenced this issue Oct 31, 2018
- fix #48
- updated boiler-plate code in ParallelAnalysisBase.run and copied and pasted into
  leaflet.LeafletFinder.run() (TODO: makes this more DRY)
- dask.distributed added as dependency (it is recommended by dask for a single node anyway, and
  it avoids imports inside if statements... much cleaner code in PMDA)
- removed scheduler kwarg: use dask.config.set(scheduler=...)
- 'multiprocessing' and n_jobs=-1 are now only selected if nothing is set by dask;
  if one wants n_jobs=-1 to always grab all cores then you must set the multiprocessing
  scheduler
- default for n_jobs=1 (instead of -1), i.e., the single threaded scheduler
- updated tests
- removed unnecessary broken(?) test for "no deprecations" in parallel.ParallelAnalysisBase
- updated CHANGELOG
@VOD555 VOD555 closed this as completed in #66 Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants