From f64596517248c451a6462f780a4de0073418e287 Mon Sep 17 00:00:00 2001 From: HarvsG <11440490+HarvsG@users.noreply.github.com> Date: Thu, 29 Sep 2022 17:24:06 +0000 Subject: [PATCH] Add repair for missing Bayesian `prob_given_false` (#79303) --- .../components/bayesian/binary_sensor.py | 19 ++++++-- homeassistant/components/bayesian/repairs.py | 17 ++++++- .../components/bayesian/strings.json | 12 +++++ .../components/bayesian/translations/en.json | 4 ++ .../components/bayesian/test_binary_sensor.py | 44 +++++++++++++++++++ 5 files changed, 91 insertions(+), 5 deletions(-) create mode 100644 homeassistant/components/bayesian/strings.json diff --git a/homeassistant/components/bayesian/binary_sensor.py b/homeassistant/components/bayesian/binary_sensor.py index 0e943b2d0ad..706c7ecdfd7 100644 --- a/homeassistant/components/bayesian/binary_sensor.py +++ b/homeassistant/components/bayesian/binary_sensor.py @@ -3,6 +3,7 @@ from __future__ import annotations from collections import OrderedDict import logging +from typing import Any import voluptuous as vol @@ -34,7 +35,7 @@ from homeassistant.helpers.template import result_as_boolean from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType from . import DOMAIN, PLATFORMS -from .repairs import raise_mirrored_entries +from .repairs import raise_mirrored_entries, raise_no_prob_given_false ATTR_OBSERVATIONS = "observations" ATTR_OCCURRED_OBSERVATION_ENTITIES = "occurred_observation_entities" @@ -62,7 +63,7 @@ NUMERIC_STATE_SCHEMA = vol.Schema( vol.Optional(CONF_ABOVE): vol.Coerce(float), vol.Optional(CONF_BELOW): vol.Coerce(float), vol.Required(CONF_P_GIVEN_T): vol.Coerce(float), - vol.Required(CONF_P_GIVEN_F): vol.Coerce(float), + vol.Optional(CONF_P_GIVEN_F): vol.Coerce(float), }, required=True, ) @@ -73,7 +74,7 @@ STATE_SCHEMA = vol.Schema( vol.Required(CONF_ENTITY_ID): cv.entity_id, vol.Required(CONF_TO_STATE): cv.string, vol.Required(CONF_P_GIVEN_T): vol.Coerce(float), - vol.Required(CONF_P_GIVEN_F): vol.Coerce(float), + vol.Optional(CONF_P_GIVEN_F): vol.Coerce(float), }, required=True, ) @@ -83,7 +84,7 @@ TEMPLATE_SCHEMA = vol.Schema( CONF_PLATFORM: CONF_TEMPLATE, vol.Required(CONF_VALUE_TEMPLATE): cv.template, vol.Required(CONF_P_GIVEN_T): vol.Coerce(float), - vol.Required(CONF_P_GIVEN_F): vol.Coerce(float), + vol.Optional(CONF_P_GIVEN_F): vol.Coerce(float), }, required=True, ) @@ -128,6 +129,16 @@ async def async_setup_platform( probability_threshold = config[CONF_PROBABILITY_THRESHOLD] device_class = config.get(CONF_DEVICE_CLASS) + # Should deprecate in some future version (2022.10 at time of writing) & make prob_given_false required in schemas. + broken_observations: list[dict[str, Any]] = [] + for observation in observations: + if CONF_P_GIVEN_F not in observation: + text: str = f"{name}/{observation.get(CONF_ENTITY_ID,'')}{observation.get(CONF_VALUE_TEMPLATE,'')}" + raise_no_prob_given_false(hass, observation, text) + _LOGGER.error("Missing prob_given_false YAML entry for %s", text) + broken_observations.append(observation) + observations = [x for x in observations if x not in broken_observations] + async_add_entities( [ BayesianBinarySensor( diff --git a/homeassistant/components/bayesian/repairs.py b/homeassistant/components/bayesian/repairs.py index a1391f8c550..a1d4f142527 100644 --- a/homeassistant/components/bayesian/repairs.py +++ b/homeassistant/components/bayesian/repairs.py @@ -31,9 +31,24 @@ def raise_mirrored_entries(hass: HomeAssistant, observations, text: str = "") -> "mirrored_entry/" + text, breaks_in_ha_version="2022.10.0", is_fixable=False, - is_persistent=False, severity=issue_registry.IssueSeverity.WARNING, translation_key="manual_migration", translation_placeholders={"entity": text}, learn_more_url="https://github.com/home-assistant/core/pull/67631", ) + + +# Should deprecate in some future version (2022.10 at time of writing) & make prob_given_false required in schemas. +def raise_no_prob_given_false(hass: HomeAssistant, observation, text: str) -> None: + """In previous 2022.9 and earlier, prob_given_false was optional and had a default version.""" + issue_registry.async_create_issue( + hass, + DOMAIN, + f"no_prob_given_false/{text}", + breaks_in_ha_version="2022.10.0", + is_fixable=False, + severity=issue_registry.IssueSeverity.ERROR, + translation_key="no_prob_given_false", + translation_placeholders={"entity": text}, + learn_more_url="https://github.com/home-assistant/core/pull/67631", + ) diff --git a/homeassistant/components/bayesian/strings.json b/homeassistant/components/bayesian/strings.json new file mode 100644 index 00000000000..338795624cd --- /dev/null +++ b/homeassistant/components/bayesian/strings.json @@ -0,0 +1,12 @@ +{ + "issues": { + "manual_migration": { + "description": "The Bayesian integration now also updates the probability if the observed `to_state`, `above`, `below`, or `value_template` evaluates to `False` rather than only `True`. So it is no longer required to have duplicate, complementary entries for each binary state. Please remove the mirrored entry for `{entity}`.", + "title": "Manual YAML fix required for Bayesian" + }, + "no_prob_given_false": { + "description": "In the Bayesian integration `prob_given_false` is now a required configuration variable as there was no mathematical rationale for the previous default value. Please add this to your `configuration.yml` for `bayesian/{entity}`. These observations will be ignored until you do.", + "title": "Manual YAML addition required for Bayesian" + } + } +} diff --git a/homeassistant/components/bayesian/translations/en.json b/homeassistant/components/bayesian/translations/en.json index ae9e5645f73..f95e153d986 100644 --- a/homeassistant/components/bayesian/translations/en.json +++ b/homeassistant/components/bayesian/translations/en.json @@ -3,6 +3,10 @@ "manual_migration": { "description": "The Bayesian integration now also updates the probability if the observed `to_state`, `above`, `below`, or `value_template` evaluates to `False` rather than only `True`. So it is no longer required to have duplicate, complementary entries for each binary state. Please remove the mirrored entry for `{entity}`.", "title": "Manual YAML fix required for Bayesian" + }, + "no_prob_given_false": { + "description": "In the Bayesian integration `prob_given_false` is now a required configuration variable as there was no mathematical rationale for the previous default value. Please add this to your `configuration.yml` for `bayesian/{entity}`. These observations will be ignored until you do.", + "title": "Manual YAML addition required for Bayesian" } } } diff --git a/tests/components/bayesian/test_binary_sensor.py b/tests/components/bayesian/test_binary_sensor.py index 0344e2b9445..e16033c66a2 100644 --- a/tests/components/bayesian/test_binary_sensor.py +++ b/tests/components/bayesian/test_binary_sensor.py @@ -587,6 +587,50 @@ async def test_mirrored_observations(hass): ) +async def test_missing_prob_given_false(hass): + """Test whether missing prob_given_false are detected and appropriate issues are created.""" + + config = { + "binary_sensor": { + "platform": "bayesian", + "name": "missingpgf", + "observations": [ + { + "platform": "state", + "entity_id": "binary_sensor.test_monitored", + "to_state": "on", + "prob_given_true": 0.8, + }, + { + "platform": "template", + "value_template": "{{states('sensor.test_monitored2') == 'off'}}", + "prob_given_true": 0.79, + }, + { + "platform": "numeric_state", + "entity_id": "sensor.test_monitored1", + "above": 5, + "prob_given_true": 0.7, + }, + ], + "prior": 0.1, + } + } + assert len(async_get(hass).issues) == 0 + assert await async_setup_component(hass, "binary_sensor", config) + await hass.async_block_till_done() + hass.states.async_set("sensor.test_monitored2", "on") + await hass.async_block_till_done() + + assert len(async_get(hass).issues) == 3 + assert ( + async_get(hass).issues[ + ("bayesian", "no_prob_given_false/missingpgf/sensor.test_monitored1") + ] + is not None + ) + + async def test_probability_updates(hass): """Test probability update function.""" prob_given_true = [0.3, 0.6, 0.8]