Hi Sadie,

This is a really nice idea. Perhaps we could go one step further and potentially avoid having to read the data from disk each time. If we defined simple wrappers for the stats functions, they could share the same underlying data. E.g.

import dask.array as da
from dask import delayed, compute

def max(x):
    return x.max()

def min(x):
    return x.min()

dx = da.arange(100, chunks=50)

stats = (delayed(min, pure=True)(dx), delayed(max, pure=True)(dx))
compute(*stats)
# (0, 99)

It would involve writing a little function for each stat (or lambdas - but I don't know how that would play with the weights kwarg), but they would be contained within the stats method ... What do you think?

(I'm sure you can visualise the above graph, but I couldn't work out how. The docs imply that the dx will be shared between functions, though, I think)

Thanks David, this:

Perhaps we could go one step further and potentially avoid having to read the data from disk each time. If we defined simple wrappers for the stats functions, they could share the same underlying data.

is a great idea. Once I've tied up some loose ends elsewhere I will have a crack at getting it implemented in the way you suggest, which also sounds nice to me.

Hi @davidhassell, I got back onto this one this week (intermittently) as per our priority list. I've updated it to incorporate your idea in #432 (comment). Let me know if I've implemented that as you intended, correctly, and if there is any further feedback. Saying that, there are a few aspects we should agree on regarding the desired output for stats in light of API updates made elsewhere during the Dask migration:

Namely, the 'new way' with Dask migration, where we return Data objects rather than values (sorry I can't find the relevant PR or Issue where we discussed that change, after quite a bit of searching, maybe you can remember which one it was?) means that the outputs (return dict values corresponding to each stats operation key) have gone from Python scalars/numbers to cf.Data containing the corresponding values, e.g. 1.0 -> cf.Data(1.0):

• I have updated the PR in line with that, but do we actually want that for the output, or do we want to instead return the scalar number values, as before? E.g. the <scalar> instead of cf.Data(<scalar>)?
• The dict value for the 'sample size' operation ("sample size" key) emerges as cf.Data([<scalar>]), i.e. has ndim of 1 rather than 0 like the others. Is that what we want? I would argue that we should have consistency across the outputs in ndim and therefore we should change it to be cf.Data(<scalar>). Maybe that was the intention anyway and I have not coded that aspect up correctly?

Regarding the fact that the data type of the scalar values themselves is not consistent, at least for our testing case with a stats input of cf.Data[1, 1] and weights=1 set, namely that the values are in some cases float and in some cases int (see the test_Data_stats dict compared output now, noting that this is the form when the test passes, with the stats method code as-is), I think that is fine because it makes sense in the context of the different operations calculations, .e.g a maximum of 1 and 1 is 1 not 1.0, but the mean is 1.0.

Updating now in line with an offline discussion with @davidhassell, in which we realised that the approach in #432 (comment) wasn't worthwhile. In the meantime, David, what are your thoughts regarding the output value format and ndim as I outline in #432 (comment), in case we want to tweak the output format?

Hi Sadie - thanks for the nudge. This is a bit tricky, as the results haven't actually been computed, now! Perhaps we could have a compute keyword parameter, that defaults to True, which determines whether or not computed <scalar> values are returned, or else non-computed Data objects?

Perhaps we could have a compute keyword parameter, that defaults to True, which determines whether or not computed values are returned, or else non-computed Data objects?

Aha, I like that idea! Thanks, let's do that. I'll add it in today. I was also updating the test to cover a representative range of possible input parameters since it was too basic as it was. I'll push the commits up shortly.

All feedback incorporated so this is now ready for re-review @davidhassell. Thanks. (Note sinec last review I've also greatly improved the stats test coverage and added any outstanding test warning messages to those to be filtered out, to keep us from being spammed by them when running the test_Data overall.)

@davidhassell ready for re-review I think. Have my last two commits amended this to what you were thinking, namely:

I think we should return the actual Data objects when compute=False, rather than dask.delayed objects.

?

I assume the Data from the compute=False outputs are showing data values when printed because can_compute is set to return True for development purposes at the moment? Or because a representation operation forces a compute, at least right now? Unless I have missed something?

This look perfect to me. I tried:

In [1]: import cf

In [2]: f = cf.example_field(0)

In [3]: f.data.stats()
Out[3]: 
{'minimum': 0.003,
 'mean': 0.046075,
 'median': 0.036,
 'maximum': 0.146,
 'range': 0.143,
 'mid_range': 0.0745,
 'standard_deviation': 0.03613612285511549,
 'root_mean_square': 0.05855531572795078,
 'sample_size': 40}

In [4]: f.data.stats(compute=False)
Out[4]: 
{'minimum': <CF Data(): 0.003 1>,
 'mean': <CF Data(): 0.046075 1>,
 'median': <CF Data(): 0.036 1>,
 'maximum': <CF Data(): 0.146 1>,
 'range': <CF Data(): 0.143 1>,
 'mid_range': <CF Data(): 0.0745 1>,
 'standard_deviation': <CF Data(): 0.03613612285511549 1>,
 'root_mean_square': <CF Data(): 0.05855531572795078 1>,
 'sample_size': <CF Data(1, 1): [[40]]>}

In [5]: 

The values are appearing in the Data repr for exactly the reason you say (can_compute=True), but if we did stats = f.data.stats(compute=False) then we'd still be lazy, of course.

The values are appearing in the Data repr for exactly the reason you say (can_compute=True), but if we did stats = f.data.stats(compute=False) then we'd still be lazy, of course.

Brilliant, thanks for clarifying. Merging now.