Skip to content
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

Weather template validation is too strict and/or NWS integration returns bad forecasts #114799

Open
mdonoughe opened this issue Apr 4, 2024 · 33 comments · May be fixed by #117254 or #116826
Open

Weather template validation is too strict and/or NWS integration returns bad forecasts #114799

mdonoughe opened this issue Apr 4, 2024 · 33 comments · May be fixed by #117254 or #116826

Comments

@mdonoughe
Copy link
Contributor

The problem

I have a local weather station so I use a weather template to combine NWS forecasts with my local data. After upgrading to 2024.04, the (still documented) forecast_template property was forbidden to be set so I had no weather.

I figured out how to make a template sensor that saves the result of making the weather.get_forecasts service (why can't the template just delegate it's service handler to a different service?), but my forecasts weren't showing up even though I could copy the template into the template editor and see the expected result.

template.weather validates the contents of the forecast, and it discards your forecast if it contains any keys that aren't on a whitelist:

@callback
def _validate_forecast(
self,
forecast_type: Literal["daily", "hourly", "twice_daily"],
result: Any,
) -> list[Forecast] | None:
"""Validate the forecasts."""
if result is None:
return None
if not isinstance(result, list):
raise vol.Invalid(
"Forecasts is not a list, see Weather documentation https://www.home-assistant.io/integrations/weather/"
)
for forecast in result:
if not isinstance(forecast, dict):
raise vol.Invalid(
"Forecast in list is not a dict, see Weather documentation https://www.home-assistant.io/integrations/weather/"
)
diff_result = set().union(forecast.keys()).difference(CHECK_FORECAST_KEYS)
if diff_result:
raise vol.Invalid(
f"Only valid keys in Forecast are allowed, unallowed keys: ({diff_result}), "
"see Weather documentation https://www.home-assistant.io/integrations/weather/"
)
if forecast_type == "twice_daily" and "is_daytime" not in forecast:
raise vol.Invalid(
"`is_daytime` is missing in twice_daily forecast, see Weather documentation https://www.home-assistant.io/integrations/weather/"
)
if "datetime" not in forecast:
raise vol.Invalid(
"`datetime` is required in forecasts, see Weather documentation https://www.home-assistant.io/integrations/weather/"
)
continue
return result

The NWS integration adds its own key detailed_description which isn't on the whitelist:

ATTR_FORECAST_DETAILED_DESCRIPTION: Final = "detailed_description"

So if you try to use NWS with a template, you get no forecasts unless you do something like this:

forecast_twice_daily_template: |-
  {% set forecast = state_attr('sensor.weather_forecast_twice_daily', 'forecast') %}
  {% set clean = namespace(forecast=[]) %}
  {% for f in forecast %}
  {% set clean.forecast = clean.forecast + [dict(f | items | selectattr('0', 'ne', 'detailed_description'))] %}
  {% endfor %}
  {{ clean.forecast }}
forecast_hourly_template: |-
  {% set forecast = state_attr('sensor.weather_forecast_hourly', 'forecast') %}
  {% set clean = namespace(forecast=[]) %}
  {% for f in forecast %}
  {% set clean.forecast = clean.forecast + [dict(f | items | selectattr('0', 'ne', 'detailed_description'))] %}
  {% endfor %}
  {{ clean.forecast }}

All together, this turns what used to be a simple 2 line delegation from the template to the NWS weather entity into 48 lines of YAML and a lot of time spent troubleshooting.

What version of Home Assistant Core has the issue?

core-2024.4.0

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Core

Integration causing the issue

nws

Link to integration documentation on our website

https://www.home-assistant.io/integrations/nws/

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

Error validating template result '[{'detailed_description': '', 'datetime': '2024-04-03T22:00:00-04:00',…}]' from template 'Template<template=({{ state_attr('sensor.weather_forecast_hourly', 'forecast') }}) renders=6>' for attribute '_forecast_hourly' in entity weather.local validation message 'Only valid keys in Forecast are allowed, unallowed keys: ({'detailed_description'}), see Weather documentation https://www.home-assistant.io/integrations/weather/'

Additional information

No response

@home-assistant
Copy link

home-assistant bot commented Apr 4, 2024

Hey there @home-assistant/core, mind taking a look at this issue as it has been labeled with an integration (weather) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of weather can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign weather Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


weather documentation
weather source
(message by IssueLinks)

@home-assistant
Copy link

home-assistant bot commented Apr 4, 2024

Hey there @MatthewFlamm, @kamiyo, mind taking a look at this issue as it has been labeled with an integration (nws) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of nws can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign nws Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


nws documentation
nws source
(message by IssueLinks)

@home-assistant
Copy link

