Rationalise percentile methods

[rcomer]

The _percentile function that iris.analysis.PERCENTILE is based on allows the keyword fast_percentile_method so that np.percentile may be used in place of scipy.mstats.mquantiles. However, this option does not appear in the documentation for iris.analysis.PERCENTILE. [Addresssed in #3901]

Also, is there any reason why the _percentile function and PERCENTILE aggregator shouldn't allow passing of **kwargs to np.percentile if this option is chosen? If implemented, this would allow users to specify an interpolation method in np.percentile. [Addressed in #4791]

[mo-g]

Stylistically, would it be cleaner to allow passing all kwargs, to filter passed kwargs, or to manually expose the desired functionality by manually supporting and passing the specific argument?

It's been so long since I coded with others, that I honestly don't remember at all.

[DPeterK]

It's probably just due to ignorance / oversight at the time of putting PERCENTILE together... Have you tried just passing the kwarg and seeing what happens? If it all just works then a quick docs update would be great, and if it doesn't and you feel like tackling the error (for example, the return type / shape / other from the NumPy functionality may not match what's expected by Iris) then that's also most welcome, of course 👍

[rcomer]

I should probably add, this was prompted by a question on our internal Yammer. The user specifically wanted to use "nearest" interpolation method, which does not appear to be available with scipy.mstats.mquantiles, but is available in np.percentile.

Have you tried just passing the kwarg and seeing what happens?

I guess you mean the fast_percentile_method kwarg? No I haven't, but I'd be willing to bet that you're right and it just works. Any thoughts on additional passing of the interpolation kwarg? As @mo-g points out, the right way to do that is not trivially obvious. It also occurs to me that we have 3 methods that do very similar things and we don't want to confuse users more than necessary:

• PERCENTILE with fast_percentile_method kwarg: fastest but does not handle weights or masked data.
• straight PERCENTILE: slower but handles masked data.
• WPERCENTILE: (probably) slowest but handles both masked data and weights.

[DPeterK]

You might just be able to pass interpolation='nearest', but I'm much less convinced that will work as expected!

Sometime, a bunch of people sitting down and looking very hard at the aggregators would be a really good idea. In particular it would be good to look at percentile because it necessarily doesn't work quite the same as the other aggregators, and there are now three very similar aggregators that we might be able to bring together to some extent...

[mo-g]

Percentile, WPercentile and ...?

Edit: I get it. Silly me!

[pp-mo]

@rcomer we have 3 methods that do very similar things ...

• PERCENTILE with fast_percentile_method ... not handle weights or masked data.
• straight PERCENTILE ... handles masked data.
• WPERCENTILE ... handles both masked data and weights.

Well I hadn't grasped that : That's a really nice simple description 🥇
So, if those different scopes are determined entirely by the keyword usage, and the results are equivalent where the scopes overlap, then that sounds like a really easy win waiting to happen ...

Does your understanding support that possibility ?

[rcomer]

if those different scopes are determined entirely by the keyword usage

keywords are:

• PERCENTILE with fast_percentile_method: use interpolate kwarg
• straight PERCENTILE: use alphap and betap kwargs
• WPERCENTILE: use kind kwarg [edit: and weights kwarg]

the results are equivalent where the scopes overlap

No idea, and it might be a reasonable amount of work to test since there are several permutations. OTOH, in all three cases we are ultimately leaving it to a numpy or scipy function, so it seems reasonable to me to just reference said function in the docs so that the user can verify whether it really does what they require.

[pp-mo]

@mo-g Stylistically, would it be cleaner to allow passing all kwargs, to filter passed kwargs, or to manually expose the desired functionality by manually supporting and passing the specific argument?

Matplotlib style is to pass all kwargs to possibly multiple underlying routines, try to have different keywords to adjust different aspects (e.g. 'markeredgecolor', 'markeredgewidth' etc), and each routine just ignores what it doesn't recognise.
The problem with that style is that there is no validity checking : a miss-spelled keyword causes no warning or error.

Another way is to package kwargs in dictionary keywords, e.g. percentile_kwargs=dict(nearest=True). More flexible but not neat for user code.

What Iris does (?mostly) is to name and document the supported keys at the top level.
Which has the benefit of clarity, but means Iris doesn't automatically support underlying options, especially if they evolve with time.

[rcomer]

To add to the mix, as noted at #3901 (comment) there is also a numpy.nanpercentile. This could give us a method for masked arrays that is equivalent to our current "fast percentile method" in terms of calling sequence.

[rcomer]

Updated title to reflect the wider issues in this discussion.

[rcomer]

@pp-mo Matplotlib style is to pass all kwargs to possibly multiple underlying routines, try to have different keywords to adjust different aspects (e.g. 'markeredgecolor', 'markeredgewidth' etc), and each routine just ignores what it doesn't recognise.
The problem with that style is that there is no validity checking : a miss-spelled keyword causes no warning or error.

Is that problem because they have multiple underlying functions? In our case we choose one underlying numpy/scipy function and so could pass all the kwargs to that chosen function. Then the numpy/scipy function would presumably throw an error if you tried to pass the wrong kwargs.

[rcomer]

I'm going to close this one as the points raised in the OP have now been addressed. The conversation was rather more wide-ranging but I think it might be better to pull specific aspects out as their own issues if anyone wants to. That would make them easier to vote on, and easier to say if/when they've been addressed.