8000 Categorical plugin accepts deterministic conditions by mspelman07 · Pull Request #1965 · metoppv/improver · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Categorical plugin accepts deterministic conditions #1965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 27, 2023

Conversation

mspelman07
Copy link
Contributor
@mspelman07 mspelman07 commented Nov 13, 2023

This PR makes some further changes to the wx symbols plugin so it can work with a decision tree using deterministic inputs. This PR builds on PR #1944 which refactored the code to work with any decision tree. This branch points at master but can't be merged before the previous PR.

I have attempted to write this code so that it doesn't require any changes to the current weather symbols decision tree. This is partially to limit the number of changes that need to be made but also so I can show that the changes I've made don't effect the existing decision tree.

Testing:

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

Copy link
codecov bot commented Nov 13, 2023

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (3eee0a9) 98.40% compared to head (f215cb4) 98.39%.

Files Patch % Lines
improver/categorical/decision_tree.py 96.15% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1965      +/-   ##
==========================================
- Coverage   98.40%   98.39%   -0.01%     
==========================================
  Files         124      124              
  Lines       12028    12055      +27     
==========================================
+ Hits        11836    11862      +26     
- Misses        192      193       +1     

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

@MoseleyS MoseleyS changed the title Wx refactor 2 Categorical plugin accepts deterministic conditions Nov 14, 2023
Copy link
Contributor
@MoseleyS MoseleyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this does what we need, but I have some suggestions to improve the code.

@MoseleyS MoseleyS assigned mspelman07 and unassigned MoseleyS Nov 20, 2023
@MoseleyS
Copy link
Contributor

I think I should have reassigned this to Marcus last week.

Copy link
Contributor
@MoseleyS MoseleyS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of things to change, and one to consider.

@MoseleyS MoseleyS removed their assignment Nov 23, 2023
@brhooper brhooper self-assigned this Nov 24, 2023
Copy link
Contributor
@brhooper brhooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mspelman07. This looks fine - tests show that behaviour is unchanged for existing, probabilistic decision trees, and the new functionality for deterministic decision trees appears to work as intended.

@brhooper brhooper merged commit ebbfe31 into metoppv:master Nov 27, 2023
@brhooper brhooper assigned mspelman07 and unassigned brhooper Nov 27, 2023
@mspelman07 mspelman07 added the EPP PR's related to Enhancing Post Processing team label Jan 9, 2024
MoseleyS pushed a commit to MoseleyS/improver that referenced this pull request Aug 22, 2024
* Make check decision tree work with deterministic nodes

* Adds in tests and changes to make ApplyDecisionTree work with deterministic inputs

* Formatting

* Update documentation to include deterministic nodes

* Review response and add test for missing input for deterministic forecasts

* Update handling of strings and review response

* formatting

---------

Co-authored-by: Marcus Spelman <marcus.spelman@metoffice.gov.uk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EPP PR's related to Enhancing Post Processing team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0