Refactor weather code plugins and CLIs #1944

MoseleyS · 2023-09-22T11:49:27Z

Time spent on this ticket should be charged to a specific code. Ask @MoseleyS.

As Enhancing Post Processing Science Lead, I want the IMPROVER weather code plugin to be refactored so that I can run a precipitation type code configuration.

The essential changes are around metadata. The output cube name and categorical attributes are now derived from the decision tree, which has been updated as a result and is now also a required input to the modal categories plugin.

To achieve this, I have done some refactoring:

code and acceptance test data moved to directories named categorical
CLI names changed:
- wxcode -> categorical
- wxcode-modal -> categorical-modes
Both these CLIs now require the decision tree, with the key name changing from wxtree to decision-tree
Plugin names changed:
- WeatherSymbols -> ApplyDecisionTree
- ModalWeatherCode -> ModalCategory
Several internal variable names and comments have been updated to be more generic.
Meta data definitions of the cube name, attribute names and contents have moved out of the code and into the decision tree.
Grouping and day -> night mapping have moved out of the code and into the decision tree.

Acceptance test data updates in metoppv/improver_test_data#28

For reviewing, I recommend looking in two parts:

Most of the refactoring is in the first four commits: aaf3ae6...5734e30
Then everything else: 5734e30...99f6802

