Add humidity to NWS forecast#95575
Conversation
|
Hey there @MatthewFlamm, @kamiyo, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
|
Thanks! Have you tested this locally? It looks like "relativeHumidity" is a relatively new return from NWS endpoint with a different structure compared to other values in the forecase, i.e. it is a nested dict similar to the observations. This isn't in the testing of pynws currently. |
Should have read your code a little closer before commenting, sorry. It looks like you have handled it the integration code. I wonder if it should instead be built into pynws to return the numerical value in a consistent manner rather than kludging it here? |
|
There are also a bunch of new attributes that could be added: wind gust, apparent temp (I assume this is heat index or wind chill in NWS terms). I see other PRs adding these all in one PR if you prefer, and I'm also fine if you don't want to take that on. |
I created PR MatthewFlamm/pynws#117 to use as a starting point. Remaining tasks are:
|
|
It seems a bit unclear from above comments but is this ready for merging (in case would you approve @MatthewFlamm ) or should we set it as draft waiting for upstream changes? |
|
The decision was made to move the changes upstream, so I moved it to draft status |
|
I concur. |
|
pynws has a new release with better support for the new values, so this PR can be updated accordingly. Thanks to @lymanepp for taking on changes there and here. |
MatthewFlamm
left a comment
There was a problem hiding this comment.
LGTM. Not sure if the failing patch coverage will be required as your change on that line is fairly straightforward. If it is required, then I can help adding the test needed.
|
@gjohansson-ST thanks for fixing the reviewer mess that I inadvertently created |
|
@lymanepp please do a rebase onto the latest dev branch |
MatthewFlamm
left a comment
There was a problem hiding this comment.
Test failure looks unrelated
Would be great if we could add the missing coverage in a separate PR. |
* Add humidity to NWS forecast to address home-assistant#95572 * Use pynws 1.5.0 enhancements for probabilityOfPrecipitation, dewpoint, and relativeHumidity. * Update requirements to match pynws version * test for clear night * update docstring --------- Co-authored-by: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com>
Proposed change
HA Core recently added support for including humidity in weather forecasts. Add humidity to NWS forecast to address #95572
Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.To help with the load of incoming pull requests: