Update docstrings #7374

fabaff · 2017-04-30T08:08:12Z

Description:

Preparation for #7357.

Checklist:

If the code communicates with devices, web services, or third-party tools:

Local tests with tox run successfully. Your PR cannot be merged unless tests pass

mention-bot · 2017-04-30T08:08:13Z

@fabaff, thanks for your PR! By analyzing the history of the files in this pull request, we identified @MartinHjelmare, @rytilahti and @olimpiurob to be potential reviewers.

houndci-bot · 2017-04-30T09:38:23Z

homeassistant/components/media_player/plex.py

        return self._media_album_artist

    @property
    def media_track(self):
-        """Track number of current playing media, music track only."""
+        """Return the track number of current playing media, music track only."""


line too long (81 > 79 characters)

houndci-bot · 2017-04-30T09:38:23Z

homeassistant/components/media_player/onkyo.py

            else:
-                _LOGGER.info('%s is disconnected. Attempting to reconnect.',
+                _LOGGER.info(""%s is disconnected. Attempting to reconnect"",


missing whitespace around modulo operator
SyntaxError: invalid syntax

houndci-bot · 2017-04-30T09:38:23Z

homeassistant/components/media_player/emby.py

@@ -85,20 +85,20 @@ def device_update_callback(data):
                new_devices.append(new)

            elif dev_id in inactive_emby_devices:
-                if emby.devices[dev_id].state != 'Off':
+                if Callbackemby.devices[dev_id].state != 'Off':


undefined name 'Callbackemby'

houndci-bot · 2017-04-30T09:38:24Z

homeassistant/components/media_player/plex.py

        return self._media_album_artist

    @property
    def media_track(self):
-        """Track number of current playing media, music track only."""
+        """Return the track number of current playing media, music track only."""


line too long (81 > 79 characters)

houndci-bot · 2017-04-30T09:38:24Z

homeassistant/components/media_player/onkyo.py

            else:
-                _LOGGER.info('%s is disconnected. Attempting to reconnect.',
+                _LOGGER.info(""%s is disconnected. Attempting to reconnect"",


missing whitespace around modulo operator
SyntaxError: invalid syntax

houndci-bot · 2017-04-30T09:38:24Z

homeassistant/components/media_player/emby.py

@@ -85,20 +85,20 @@ def device_update_callback(data):
                new_devices.append(new)

            elif dev_id in inactive_emby_devices:
-                if emby.devices[dev_id].state != 'Off':
+                if Callbackemby.devices[dev_id].state != 'Off':


undefined name 'Callbackemby'

MartinHjelmare · 2017-04-30T09:41:48Z

I'd like us to decide if we should have double quotes on all log strings or not. It's not part of PEP8 as far as I know but if home assistant makes it its standard we can all follow that. Either we make it standard or we should stop changing it. I've been following whole module consistency, but not whole project consistency. I don't care what we decide, but we need to have a standard to follow.

houndci-bot · 2017-04-30T10:33:53Z

homeassistant/components/mqtt/server.py

@@ -1,4 +1,4 @@
-"""
+ """


IndentationError: unexpected indent
indentation is not a multiple of four
unexpected indentation

houndci-bot · 2017-04-30T12:40:39Z

homeassistant/components/sensor/octoprint.py

-                                                 SENSOR_TYPES[octo_type][1],
-                                                 tool)
+                    new_sensor = OctoPrintSensor(
+                        octoprint.OCTOPRINT, temp_type,temp_type, name,


missing whitespace after ','

houndci-bot · 2017-04-30T12:40:39Z

homeassistant/components/sensor/knx.py

-                                 ILLUMINANCE, SPEED_MS, CONF_MINIMUM,
-                                 CONF_MAXIMUM)
+from homeassistant.const import (
+    TEMP_CELSIUS, TEMPERATURE, CONF_TYPE,ILLUMINANCE, SPEED_MS, CONF_MINIMUM,


missing whitespace after ','

houndci-bot · 2017-04-30T17:56:15Z

homeassistant/components/switch/broadlink.py

-                                 CONF_TYPE)
+from homeassistant.const import (
+    CONF_FRIENDLY_NAME, CONF_SWITCHES, CONF_COMMAND_OFF, CONF_COMMAND_ON,
+    ONF_TIMEOUT, CONF_HOST, CONF_MAC,CONF_TYPE)


missing whitespace after ','

houndci-bot · 2017-04-30T17:56:15Z

homeassistant/components/switch/broadlink.py

-                                 CONF_COMMAND_OFF, CONF_COMMAND_ON,
-                                 CONF_TIMEOUT, CONF_HOST, CONF_MAC,
-                                 CONF_TYPE)
+from homeassistant.const import (


'homeassistant.const.ONF_TIMEOUT' imported but unused

rytilahti · 2017-04-30T20:35:39Z

homeassistant/components/binary_sensor/netatmo.py

+    "Outdoor motion": 'motion',
+    "Outdoor human": 'motion',
+    "Outdoor animal": 'motion',
+    "Outdoor vehicle": 'motion'


Converting only the value part doesn't make much sense, does it?

rytilahti · 2017-04-30T20:38:03Z

homeassistant/components/light/yeelight.py

-        name = "yeelight_%s_%s" % (discovery_info["device_type"],
-                                   discovery_info["properties"]["mac"])
+        name = 'yeelight_{}_{}'.format(
+            discovery_info['device_type'], discovery_info['properties']['mac'])


Imo format string changes should be discussed separately. In the future (with 3.6) it'd be nice to use f-strings directly, however for now I don't see what good does this change do (when not naming the placeholders, that is)?

If we suggest to use string formatting in PR then we should be consistent.

rytilahti · 2017-04-30T20:39:47Z

homeassistant/components/light/yeelight.py

@@ -85,7 +84,7 @@ def setup_platform(hass, config, add_devices, discovery_info=None):
            device = {'name': device_config[CONF_NAME], 'ipaddr': ipaddr}
            lights.append(YeelightLight(device, device_config))

-    add_devices(lights, True)  # true to request an update before adding.
+    add_devices(lights, True)


This is there to remind that add_devices() has a magic boolean flag. It may also be useful for developers looking how others have done adding. In any case I think this kind of cleanups would deserve their own PR & discussion.

That's covered in the docs. No need to have that on a single place in the code.

Very implicitly, but can be fixed with a documentation update. Maybe enforcing using kwarg for booleans as a general policy? Would turn it to add_devices(lights, update_before_adding=True) to make it clear?

rytilahti · 2017-04-30T20:41:31Z

homeassistant/components/media_player/cast.py

@@ -204,7 +204,7 @@ def media_track(self):

    @property
    def media_series_title(self):
-        """The title of the series of current playing media (TV Show only)."""
+        """Return te title of the series of current playing media (TV only)."""


rytilahti · 2017-04-30T20:43:25Z

homeassistant/components/media_player/denon.py

@@ -101,15 +101,15 @@ def _setup_sources(self, telnet):
    @classmethod
    def telnet_request(cls, telnet, command, all_lines=False):
        """Execute `command` and return the response."""
-        _LOGGER.debug('Sending: "%s"', command)
+        _LOGGER.debug("Sending: %s", command)


Here changing from '' to "" instead of other way around? It'd however be nice to keep the outputted command quoted as it was.

rytilahti · 2017-04-30T20:53:51Z

homeassistant/components/sensor/fastdotcom.py

+            track_time_change(
+                hass, self.update, second=config.get(CONF_SECOND),
+                minute=config.get(CONF_MINUTE), hour=config.get(CONF_HOUR),
+                day=config.get(CONF_DAY))


The existing one looks to me much more readable with one line per kwarg.

balloob · 2017-05-01T06:28:23Z

I would prefer if we keep cleanups to the minimum necessary to get the upgrade of pydocstyle or pylint going. By updating other code, we can a) introduce bugs and b) mask author of code in git blame

fabaff · 2017-05-01T07:28:54Z

I would prefer if we keep cleanups to the minimum necessary to get the upgrade of pydocstyle or pylint going.

Me too but as I needed to edit almost every file it was only efficient to update the most obvious and minor things we want have changed in PRs like string formatting and the removal of blank lines or update of comments was a side effect.
People will always have a different opinions about quotes, style, line length, init systems, or what every. To come back to the log messages and the quotes: The most used style won. As it's perhaps not the best/smartest/nicest way to do it but now it's on the way to be consistent across the whole code.

The format of the log messages is a different topic and for another day. It wouldn't harm to have a more uniform style as we know for sure that people are feeding the log to their tools.

fabaff · 2017-05-01T13:28:26Z

Ok, I collected the things which always lead to discussions and create a page with details about the style.

rytilahti · 2017-05-01T14:00:54Z

As I don't know where this would be best discussed, just adding my last comment regarding to the quote styling. Having two types of quotes makes it very hard to keep it consistent on the long run if the content of those variables may live (e.g. are not just constants). If this is to be enforced it would be nice to describe in the style guide why it makes sense to do so.

P.S. about the quotations in log messages, using them makes it (in my opinion) easier, not harder to parse for both people and with regular expressions. A fully structured logging format would be the best but it may be hard to pull consistently.

P.P.S. In the file header example, "For more details about this platform, please refer to the documentation at" crosses the line length limit of 72 for docstrings as defined in PEP8.

Just my 0.02e :-)

