Yahoo Weather update, supports forecast for more days by fanthos · Pull Request #8626 · home-assistant/core

fanthos · 2017-07-24T10:39:48Z

Description:

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#3047

Works perfectly with home-assistant/frontend#352 updated weather panel.

Example entry for `configuration.yaml` (if applicable):

weather:
  - platform: yweather
    name: weather_today

Breaking change
forecast is now every time present and the options was removed.

Checklist:

Documentation added/updated in home-assistant.github.io
Local tests with tox run successfully. Your PR cannot be merged unless tests pass

Tested on my device with hass --debug.

houndci-bot · 2017-07-24T10:39:55Z

-            ATTR_FORECAST_TEMP:
-                self._data.yahoo.Forecast[self._forecast]['high'],
-        }]
+        return [{'datetime':v['date'], ATTR_FORECAST_TEMP:int(v['high']), 'templow': int(v['low']),


missing whitespace after ':'
line too long (99 > 79 characters)

houndci-bot · 2017-07-24T10:39:55Z

+
    add_devices([YahooWeatherWeather(yahoo_api, name, forecast)], True)

+def _windtostring(degree):


expected 2 blank lines, found 1

houndci-bot · 2017-07-24T10:39:56Z

 ATTRIBUTION = "Weather details provided by Yahoo! Inc."

+ATTR_FORECAST_DATE = 'datetime'
+ATTR_FORECAST_TEMP = 'temperature'


redefinition of unused 'ATTR_FORECAST_TEMP' from line 13

houndci-bot · 2017-07-24T10:48:48Z

-                self._data.yahoo.Forecast[self._forecast]['high'],
-        }]
+        return [{ATTR_FORECAST_TIME: v['date'], ATTR_FORECAST_TEMP:int(v['high']), ATTR_FORECAST_TEMP_LOW: int(v['low']),
+                 ATTR_FORECAST_CONDITION: CONDITION_CLASSES_LIST[int(v['code'])]}


line too long (81 > 79 characters)

houndci-bot · 2017-07-24T10:48:48Z

-            ATTR_FORECAST_TEMP:
-                self._data.yahoo.Forecast[self._forecast]['high'],
-        }]
+        return [{ATTR_FORECAST_TIME: v['date'], ATTR_FORECAST_TEMP:int(v['high']), ATTR_FORECAST_TEMP_LOW: int(v['low']),


line too long (121 > 79 characters)

houndci-bot · 2017-07-24T10:48:48Z

 ATTRIBUTION = "Weather details provided by Yahoo! Inc."

-CONF_FORECAST = 'forecast'
+ATTR_FORECAST_TIME = 'datetime'


redefinition of unused 'ATTR_FORECAST_TIME' from line 13

houndci-bot · 2017-07-24T10:48:48Z

 from homeassistant.components.weather import (
-    WeatherEntity, PLATFORM_SCHEMA, ATTR_FORECAST_TEMP)
-from homeassistant.const import (TEMP_CELSIUS, CONF_NAME, STATE_UNKNOWN)
+    WeatherEntity, PLATFORM_SCHEMA, ATTR_FORECAST, ATTR_FORECAST_TEMP, ATTR_FORECAST_TIME)


line too long (90 > 79 characters)

pvizeli

Remove UI change from this PR

pvizeli · 2017-07-24T12:14:00Z

-    vol.Optional(CONF_FORECAST, default=0):
-        vol.All(vol.Coerce(int), vol.Range(min=0, max=5)),
+    vol.Optional(CONF_FORECAST, default=5):
+    vol.All(vol.Coerce(int), vol.Range(min=0, max=5)),


Revert this change.

pvizeli · 2017-07-24T12:18:16Z

-                    int(self._data.yahoo.Now['code']) in v][0]
-        except IndexError:
-            return STATE_UNKNOWN
+        return CONDITION_CLASSES_LIST[int(self._data.yahoo.Now['code'])]


You should catch ValueError and IndexError

pvizeli · 2017-07-24T12:22:57Z


+    for cond, condlst in CONDITION_CLASSES.items():
+        for condi in condlst:
+            CONDITION_CLASSES_LIST[condi] = cond