home-assistant bot commented Apr 4, 2024

Hey there @PhracturedBlue, @tetienne, @home-assistant/core, mind taking a look at this issue as it has been labeled with an integration (template) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of template can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign template Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


template documentation
template source
(message by IssueLinks)

@austinbeam
Copy link

Just to pile on a bit here and to the point @mdonoughe makes, this is a rather frustrating error message considering the forecast_template is still in the documentation.

I'm not sure the best way to resolve this on my side considering the weather forecast change, I have the same use case where I'm merging my local WS data with an external forecast and this has worked seamlessly for years.

Logger: homeassistant.config
Source: config.py:1324
First occurred: 10:14:51 PM (1 occurrences)
Last logged: 10:14:51 PM

Invalid config for 'weather' from integration 'template' at configuration.yaml, line 152: 'forecast_template' is an invalid option for 'template.weather', check: forecast_template, please check the docs at https://www.home-assistant.io/integrations/template

@GSzabados
Copy link

The documentation with the forecast option is out of date. And I am with @mdonoughe, this whole issue should have been dealt with months ago.

The use case is fairly valid, as I do the same, but I did not try to update until 2024.4 came out and the previous one has failed.

I use a Ecowitt weather station with Tomorrow.io's forecast.

@ausfas
Copy link

ausfas commented Apr 4, 2024

I'm getting this error

Source: config.py:1324
First occurred: 8:54:01 PM (1 occurrences)
Last logged: 8:54:01 PM

Invalid config for 'weather' from integration 'template' at packages/integrations/weather.yaml, line 7: 'forecast_template' is an invalid option for 'template.weather', check: forecast_template, please check the docs at https://www.home-assistant.io/integrations/template

For this code

weather:
  - platform: template
    name: "Forecast Home"
    condition_template: "{{ states('weather.home') }}"
    temperature_template: "{{ state_attr('weather.home', 'temperature') | round(0, default=-99) }}"
    humidity_template: "{{ state_attr('weather.home', 'humidity') | round(0, default=-99) }}"
    forecast_template: >
      {% set ns = namespace(z = []) %}
      {% for x in state_attr('weather.home', 'forecast') %}
        {% set ns.z = ns.z +
          [{
          'condition': x.condition,
          'precipitation': x.precipitation,
          'temperature': x.temperature | round(0, default=-99),
          'templow': x.templow | round(0, default=-99),
          'datetime': x.datetime,
          'wind_bearing': x.wind_bearing,
          'wind_speed': x.wind_speed
          }] %}
       {% endfor %}
       {{ ns.z }}

@GSzabados
Copy link

forecast_template

You need to use forecast_daily_template or forecast_hourly_template

@zubir2k

This comment was marked as off-topic.

@lnxsrt

This comment was marked as off-topic.

@Ubuntubirdy

This comment was marked as off-topic.

@mdonoughe

This comment was marked as off-topic.

@lnxsrt
Copy link

lnxsrt commented Apr 6, 2024

Same issue here with my Template Weather Provider.
Having the forecast attribute was also helpful for esphome.io weather display devices. I don't think the service call option in esphome.io can even deal with return data (ie the forecast). So now I will have to hack together some sort of text template sensor in homeassistant that I can get the state of in the epshome.io device.
Feels like a much worse option than just leaving the forecast attribute on weather entities.

That's a different issue. This issue is about not being able to configure the template weather provider because it rejects the forecasts created by other weather providers.

Gotcha, I assumed the reason you had to create template sensors was to try to feed the Template Weather Provider since the forecast attribute was eliminated. Sorry if that wasn't the case.

Also the docs for Template Weather Provider still show that you can use the forecast attribute.

forecast_daily_template: "{{ state_attr('weather.my_region', 'forecast') }}"

@FrnchFrgg

This comment was marked as off-topic.

@GSzabados

This comment was marked as off-topic.

@rbflurry

This comment was marked as off-topic.

@matt-laird

This comment was marked as off-topic.

@mib1185

This comment was marked as off-topic.

@GSzabados

This comment was marked as off-topic.

@gcormier

This comment was marked as off-topic.

@mdonoughe
Copy link
Contributor Author

This ticket is about the NWS integration being incompatible with template weather. Adding "me too"s about the change from the forecast attribute to the forecast service on this ticket is not helpful.

@badewanne1234
Copy link
Contributor

This ticket is about the NWS integration being incompatible with template weather. Adding "me too"s about the change from the forecast attribute to the forecast service on this ticket is not helpful.

The chances of getting this fixed are bigger if you dont focus solely on the NWS Weatger integration - other integrations most likely face the same problem. The more affected people, the more likely it gets fixed.

@FrnchFrgg
Copy link
Contributor