MartinHjelmare · 2017-05-01T14:05:21Z

Good with the page. In my opinion mixing single and double quotes and having different style depending on the length of the string, makes for a harder to follow style. I'd prefer it to be clear and simple. Eg either use single quotes in whole module or double quotes in whole module.

balloob · 2017-05-02T15:45:01Z

I agree with @fabaff , thanks for adding the page 👍

homeassistant added the cla-signed label Apr 30, 2017

balloobbot added the platform label Apr 30, 2017

homeassistant added the cla-signed label Apr 30, 2017

houndci-bot reviewed Apr 30, 2017

View reviewed changes

fabaff mentioned this pull request Apr 30, 2017

Upgrade flake8-docstrings to 1.1.0 #7358

Closed

homeassistant added the cla-signed label Apr 30, 2017

houndci-bot reviewed Apr 30, 2017

View reviewed changes

homeassistant added the cla-signed label Apr 30, 2017

rytilahti reviewed Apr 30, 2017

View reviewed changes

homeassistant added cla-signed labels Apr 30, 2017

rytilahti reviewed Apr 30, 2017

View reviewed changes

fabaff mentioned this pull request May 1, 2017

Updated docstrings #7383

Merged

fabaff added 6 commits May 1, 2017 09:51

Update docstrings

b8de5c6