We do not allow to use globals. You need do that inside data with a membervariable

CONDITION_CLASSES_LIST should be a const, so I think I forget to move the init code into globle scope.

CONDITION_CLASSES_LIST = [str(x) for x in range(0, 50)] for cond, condlst in CONDITION_CLASSES.items(): for condi in condlst: CONDITION_CLASSES_LIST[condi] = cond

houndci-bot · 2017-07-24T13:01:02Z

-            ATTR_FORECAST_TEMP:
-                self._data.yahoo.Forecast[self._forecast]['high'],
-        }]
+        if self._forecast == None:


comparison to None should be 'if cond is None:'

This reverts commit 28b4972. revert unintentional submodule change

fix typo, add try catch to another int()

fanthos · 2017-07-24T17:12:48Z

@pvizeli
everything should fixed

fabaff · 2017-07-24T19:22:59Z

    def forecast(self):
        """Return the forecast array."""
+        if not self._forecast:
+            return []


Not needed...CONF_FORECAST will never be None as it defaults to 5 if not provided.

fabaff · 2017-07-25T06:02:34Z

        return self._data.yahoo.Wind['speed']

+    @property
+    def wind_bearing(self):


Bearing not speed.

fabaff · 2017-07-25T06:03:41Z

-            return STATE_UNKNOWN
+            return CONDITION_CLASSES_LIST[int(self._data.yahoo.Now['code'])]
+        except (ValueError, IndexError):
+            return ''


Wouldn't it be better to return STATE_UNKNOWN instead of an empty string?

fanthos · 2017-07-25T06:28:45Z

Done
@fabaff

myusuf3

This looks good to me now. nice work @fanthos

pvizeli · 2017-07-26T07:12:49Z

    vol.Optional(CONF_NAME, default=DEFAULT_NAME): cv.string,
-    vol.Optional(CONF_FORECAST, default=0):
-        vol.All(vol.Coerce(int), vol.Range(min=0, max=5)),
+    vol.Optional(CONF_FORECAST, default=5):


Don't make a breaking change. I see no reason to change this now.

The forecast in yaml works different from the old one, so I think maybe its better to change forecast to something like forecast_days.

pvizeli · 2017-07-26T07:13:42Z

    'exceptional': [0, 1, 2, 3, 4, 25, 36],
 }

+_CONDITION_CLASSES_MAX = 50


Remove this

houndci-bot · 2017-07-26T13:54:40Z

-            return [k for k, v in CONDITION_CLASSES.items() if
-                    int(self._data.yahoo.Now['code']) in v][0]
-        except IndexError:
+            return hass.data[DATA_CONDITION][int(self._data.yahoo.Now['code'])]


undefined name 'hass'

houndci-bot · 2017-07-26T13:54:40Z

        _LOGGER.error("Yahoo! only support %d days forecast",
                      len(yahoo_api.yahoo.Forecast))
        return False
+


blank line contains whitespace

fanthos · 2017-07-26T14:03:51Z

Thanks for the push.
When I am trying the Yahoo API on my computer, I received forecast data for 10 days.
https://developer.yahoo.com/weather/

pvizeli · 2017-07-26T14:06:53Z

Anyway. We had the data present, so we can show the data to UI :) It cost not more if we don't use it.

addressed

fanthos · 2017-07-26T14:12:05Z

I think some text like weather condiction, and some other common fields should goes to weather platform code.

pvizeli · 2017-07-26T14:46:37Z

👍

rpitera · 2017-08-03T20:19:01Z

Since the update, I am seeing some unusually high temperatures in my area.

All the other data are correct, but I'm pretty sure it's not 185 degrees out, despite feeling like it.

Issue opened: #8758

…#8626) * work on weather panel * update yahooweather with more forecast details * Update yweather to allow user input forecast date * fix for houndci * fix long line * fix1 * Revert "work on weather panel" This reverts commit 28b4972. revert unintentional submodule change * fix2 fix typo, add try catch to another int() * fix pylint * fix3 * fix4 * Update yweather.py * Update yweather.py * Remove global data construct * Yahoo API support only 5 days forecast * remove forecast * fix lint * fix lint p2 * Update yweather.py