This PR MUST be merged with the accompanying suite PR: (https://github.com/MetOffice/improver_suite/pull/1823)

Testing:

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

- CLI renamed from wxcode to categorical - WeatherSymbols plugin renamed to ApplyDecisionTree - Code moved from wxcode directory to categorical - Comments and variable names replaced with more agnostic definitions

- acceptance test data locations - extended docs locations

…ctionality.

- Includes method for mapping day and night symbols - Includes extended documentation

… day-night and groupings info.

…he CLI.

codecov · 2023-09-22T11:56:56Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (aaf3ae6) 98.38% compared to head (01af03c) 98.40%.
Report is 17 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1944      +/-   ##
==========================================
+ Coverage   98.38%   98.40%   +0.01%     
==========================================
  Files         123      124       +1     
  Lines       11795    12028     +233     
==========================================
+ Hits        11605    11836     +231     
- Misses        190      192       +2

Files	Coverage Δ
improver/categorical/decision_tree.py	`99.30% <100.00%> (ø)`
improver/categorical/modal_code.py	`100.00% <100.00%> (ø)`
improver/categorical/utilities.py	`100.00% <100.00%> (ø)`
improver/developer_tools/metadata_interpreter.py	`99.05% <100.00%> (ø)`

... and 16 files with indirect coverage changes

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

- Each leaf has a unique value - "if_night" values do not point to leaves that have "if_night" values

…such.

mspelman07

Thanks Stephen. A few comments mostly around clarifying some docstrings but otherwise this looks good.

doc/source/extended_documentation/categorical/build_a_decision_tree.rst

improver/categorical/decision_tree.py

improver/categorical/modal_code.py

improver/cli/categorical.py

MoseleyS · 2023-10-03T09:16:22Z

improver.categorical.utilities.categorical_attributes improver.developer_tools.metadata_interpreter.MOMetadataInterpreter (in function run)

The categorical utilities just has a few variables called wx_keys which probably wants updating but I'm not sure about the developer_tools one?

I've updated the variable names in both files. This allows us to add other categorical data to the metadata interpreter in the future, if required.

gavinevans

Thanks @MoseleyS 👍

I've added some very minor comments.

doc/source/extended_documentation/categorical/build_a_decision_tree.rst

improver/categorical/decision_tree.py

improver_tests/categorical/decision_tree/test_ApplyDecisionTree.py

improver/categorical/decision_tree.py

improver/categorical/utilities.py

improver_tests/categorical/decision_tree/__init__.py

improver_tests/categorical/test_utilities.py

…another utility. Adds test.

gavinevans

Thanks for the updates, @MoseleyS 👍

I've added one trivial comment.

improver/categorical/modal_code.py

* Refactors wxcode to be more agnostic - CLI renamed from wxcode to categorical - WeatherSymbols plugin renamed to ApplyDecisionTree - Code moved from wxcode directory to categorical - Comments and variable names replaced with more agnostic definitions * Fixes failing tests following refactor * isort * More refactoring - acceptance test data locations - extended docs locations * Updates testing decision tree and check_tree method and tests * Updates categorical attribute generation and tests, and day_night functionality. * categorical utilities tests now all pass * Updates tests for create_categorical_cube * Updates ApplyDecisionTree and tests - Includes method for mapping day and night symbols - Includes extended documentation * Updates ModalWeatherCode which now needs the decision_tree to get the day-night and groupings info. * Renames ModalWeatherCode as ModalCategory and adds decision_tree to the CLI. * Updates checksums * Removes obsolete KeyError trap * Updates checksums * Tests bare category error in check tree * Adds test for day_night_map * Adds checks to make sure that leaf nodes meet these criteria - Each leaf has a unique value - "if_night" values do not point to leaves that have "if_night" values * isort * Adds test for case where a leaf is unreachable and isn't declared as such. * Adds "is_unreachable" to decision tree documentation * Updates following review. * Removes `wx` from variable names in categorical_attributes * Removes `wx` from variable names in metadata_interpreter.py * Simple review updates (non-functional) * Moves simple is_decision_node method into utilities and reuses it in another utility. Adds test. * Simplifies test to one line * Uses is_decision_node in another place * Grammar

MoseleyS added 12 commits September 21, 2023 08:38

Refactors wxcode to be more agnostic

240ed3d

- CLI renamed from wxcode to categorical - WeatherSymbols plugin renamed to ApplyDecisionTree - Code moved from wxcode directory to categorical - Comments and variable names replaced with more agnostic definitions

Fixes failing tests following refactor

4a55f1f

isort

54f63be

More refactoring

5734e30

- acceptance test data locations - extended docs locations

Updates testing decision tree and check_tree method and tests

77dead5

Updates categorical attribute generation and tests, and day_night fun…

2a82caa

…ctionality.

categorical utilities tests now all pass

55bad94

Updates tests for create_categorical_cube

76795c7

Updates ApplyDecisionTree and tests

cf698ff

- Includes method for mapping day and night symbols - Includes extended documentation

Updates ModalWeatherCode which now needs the decision_tree to get the…

403177d

… day-night and groupings info.

Renames ModalWeatherCode as ModalCategory and adds decision_tree to t…

97ea446

…he CLI.

Updates checksums

4e3fb7e

MoseleyS added the don't merge yet label Sep 22, 2023

MoseleyS mentioned this pull request Sep 22, 2023

Test data update for weather code refactoring metoppv/improver_test_data#28

Merged

MoseleyS added 6 commits September 22, 2023 13:04

Removes obsolete KeyError trap

e0ca878

Updates checksums

fd2140d

Tests bare category error in check tree

6603c48

Adds test for day_night_map

ec4a11b

Adds checks to make sure that leaf nodes meet these criteria

bf4bdaa

- Each leaf has a unique value - "if_night" values do not point to leaves that have "if_night" values

isort

99f6802

MoseleyS self-assigned this Sep 22, 2023

MoseleyS added 2 commits September 22, 2023 16:01

Adds test for case where a leaf is unreachable and isn't declared as …

6119466

…such.

Adds "is_unreachable" to decision tree documentation

dd9202a

MoseleyS marked this pull request as ready for review September 22, 2023 15:43

MoseleyS removed their assignment Sep 22, 2023

bayliffe assigned mspelman07 Sep 26, 2023

mspelman07 requested changes Sep 27, 2023

View reviewed changes

mspelman07 assigned MoseleyS and unassigned mspelman07 Sep 27, 2023

MoseleyS assigned mspelman07 and unassigned MoseleyS Oct 3, 2023

mspelman07 previously approved these changes Oct 3, 2023

View reviewed changes

mspelman07 removed their assignment Oct 3, 2023

gavinevans self-assigned this Oct 20, 2023

gavinevans requested changes Oct 20, 2023

View reviewed changes

gavinevans assigned MoseleyS and unassigned gavinevans Oct 20, 2023

Simple review updates (non-functional)

6c8a616

MoseleyS dismissed mspelman07’s stale review via 6c8a616 November 1, 2023 12:43

MoseleyS added 3 commits November 1, 2023 14:09

Moves simple is_decision_node method into utilities and reuses it in …

d3173e5

…another utility. Adds test.

Simplifies test to one line

8730e03

Uses is_decision_node in another place

1ae849d

MoseleyS assigned gavinevans and unassigned MoseleyS Nov 1, 2023

gavinevans requested changes Nov 2, 2023

View reviewed changes

improver/categorical/modal_code.py Outdated Show resolved Hide resolved

gavinevans assigned MoseleyS and unassigned gavinevans Nov 2, 2023

Grammar

01af03c

MoseleyS assigned gavinevans and unassigned MoseleyS Nov 2, 2023

gavinevans approved these changes Nov 2, 2023

View reviewed changes

gavinevans assigned MoseleyS and unassigned gavinevans Nov 2, 2023

mspelman07 mentioned this pull request Nov 13, 2023

Categorical plugin accepts deterministic conditions #1965

Merged

2 tasks

MoseleyS removed the don't merge yet label Nov 14, 2023

MoseleyS merged commit 3eee0a9 into metoppv:master Nov 14, 2023
10 checks passed

MoseleyS deleted the refactor_wxcode branch November 14, 2023 13:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor weather code plugins and CLIs #1944

Refactor weather code plugins and CLIs #1944

MoseleyS commented Sep 22, 2023 •

edited

Loading

codecov bot commented Sep 22, 2023 •

edited

Loading

mspelman07 left a comment

MoseleyS commented Oct 3, 2023

gavinevans left a comment

gavinevans left a comment

Refactor weather code plugins and CLIs #1944

Refactor weather code plugins and CLIs #1944

Conversation

MoseleyS commented Sep 22, 2023 • edited Loading

codecov bot commented Sep 22, 2023 • edited Loading

Codecov Report

mspelman07 left a comment

Choose a reason for hiding this comment

MoseleyS commented Oct 3, 2023

gavinevans left a comment

Choose a reason for hiding this comment

gavinevans left a comment

Choose a reason for hiding this comment

MoseleyS commented Sep 22, 2023 •

edited

Loading

codecov bot commented Sep 22, 2023 •

edited

Loading