Update docstrings

81520e0

Update docstrings

69c16a2

Update docstrings

e1baa8c

Update docstrings

ce6c455

Update docstrings

f9c231b

fabaff added 5 commits May 1, 2017 09:52

Update docstring

02fc81d

Update docstrings

2d87187

Update docstrings

7a702bd

Fix lint issues

3f799f9

Update docstrings

d2e8e96

homeassistant added the cla-signed label May 1, 2017

Revert changes in dict

3ca7bba

Merge branch 'dev' into prepare-pep257

eeef56e

homeassistant added the cla-signed label May 2, 2017

balloob merged commit a4f1f6e into home-assistant:dev May 2, 2017

fabaff deleted the prepare-pep257 branch May 2, 2017 19:02

balloob mentioned this pull request May 5, 2017

0.44 #7444

Merged

home-assistant locked and limited conversation to collaborators Aug 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update docstrings #7374

Update docstrings #7374

fabaff commented Apr 30, 2017

mention-bot commented Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

MartinHjelmare commented Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

houndci-bot Apr 30, 2017

rytilahti Apr 30, 2017

rytilahti Apr 30, 2017

fabaff May 1, 2017

rytilahti Apr 30, 2017

fabaff May 1, 2017

rytilahti May 1, 2017

rytilahti Apr 30, 2017

rytilahti Apr 30, 2017

rytilahti Apr 30, 2017

balloob commented May 1, 2017

fabaff commented May 1, 2017

fabaff commented May 1, 2017

rytilahti commented May 1, 2017

MartinHjelmare commented May 1, 2017

balloob commented May 2, 2017

Update docstrings #7374

Update docstrings #7374

Conversation

fabaff commented Apr 30, 2017

Description:

Checklist:

mention-bot commented Apr 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinHjelmare commented Apr 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balloob commented May 1, 2017

fabaff commented May 1, 2017

fabaff commented May 1, 2017

rytilahti commented May 1, 2017

MartinHjelmare commented May 1, 2017

balloob commented May 2, 2017