fanthos added 3 commits July 23, 2017 17:18

work on weather panel

28b4972

update yahooweather with more forecast details

4f929ea

Update yweather to allow user input forecast date

9be313b

homeassistant added cla-signed platform: weather.yweather labels Jul 24, 2017

balloobbot added the platform label Jul 24, 2017

houndci-bot reviewed Jul 24, 2017

View reviewed changes

fix for houndci

3eae6dd

homeassistant added the cla-signed label Jul 24, 2017

houndci-bot reviewed Jul 24, 2017

View reviewed changes

fix long line

bda23f7

fanthos mentioned this pull request Jul 24, 2017

Update docs for yahoo weather home-assistant/home-assistant.io#3047

Merged

pvizeli requested changes Jul 24, 2017

View reviewed changes

fix1

b945231

houndci-bot reviewed Jul 24, 2017

View reviewed changes

fanthos added 3 commits July 24, 2017 21:16

Revert "work on weather panel"

c93fa08

This reverts commit 28b4972. revert unintentional submodule change

fix2

ce00d78

fix typo, add try catch to another int()

fix pylint

7bdc2b6

fabaff previously requested changes Jul 25, 2017

View reviewed changes

fix3

0e1df57

homeassistant added the cla-signed label Jul 25, 2017

myusuf3 approved these changes Jul 26, 2017

View reviewed changes

pvizeli requested changes Jul 26, 2017

View reviewed changes

fix4

dd16584

homeassistant added the cla-signed label Jul 26, 2017

pvizeli added 2 commits July 26, 2017 15:24

Update yweather.py

bc7b179

Update yweather.py

9ec5d9c

houndci-bot reviewed Jul 26, 2017

View reviewed changes

pvizeli added 2 commits July 26, 2017 15:57

Yahoo API support only 5 days forecast

72bbde7

remove forecast

3733b3e

home-assistant deleted a comment from houndci-bot Jul 26, 2017

fix lint

719681e

homeassistant added the cla-signed label Jul 26, 2017

home-assistant deleted a comment from houndci-bot Jul 26, 2017

fix lint p2

3d0b569

homeassistant added the cla-signed label Jul 26, 2017

pvizeli approved these changes Jul 26, 2017

View reviewed changes

pvizeli added the breaking-change label Jul 26, 2017

home-assistant deleted a comment from houndci-bot Jul 26, 2017

Update yweather.py

1366721

homeassistant added the cla-signed label Jul 26, 2017

pvizeli merged commit abcfcdd into home-assistant:dev Jul 26, 2017

fanthos mentioned this pull request Jul 26, 2017

Yahoo Weather: Update docs after PR changed. home-assistant/home-assistant.io#3060

Merged

balloob mentioned this pull request Jul 29, 2017

0.50 #8685

Merged

home-assistant locked and limited conversation to collaborators Dec 11, 2017

fanthos deleted the weatherpanel branch April 12, 2018 07:11

ghost added integration: yweather and removed platform: weather.yweather labels Mar 21, 2019


		add_devices([YahooWeatherWeather(yahoo_api, name, forecast)], True)

		def _windtostring(degree):

Uh oh!

Conversation

fanthos commented Jul 24, 2017 • edited by pvizeli Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanthos Jul 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanthos commented Jul 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanthos commented Jul 25, 2017

Uh oh!

myusuf3 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvizeli Jul 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fanthos commented Jul 26, 2017

Uh oh!

pvizeli commented Jul 26, 2017

Uh oh!

fanthos commented Jul 26, 2017

Uh oh!

pvizeli commented Jul 26, 2017

Uh oh!

rpitera commented Aug 3, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

fanthos commented Jul 24, 2017 •

edited by pvizeli

Loading

Example entry for `configuration.yaml` (if applicable):

pvizeli left a comment •

edited

Loading

fanthos Jul 24, 2017 •

edited

Loading

myusuf3 left a comment •

edited

Loading

pvizeli Jul 26, 2017 •

edited

Loading