Add forecast service call for extra attributes for nws #32729
Conversation
home-assistant
bot
added
has-parent
✅ Deploy Preview for home-assistant-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
I've always thought the requirements for a platform using heterogeneous APIs should be a common baseline and not a limit. This is in the direction of goodness. |
source/_integrations/nws.markdown
Outdated
| | `datetime` | The time of the forecasted conditions. | 2023-02-17T14:00:00+00:00 | ||
| | `is_daytime` | Only set for `twice_daily` forecasts. | True | ||
| | `detailed_description` | Only set for `twice_daily` forecasts. | 50% Chance of rain, otherwise partly cloudy with a high of 75F. | ||
| | `dewpoint` | Dewpoint in F | 52 |
There was a problem hiding this comment.
Please align with the resulting core PR 👍
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
WalkthroughWalkthroughThe changes introduce a new action Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Additional context usedLanguageTool
Markdownlint
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
I updated based on the changes in the core PR, also I needed to make some changes due to a recent change in language from service->action. Hopefully I captured that change correctly here. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
source/_integrations/nws.markdown (1)
29-34: Consider adding a link to the NWS API documentation fornws.get_forecasts_extra.Providing a direct link to the API documentation can help users understand the new action better.
+ For more details, see the [NWS API documentation](https://www.weather.gov/documentation/services-web-api).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
source/_integrations/nws.markdown (1)
31-33: Clarify the relationship betweennws.get_forecasts_extraandweather.get_forecasts.The documentation mentions that
nws.get_forecasts_extraprovides extra data similar toweather.get_forecasts. It would be helpful to clarify ifnws.get_forecasts_extrais an extension or a replacement ofweather.get_forecasts.- `nws.get_forecasts_extra` provides extra data in a form similar to `weather.get_forecasts`. + `nws.get_forecasts_extra` provides additional forecast data that complements the data from `weather.get_forecasts`.
| The response data field is a mapping of called target entities, each containing the `forecast` field. | ||
| `forecast` is a list of forecasted conditions at a given point in time that are not returned from `weather.get_forecasts`. The `datetime` and `is_daytime` attributes are still provided for context. |
There was a problem hiding this comment.
Fix redundancy in the description.
The phrase "at a given point in time" is redundant.
- `forecast` is a list of forecasted conditions at a given point in time that are not returned from `weather.get_forecasts`.
+ `forecast` is a list of forecasted conditions at a point in time that are not returned from `weather.get_forecasts`.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The response data field is a mapping of called target entities, each containing the `forecast` field. | |
| `forecast` is a list of forecasted conditions at a given point in time that are not returned from `weather.get_forecasts`. The `datetime` and `is_daytime` attributes are still provided for context. | |
| The response data field is a mapping of called target entities, each containing the `forecast` field. | |
| `forecast` is a list of forecasted conditions at a point in time that are not returned from `weather.get_forecasts`. The `datetime` and `is_daytime` attributes are still provided for context. |
Tools
LanguageTool
[style] ~50-~50: This phrase is redundant. Consider writing “point” or “time”.
Context: ...ist of forecasted conditions at a given point in time that are not returned from `weather.get...(MOMENT_IN_TIME)
There was a problem hiding this comment.
The phrasing "forecast is a list of forecasted conditions at a given point in time" is mirrored from the parent weather.get_forecasts action. This language should be kept consistent.
There was a problem hiding this comment.
@MatthewFlamm Thank you for pointing out the importance of maintaining consistency with the weather.get_forecasts documentation. I agree that we should keep the phrasing "at a given point in time" to ensure uniformity across the documentation.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the `weather.get_forecasts` action documentation to verify the phrasing.
rg "weather.get_forecasts" -A 10
Length of output: 9388
There was a problem hiding this comment.
If the weather.get_forecasts documentation language is changed, we should change here in the future.
There was a problem hiding this comment.
@MatthewFlamm Thank you for the clarification. I agree that if the weather.get_forecasts documentation language is updated in the future, we should also update the nws.get_forecasts_extra documentation to maintain consistency.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
| | Response data | Description | Example | | ||
| | ---------------------- | ----------- | -------- | | ||
| | `datetime` | The time of the forecasted conditions. | 2023-02-17T14:00:00+00:00 | | ||
| | `is_daytime` | Only set for `twice_daily` forecasts. | True | | ||
| | `detailed_description` | Only set for `twice_daily` forecasts. | 50% Chance of rain, otherwise partly cloudy with a high of 75F. | | ||
| | `short_description` | Short weather condition | Partly Sunny then Slight Chance Showers And Thunderstorms | |
There was a problem hiding this comment.
Fix table formatting and remove redundancy in description.
The table rows are missing trailing pipes, and the description of the forecast field is redundant.
| Response data | Description | Example |
| ---------------------- | ----------- | -------- |
| `datetime` | The time of the forecasted conditions. | 2023-02-17T14:00:00+00:00 |
| `is_daytime` | Only set for `twice_daily` forecasts. | True |
| `detailed_description` | Only set for `twice_daily` forecasts. | 50% Chance of rain, otherwise partly cloudy with a high of 75F. |
| `short_description` | Short weather condition | Partly Sunny then Slight Chance Showers And Thunderstorms |Additionally, update the description to remove redundancy:
-`forecast` is a list of forecasted conditions at a given point in time that are not returned from `weather.get_forecasts`. The `datetime` and `is_daytime` attributes are still provided for context.
+`forecast` is a list of forecasted conditions at a point in time that are not returned from `weather.get_forecasts`. The `datetime` and `is_daytime` attributes are still provided for context.Tools
LanguageTool
[uncategorized] ~57-~57: Possible missing preposition found.
Context: ...ition | Partly Sunny then Slight Chance Showers And Thunderstorms | ## Details Detail...(AI_EN_LECTOR_MISSING_PREPOSITION)
| | Data attribute | Optional | Description | Example | | ||
| | ---------------------- | -------- | ----------- | --------| | ||
| | `type` | no | The type of forecast, must be either `twice_daily` or `hourly`. | `twice_daily` | |
There was a problem hiding this comment.
Fix table formatting and improve attribute description.
The table rows are missing trailing pipes, and the description for the type attribute should specify the forecast types.
| Data attribute | Optional | Description | Example |
| ---------------------- | -------- | ----------- | --------|
| `type` | no | The type of forecast, must be either `twice_daily` or `hourly`. | `twice_daily` |Committable suggestion was skipped due to low confidence.
| ## Action `nws.get_forecasts_extra` | ||
|
|
||
| `nws.get_forecasts_extra` provides extra data in a form similar to `weather.get_forecasts`. See [`weather.get_forecasts` documentation](/integrations/weather/action--weatherget_forecasts). |
There was a problem hiding this comment.
Fix trailing spaces and align terminology.
Remove trailing spaces and align the terminology with the rest of the document.
-## Action `nws.get_forecasts_extra`
+## Action `nws.get_forecasts_extra`Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Action `nws.get_forecasts_extra` | |
| `nws.get_forecasts_extra` provides extra data in a form similar to `weather.get_forecasts`. See [`weather.get_forecasts` documentation](/integrations/weather/action--weatherget_forecasts). | |
| ## Action `nws.get_forecasts_extra` | |
| `nws.get_forecasts_extra` provides extra data in a form similar to `weather.get_forecasts`. See [`weather.get_forecasts` documentation](/integrations/weather/action--weatherget_forecasts). |
Tools
Markdownlint
31-31: Expected: 0 or 2; Actual: 1
Trailing spaces(MD009, no-trailing-spaces)
|
@gjohansson-ST, are you good with the changes? |
|
@c0ffeeca7 I'm good with it 👍 |
home-assistant
bot
removed
the
parent-merged
Proposed change
Documentation for new service to return extra data for forecasts that are not yet supported by weather platform. For home-assistant/core#117254.
Type of change
currentbranch).currentbranch).nextbranch).nextbranch).Additional information
Checklist
currentbranch.nextbranch.Summary by CodeRabbit
New Features
nws.get_forecasts_extrafor more detailed weather forecasts, including attributes liketype,datetime,is_daytime,detailed_description, andshort_description.Documentation
nws.get_forecasts_extraaction and the expanded response data structure, including usage examples.