Mobt 2066 convert forecast to climatological anomaly

[maxwhitemet]

Addresses #2066

Testing:

• Ran tests and they passed OK
• Added new tests for the new feature(s)

The structure of the new CalculateClimateAnomalies class follows that agreed with @brhooper

• Initialisation
• Verification of inputs
  • Matching standard names
  • Matching units
  • Matching grids
  • Matching time coordinates
• Calculation of anomalies: both unstandardised and standardised
• Creation of output cube with appropriate metadata

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.42%. Comparing base (84a8944) to head (93cecae).
Report is 74 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2072      +/-   ##
==========================================
+ Coverage   98.39%   98.42%   +0.02%     
==========================================
  Files         124      135      +11     
  Lines       12212    13380    +1168     
==========================================
+ Hits        12016    13169    +1153     
- Misses        196      211      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[brhooper]

Thanks for this @maxwhitemet, it's a good start to fulfilling the requirements t have a plugin which calculates anomaly data for use in the SAMOS work.

I've added some comments to the plugin and what I think are some spurious changes to the checksums. Once these have been resolved, I'll review the unit tests more thoroughly. I've not done this in detail yet as I think that some of the changes I've suggested to the plugin will necessitate changes to the tests anyway.

[brhooper]

Thanks @maxwhitemet - I think we're heading in the correct direction on this. I've added some comments to the plugin changes you've made. Once we get the plugin finalised we can talk in more depth about the tests, though I will say that codecov bot thinks that some lines of code are not being touched by the tests.

[brhooper]

Thanks @maxwhitemet, I think we're getting there with this PR. I've left a few comments on the plugin itself. We've also had a chat about how the unit tests can be re-written to make it easier for people reading the code to understand what is being tested, as well as sketching out what tests are required.

[brhooper]

Thanks @maxwhitemet, your latest changes have really improved the unit tests. I've made some additional requests for changes, but overall I think that the PR is close to be ready for another review.

[brhooper]

Thanks @maxwhitemet. I've suggested a few small changes.

[maxwhitemet]

Thank you for your review @brhooper. I have implemented the suggested changes.

[brhooper]

Thanks @maxwhitemet, I think I'm happy with this now. I've added 3 very minor comments. I think we might also need to talk about running the formatting tests (black, isort, flake8 etc) detailed here using, for example bin/improver-tests black.

I also think that your checking of the reference_epoch coordinate could be replaced with a simple function to reduce repeated code. Something like:

def check_reference_epoch_coord(result_cube, reference_cube):
    """Check that the reference_epoch coordinate on the result cube is as expected."""
    if "reference_epoch" not in [coord.name() for coord in result_cube.coords()]:
        return False
    else:
        results = []
        results.append(
            result_cube.coord("reference_epoch").points
            == reference_cube.coord("time").points
        )
        results.append(
            np.array_equal(
                result_cube.coord("reference_epoch").bounds,
                reference_cube.coord("time").bounds,
            )
        )
        results.append(
            result_cube.coord("reference_epoch").units
            == reference_cube.coord("time").units
        )
        return all(results)

and then using the following within each test:

assert check_reference_epoch_coord(result_cube, mean_cube)

though I'm happy to leave this decision to you/the 2nd reviewer.

Overall, I'm happy to move this ticket into 2nd review now, with you making the 3 minor changes indicated in my comments. Thanks for all your work on this.

[gavinevans]

Thanks @maxwhitemet 👍

I've added some comments mainly related to getting rid of the long times from docstrings and code comments, and the addition of type hints.

[maxwhitemet]

Thank you for your review @gavinevans. I have made the requested changes.

[gavinevans]

Thanks @maxwhitemet 👍

I've added a few more minor comments.

[maxwhitemet]

Thanks for the further comments @gavinevans 👍
I have implemented the requested changes.

[brhooper]

Thanks @maxwhitemet, this all looks fine to me. Good work.