Add skeleton for generation of derived solar fields for RainForests #1705

benowen-bom · 2022-04-28T08:12:30Z

This PR adds a skeleton for CLIs and plugins for generation of derived solar fields that are required as inputs for RainForests calibration of precipitation accumulations.

Future PRs following on from this will add in functionality and tests, however the CLI interfaces should remain unchanged from those presented herein.

Testing:

Ran tests and they passed OK
Added new tests for the new feature(s) (No new features added)

CLA

If a new developer, signed up to CLA

codecov · 2022-04-28T08:17:37Z

Codecov Report

Merging #1705 (25a1271) into master (51644de) will decrease coverage by 0.02%.
The diff coverage is 89.28%.

@@            Coverage Diff             @@
##           master    #1705      +/-   ##
==========================================
- Coverage   98.15%   98.13%   -0.03%     
==========================================
  Files         110      111       +1     
  Lines       10162    10190      +28     
==========================================
+ Hits         9975    10000      +25     
- Misses        187      190       +3

Impacted Files	Coverage Δ
...erate_ancillaries/generate_derived_solar_fields.py	`89.28% <89.28%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 51644de...25a1271. Read the comment docs.

tjtg

This is a good starting point for bringing these derived solar fields used by RainForests into IMPROVER. I've made a few comments, mostly around the CLI and plugin interface.

improver/cli/generate_clearsky_solar_radiation.py

improver/generate_ancillaries/generate_derived_solar_fields.py

improver/cli/generate_clearsky_solar_radiation.py

tjtg · 2022-04-29T06:54:58Z

Codecov is currently "failing" as it expects every PR to have line coverage at least equal to the project-wide coverage (currently ~98%). Personally, I think that's an excessive requirement and would prefer codecov to present the information then let reviewers decide whether test coverage is appropriate or not.

There seems to be a way to configure a lower threshold - see codecov docs. I'll look at creating a PR for that configuration change next week.

tjtg · 2022-05-04T04:55:05Z

I'm happy with this now - ready for a second reviewer to take a look.

benowen-bom · 2022-05-04T04:58:57Z

I'm happy with this now - ready for a second reviewer to take a look.

Great. Thanks for help on improving this!

bayliffe

A few comments to consider. Thanks.

improver/cli/generate_solar_time.py

improver/cli/generate_clearsky_solar_radiation.py

…skySolarRadiation Plugin.

…and linke-turbidity.

benowen-bom · 2022-05-05T04:26:56Z

@tjtg Based on @bayliffe review, a few things have changed (and been added), so I think it would be worthwhile for you to have another look at this.

improver/generate_ancillaries/generate_derived_solar_fields.py

improver/cli/generate_clearsky_solar_radiation.py

bayliffe · 2022-05-05T07:20:07Z

A couple more tiny things, but I'm otherwise happy with this now. Thanks again.

tjtg

I've had another look through and all looks good now. @bayliffe, please check you're happy with those last few remaining items and update your review status when you're ready.

…etoppv#1705) * Add modules to evaluate the derived solar fields. * Fill out Solar-time CLI, and add skeleton GenerateSolarTime Plugin. * Fill out Clearsky solar-radiation CLI, and add skeleton GenerateClearskySolarRadiation Plugin. * Update docstrings. * Fix license. * Assume single valid time for linke turbidity rather than climatology. * Add modules to evaluate the derived solar fields. * Fix license. * Update typing in CLIs. * Linting. * Update temporal_spacing typing. * Add datetime value converter. * Update contributors list. * Update interfaces to reflect logic around default values for alitude and linke-turbidity. * Add function to create cube from target grid. * Change to using create_new_diagnostic_cube. * Formatting changes. * Replace altitude by surface_altitude. * Update docstring for temporal_spacing. * Reorder args to ensure cubes are bucnhed together in the interface. * Change time to named arg to bring into line with other CLIs. * Move initialisation of default cubes into plugin. * Add unit tests for _initialise_input_cubes. * Typo and linting fix. * Update docstrings and fix typo, as per reviewer comments.

benowen-bom changed the title ~~Rainforest derived fields skeleton~~ Add skeleton for generation of derived solar fields for RainForests Apr 28, 2022

tjtg reviewed Apr 29, 2022

View reviewed changes

tjtg added the MO review required PRs opened by non-Met Office developers that require a Met Office review label Apr 29, 2022

This was referenced May 2, 2022

Inconsistent date/time handling in CLIs #1707

Open

Avoid codecov marking PRs as failed #1709

Merged

bayliffe requested changes May 4, 2022

View reviewed changes

benowen-bom added 20 commits May 5, 2022 14:16

Add modules to evaluate the derived solar fields.

6163941

Fill out Solar-time CLI, and add skeleton GenerateSolarTime Plugin.

a93be6b

Fill out Clearsky solar-radiation CLI, and add skeleton GenerateClear…

f9ff51d

…skySolarRadiation Plugin.

Update docstrings.

d0f7362

Fix license.

83b1cab

Assume single valid time for linke turbidity rather than climatology.

fb0caff

Add modules to evaluate the derived solar fields.

af4159b

Fix license.

9de656c

Update typing in CLIs.

f71a32b

Linting.

d2595f3

Update temporal_spacing typing.

b050644

Add datetime value converter.

1bb3e36

Update contributors list.

d3e83c6

Update interfaces to reflect logic around default values for alitude …

59a749a

…and linke-turbidity.

Add function to create cube from target grid.

5638b18

Change to using create_new_diagnostic_cube.

c508299

Formatting changes.

0e1926a

Replace altitude by surface_altitude.

ac484db

Update docstring for temporal_spacing.

005a0b2

Reorder args to ensure cubes are bucnhed together in the interface.

2274143

benowen-bom added 4 commits May 5, 2022 14:19

Change time to named arg to bring into line with other CLIs.

043db06

Move initialisation of default cubes into plugin.

68dcb7c

Add unit tests for _initialise_input_cubes.

a52bf51

Typo and linting fix.

3fa2414

benowen-bom force-pushed the rainforest_derived_fields_skeleton branch from 6ed1701 to 3fa2414 Compare May 5, 2022 04:25

benowen-bom requested a review from tjtg May 5, 2022 04:25

bayliffe reviewed May 5, 2022

View reviewed changes

improver/generate_ancillaries/generate_derived_solar_fields.py Outdated Show resolved Hide resolved

bayliffe reviewed May 5, 2022

View reviewed changes

improver/cli/generate_clearsky_solar_radiation.py Outdated Show resolved Hide resolved

Update docstrings and fix typo, as per reviewer comments.

25a1271

tjtg approved these changes May 6, 2022

View reviewed changes

bayliffe approved these changes May 6, 2022

View reviewed changes

bayliffe merged commit f52049f into metoppv:master May 6, 2022

benowen-bom mentioned this pull request May 9, 2022

RainForests clearsky solar radiation cube functionality #1712

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add skeleton for generation of derived solar fields for RainForests #1705

Add skeleton for generation of derived solar fields for RainForests #1705

benowen-bom commented Apr 28, 2022 •

edited

Loading

codecov bot commented Apr 28, 2022 •

edited

Loading

tjtg left a comment

tjtg commented Apr 29, 2022

tjtg commented May 4, 2022

benowen-bom commented May 4, 2022

bayliffe left a comment

benowen-bom commented May 5, 2022

bayliffe commented May 5, 2022

tjtg left a comment •

edited

Loading

Add skeleton for generation of derived solar fields for RainForests #1705

Add skeleton for generation of derived solar fields for RainForests #1705

Conversation

benowen-bom commented Apr 28, 2022 • edited Loading

codecov bot commented Apr 28, 2022 • edited Loading

Codecov Report

tjtg left a comment

Choose a reason for hiding this comment

tjtg commented Apr 29, 2022

tjtg commented May 4, 2022

benowen-bom commented May 4, 2022

bayliffe left a comment

Choose a reason for hiding this comment

benowen-bom commented May 5, 2022

bayliffe commented May 5, 2022

tjtg left a comment • edited Loading

Choose a reason for hiding this comment

benowen-bom commented Apr 28, 2022 •

edited

Loading

codecov bot commented Apr 28, 2022 •

edited

Loading

tjtg left a comment •

edited

Loading