The chances of getting this fixed are bigger if you dont focus solely on the NWS Weatger integration - other integrations most likely face the same problem. The more affected people, the more likely it gets fixed.

The bug is indeed not about the NWS integration specifically (though it may be a bad idea to introduce non-standard elements in a standard attribute).

But this bug is not about the removal of the forecast attribute from every weather integration. OP specifically uses the as-of-now simplest method (1) to import results of get_forecasts() into the *_forecast_template slots of template weather, but even that is failing the content check of the template results. OP thus argues that the check is over-zealous and should be relaxed.

(1) current method is a) create an auxiliary sensor template with an action to call the get_forecast service b) in *_forecast_template, replace previous uses of weather entities' forecast attributes by the auxiliary sensor state.

The totally valid fact that this current method/workaround is cumbersome is not part of this bug, as @mdonoughe pointed out. Focusing this issue back to its initial bug is a good thing because even if a better way for (1) is devised, the OP's problem will probably remain. Note that @mdonoughe is actually the OP.

Sidenote: I was also among those who chimed in about the wrong bug here, so no hard feelings.

@gjohansson-ST
Copy link
Member

So there is two problems here

  1. Template still has forecast_template. This was unfortunately forgotten so it will be removed shortly, use the others for daily, hourly etc.
  2. NWS expands the forecast to include non-valid forecast items. I will look at it and possibly remove them (weather integrations should only use the approved items in the forecast dict and not expand it).

Thanks

@GSzabados

This comment has been minimized.

@TheLordVader

This comment has been minimized.

@mdonoughe
Copy link
Contributor Author

Same problem with SMHI integration. I have used my weather station with a weather template to combine SMHI forecasts with my local data.

That is not the same problem. This issue is about the "Only valid keys in Forecast are allowed" error.

@kapplejacks

This comment has been minimized.

@aredon
Copy link

aredon commented Apr 12, 2024

  1. NWS expands the forecast to include non-valid forecast items. I will look at it and possibly remove them (weather integrations should only use the approved items in the forecast dict and not expand it).

For what it's worth the detailed forecast provided by NWS makes an excellent drop into a TTS message. I'd rather it not be removed.

I'm having a possibly related issue in that my NWS weather keeps returning Unknown on only the weather location that is used in few templates. Unsure if I need to open another bug report as nothing else is in the logs beyond the templating error.

@kapplejacks
Copy link

@aredon are you able to open a new bug report? I agree that this is and issue and should be resolved. The daily weather forecast is essential in voice assistant configurations.

@aredon
Copy link

aredon commented Apr 17, 2024

@aredon are you able to open a new bug report? I agree that this is and issue and should be resolved. The daily weather forecast is essential in voice assistant configurations.

I opened one but it was determined that it was an NWS outage. Really not sure how long it's going to be broken.

@kamiyo kamiyo linked a pull request May 5, 2024 that will close this issue
20 tasks
@MatthewFlamm
Copy link
Contributor

The major NWS integration forecast updating issue should now be fixed. This issue should continue to be focused on the template validation and/or NWS forecasts structure issue.

While there is a suggested path forward for NWS to add a custom forecast service, it will require a lot of codeowner effort, i.e. a lot of time before it can be implemented, reviewed, and merged in. This detailed_description attribute has been in this integration since v0.99.0 (almost 5 years now) and by comments here, it is actively used. I am one of those users too. I say this to hopefully avoid removing it quickly while we try to work on a better solution.

@aredon
Copy link

aredon commented May 6, 2024

The major NWS integration forecast updating issue should now be fixed. This issue should continue to be focused on the template validation and/or NWS forecasts structure issue.

While there is a suggested path forward for NWS to add a custom forecast service, it will require a lot of codeowner effort, i.e. a lot of time before it can be implemented, reviewed, and merged in. This detailed_description attribute has been in this integration since v0.99.0 (almost 5 years now) and by comments here, it is actively used. I am one of those users too. I say this to hopefully avoid removing it quickly while we try to work on a better solution.

It looks to have been removed as far as I can tell. All my automations that relied on it have not been working and I no longer see it in the attributes. I'm hopeful it can be included in some other way.

@MatthewFlamm
Copy link
Contributor

It looks to have been removed as far as I can tell. All my automations that relied on it have not been working and I no longer see it in the attributes. I'm hopeful it can be included in some other way.

As mentioned up thread in multiple spots, the forecast is now only available via service call, this was not a change in NWS, but rather a homeassistant wide change. This issue is specifically about matching the template validation with an extra key that NWS supplies in the forecast. Let's keep it on topic.

@MatthewFlamm MatthewFlamm linked a pull request May 11, 2024 that will close this issue
20 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet