Add support for dask and zarr arrays

[ejeschke]

This adds support for dask and zarr arrays into BaseImage-derived objects (e.g. AstroImage), e.g.

>>> aimg = AstroImage()
>>> aimg.load_data(dask_arr)

These images can then be loaded directly into a Ginga viewer.

Three new pytest files are added: one for numpy, dask and zarr

Developer documentation has been updated.

[ejeschke]

@pllim, no hurry.

[pllim]

Not sure if I'm qualified to review this one as I have never used dask nor zarr. Maybe someone from SunPy like @nabobalis is a better person to review?

[ejeschke]

@pllim, how about just a basic code review?

[pllim]

@ejeschke , sure, I'll have a look after you resolve the conflict.

[nabobalis]

Sorry! I clearly glossed over the email notification I got from this 15 days ago.

I've not used dask myself (outside of using xarray with it). I think either @Cadair or @wtbarnes have used it in a more direct form. I am happy to provide a code review regardless.

[ejeschke]

@pllim, @nabobalis, thank you! Yes, pleased to have any code review.

You can check out this gist to see the tests I did.

[nabobalis]

It looks good by me. That is some tricky reshaping to be done for zarr.

[Cadair]

I would be happy to review this, but wont be able to get to it until next week. If you could post a review request for me, it will end up on my list. :)

[pllim]

@Cadair , can't add you as reviewer, but added you to assignee.

[pllim]

I feel like the three test modules could be merged into one to avoid code duplication via subclassing and changing a few things here and there at setup.

I wonder if a rebase will fix RTD build for this PR.

[pllim]

Can you guarantee that d_obj would definitely be Dask object at this point? Or should this be an elif instead?

[ejeschke]

good question. The current code sort of assumes that by process of elimination. Probably it should be an elif with an exception raised if it didn't match anything before. Problem is that I want to detect the cases without having to import zarr or dask, because this becomes the basic slicing function. So if a good duck-typing test could be done that might be a possibility...

[ejeschke]

At this point I think the code for this is ok, we can refactor it later if a better test for dask arrays can be found.

[pllim]

Re: RTD failure -- we'll revisit if it persists after your next round of edits.

[ejeschke]

@Cadair, do you have some examples of large images (too large for RAM) that you open up using dask arrays?

[ejeschke]

@pllim, I believe I addressed all your points. Please have another look.

[ejeschke]

I feel like the three test modules could be merged into one to avoid code duplication via subclassing and changing a few things here and there at setup.

Maybe for a future PR? I want to add some more tests to the new test_trcalc and since those would overlap with these somewhat they could all be done together.

[pllim]

I think this can be optimized, especially if you are using sizable shape values:

In [30]: shape = (1000, 500)

In [31]: %timeit np.min(np.indices(shape), axis=0)
2.52 ms ± 13.7 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

In [32]: %timeit np.asarray([min(i, j) for i in range(shape[0]) for j in range(shape[1])]).reshape(shape)
92.7 ms ± 198 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

This comment also applies to all similar occurrences throughout.

[pllim]

@ejeschke , do you not wish to address this one? Either way is fine by me, but I just want to make sure it was not overlooked.

[ejeschke]

Whups, ok...pushed another commit. Have a look.

[pllim]

Why not also remove random here?

[ejeschke]

good catch.

[ejeschke]

Rebased, still passing all tests with latest conda installs of zarr and dask.

[pllim]

I haven't been using these two packages, so as long as tests pass and you are happy with it, FFTM.