Skip to content

Adds dask lock capability for backend writes#800

Merged
shoyer merged 1 commit intopydata:masterfrom
pwolfram:dask_async_lock_fix
Mar 22, 2016
Merged

Adds dask lock capability for backend writes#800
shoyer merged 1 commit intopydata:masterfrom
pwolfram:dask_async_lock_fix

Conversation

@pwolfram
Copy link
Contributor

This fixes an error on an asynchronous write for to_netcdf
resulting in an dask.async.RuntimeError: NetCDF: HDF error

Resolves issue #793
following dask improvement at dask/dask#1053
following advice of @shoyer.

if self.sources:
import dask.array as da
da.store(self.sources, self.targets)
import dask, dask.array as da
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep each import to its own line

@shoyer
Copy link
Member

shoyer commented Mar 22, 2016

This is going to be hard to test, but honestly I don't think that's essential given the simplicity of this code -- the complexity is buried upstream in dask, which does have comprehensive tests.

If you do want to write a test for this, you could mock out a fake DataStore similar to what we wrote for the dask tests and turn on multi-threading. But I think that's probably overkill here.

This fixes an error on an asynchronous write for `to_netcdf`
resulting in an `dask.async.RuntimeError: NetCDF: HDF error`

Resolves issue pydata#793
following dask improvement at dask/dask#1053
following advice of @shoyer.
@pwolfram pwolfram force-pushed the dask_async_lock_fix branch from 5cca26a to 759db6e Compare March 22, 2016 22:12
@pwolfram
Copy link
Contributor Author

Thanks @shoyer for the review. I agree regarding this being simple enough to not require testing so this PR is probably ready to go.

shoyer added a commit that referenced this pull request Mar 22, 2016
Adds dask lock capability for backend writes
@shoyer shoyer merged commit 4fdf6d4 into pydata:master Mar 22, 2016
@shoyer
Copy link
Member

shoyer commented Mar 22, 2016

Thanks @pwolfram !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants