For some context, currently the from_zarr function doesn't always produce dask arrays with a unique .name attribute. This is because from_zarr uses the input zarr array repr to generate the output dask array name. For example,

import zarr
import dask
import dask.array as da

a = zarr.array([1, 2, 3])
b = zarr.array([4, 5, 6])
a_dsk = da.from_zarr(a)
b_dsk = da.from_zarr(b)
print(a_dsk.name)
print(b_dsk.name)
print(dask.base.tokenize(a_dsk) == dask.base.tokenize(b_dsk))

produces the following output on the current master:

zarr-<zarr.core.Array (3,) int64>
zarr-<zarr.core.Array (3,) int64>
True

The goal of this PR is to provide a fix such that from_zarr provides unique dask array names.

cc @jakirkham as you may be interested based on your discussion in zarr-developers/zarr-python#202

I have run such a test locally. The capability is innate to the from_array module, so in the original pass-through design, a test was not needed. The bug fix required was to allow proper unique hashing.

Here is the test code I have been using. I suppose it would be possible to built a separate pytest to test "again" the tokenizer, though I would think it to be redundant at present. With regards to how Dask handles creating unique hashes of large datasets in general, my intuition would be that this implies a bigger problem. For the sake of the immediate customer requirement, it seems low risk to utilize the proposed strategy for the bug fix, and devise a new strategy in a performance enhancement. At any rate, here is my current (external) unit test:

import dask.array as da
import zarr
a = zarr.open('a.zarr')
a.require_dataset('x', shape=(1000,), chunks=(100,), dtype='f8')
a['x'][()] = 3
b = zarr.open('b.zarr')
b.require_dataset('x', shape=(1000,), chunks=(100,), dtype='f8')
b['x'][()] = 5

print((a['x'][()] == b['x'][()]).any())
print(da.from_zarr(a['x']).name )
print(da.from_zarr(b['x']).name)
print(dask.base.tokenize(da.from_zarr(a['x'])))
print(dask.base.tokenize(da.from_zarr(b['x'])))

assert(da.from_zarr(a['x']).name != da.from_zarr(b['x']).name)
assert(da.from_zarr(a['x'],name='aasdf').all())
assert(da.from_zarr(b['x'],name='asdf').all())```

I agree that from_zarr ultimately passes name to from_array. I was suggesting to test the default case for from_zarr where name is None and a unique value for name is generated using tokenize. By running locally I can see that a unique name is indeed created, but a short test along the lines of

def test_from_zarr_unique_name():
    a = zarr.array([1, 2, 3])
    b = zarr.array([4, 5, 6])

    assert da.from_zarr(a).name != da.from_zarr(b).name

would act as a safeguard to ensure that this bug doesn't pop up again in the future.

In addition, since this PR adds the new name keyword to the from_zarr function signature, another short test that makes sure name is properly passed to from_array when specified would also be useful here. For example:

def test_from_zarr_name():
    a = zarr.array([1, 2, 3])
    assert da.from_zarr(a, name='foo').name == 'foo'

You may want to rebase on master to take those image and arrow xfails out of the diff

Why is appveryor having trouble with asciitree? I am running conda 4.6.8 locally, and the dependency of zarr on asciitree and fasteners is handled appropriately. I am new to appveyor, what is going on here?

ModuleNotFoundError: No module named 'asciitree'
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!
==================== 10 skipped, 1 error in 10.20 seconds =====================

The test failures showing up on Travis CI are related to a recent bokeh update and have been fixed in #4680.

Other than updating the name docstring description (ref #4663 (comment)), this PR looks good to me

Checking in here. Is there anything left to do on this PR?

There is one lingering (minor) comment about modifying the name docstring description (see #4663 (comment)). @mpeaton do you have any comments on the proposed description?

Thanks, I like your suggestion @mpeaton. One very minor comment, since there is no input parameter x for from_zarr, could we say "a hash of the input zarr array" instead of "a hash of x"?

Also, just as a note, currently the output from tokenize(z, ...) when z is a zarr array is random string. It corresponds to uuid.uuid4().hex returned here

@jrbourbeau I see. Would it be concise, yet accurate enough to replace x with url?
Which raises the question: What happens in the case where isinstance(url, str)? Does the mapper immediately evaluate the mapper or is it a lazy reference?

Would it be concise, yet accurate enough to replace x with url?

Thinking about this again, perhaps the most accurate, yet concise, description would be a general statement about hashing the input (since we're tokenize-ing all the from_zarr inputs like component and chunks). For example, the name description in the DataFrame.from_pandas function is:

A similar statement would work here too. "An optional key name for the array. Defaults to hashing the input." Apologies for going back and forth on this point.

Does the mapper immediately evaluate the mapper or is it a lazy reference?

Data should be loaded lazily

Thanks @mpeaton! I noticed this was your first code contribution to this repository, welcome!