123 crps for ensembles

[rob-taggart]

Adds crps_for_ensembles.

To do this I also wrote a second version of gather_dimensions called gather_dimensions2. The plan is to merge these two versions of gather_dimensions in a later pull request and standardise the way of calling it for existing metric functions. Tidying this up will be a separate pull request.

[nicholasloveday]

Nice work @rob-taggart . I have a few suggested changes.

I still need to work through the CRPS unit test with pen and paper to check that I get the same result.

[nicholasloveday]

Add a test that checks that this works with Dataset inputs. I think it should work find with Datasets.

[rob-taggart]

So it doesn't work for datasets.
For example, if I create a dataset ds then ds.dims returns Frozen({'station': 2}).
Whereas for a data array da, da.dims returns ('station',).

See this documentation for the difference in behaviour https://docs.xarray.dev/en/stable/generated/xarray.Dataset.dims.html#xarray.Dataset.dims

For now I have changed typehints to only accept xr.DataArray.
If gather_dimensions2 is to be used universally in scores, it will have to extract the dimensions as tuples from xr.Dataset and pd.Dataframe objects too. I think that is beyond the scope of this pull request, but happy to discuss.

[rob-taggart]

FYI, I've cross-referenced this comment to the issue concerned #125

[nicholasloveday]

Can you please also add crps for ensembles to the docs
https://github.com/nci/scores/blob/develop/docs/api.md#scoresprobability
and
https://github.com/nci/scores/blob/develop/docs/summary_table_of_scores.md
and
https://github.com/nci/scores/blob/develop/README.md

[nicholasloveday]

@rob-taggart I've finished this review for you

[rob-taggart]

@nicholasloveday , I've responded to all elements of your code review, including updating README etc.

I haven't squashed commits yet

[nicholasloveday]

Thanks @rob-taggart, all the changes look good - please tidy up the commit history. I'll let @tennlee have a look over it before it's merged.

[rob-taggart]

Thanks @nicholasloveday , commits have been squashed

[rob-taggart]

@tennlee , waiting on your review for this before it is merged

[tennlee]

Nice work @rob-taggart . I have a few suggested changes.

I still need to work through the CRPS unit test with pen and paper to check that I get the same result.

Nick, have you been through the tests with pen and paper as yet?

[tennlee]

Looks generally fine. Some words in the docstring for what to do with gather_dimensions and gather_dimensions2 in the meantime until suchtime as they have been integrated will be helpful. There are a few comments not responded to as yet - e.g. Nick's comment about validating the tests with pen and paper, and about adding the new methods to the docs. If those can be attended to, it should be ready to merge after that. I can see some work has been done on the docs, but because the conversations isn't marked as resolved, I'm not sure if all aspects of this have been fully completed or not.

[nicholasloveday]

Nice work @rob-taggart . I have a few suggested changes.
I still need to work through the CRPS unit test with pen and paper to check that I get the same result.

Nick, have you been through the tests with pen and paper as yet?

Yes I have done this and they look good to me.

[rob-taggart]

@tennlee , @nicholasloveday , I've added some comments in the gather_dimensions and gather_dimensions2 docstrings as requested. Also cross-referenced the two CRPS functions in their docstrings. Should be ready to merge now.

[rob-taggart]

@tennlee , the new tmp_coord_name is fine, just need to update the docstring. See review