Skip to content

Narrow scope of various pylint inline disables #15364

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.

Sign up for GitHub

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

Merged
balloob merged 2 commits into from
Oct 10, 2018
Merged

Narrow scope of various pylint inline disables #15364

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c3999e
Narrow scope of various pylint inline disables
scop Jul 7, 2018
d94a82c
Whitespace tweaks
scop Jul 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions homeassistant/components/apcupsd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,9 @@ def setup(hass, config):

# It doesn't really matter why we're not able to get the status, just that
# we can't.
# pylint: disable=broad-except
try:
DATA.update(no_throttle=True)
except Exception:
except Exception: # pylint: disable=broad-except
Copy link
Copy Markdown
Member

@fabaff fabaff Jul 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of in-line comment because deleting a line is way faster than marking the comment and then deleting. Also, it requires more skills with sed than if the comment is on it's own line.

Also, I see no real benefit of disabling an import-error with every import than on the top level once for all imports. We can't recover from the error in the code anyway.

Copy link
Copy Markdown
Member Author

@scop scop Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having it on the same line is different in functionality -- same line affects that line, previous line affects that entire lexical scope. I think that should override personal preferences. And I don't quite agree that the speed of deletion or ease of sed usage should be a consideration here.

Disabling all import errors at top level once doesn't seem useful to me -- if that's what desired, then the same effect can be achieved by just disabling the error globally in pylintrc. But I don't think that's a good idea.

Copy link
Copy Markdown
Member

@fabaff fabaff Jul 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having it on the same line is different in functionality -- same line affects that line, previous line affects that entire lexical scope. I think that should override personal preferences.

If you have only one try-except block somewhere then it doesn't matter if disable broad-except at the beginning of block or inside the block itself (Ok, fixing it would be the way to go). For the tests the scoping is much more sophisticated because it's needed and make sense.

And I don't quite agree that the speed of deletion or ease of sed usage should be a consideration here.

Well, it should as we disable pylint at somewhere around 500 to 1000 places. There are still left-over from the last cleaning round and new entries are sneaking in from time to time which we need to remove.

Disabling all import errors at top level once doesn't seem useful to me -- if that's what desired, then the same effect can be achieved by just disabling the error globally in pylintrc. But I don't think that's a good idea.

Disabling something like import-error for a file or globally are two different shoes.

Copy link
Copy Markdown
Member Author

@scop scop Jul 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if one can put broad-except disabling inside the try, I don't think one should, because that makes it inconsistent with a lot of other cases where such tricks are not available without undesired effects wrt scoping. (Not that I agree with it, but that inconsistency adds variability to the sed use case you mention, thus also making that harder.)

For the sed use case and acceptability of file-level import-error, I'm afraid we just have to agree to disagree. Anyway just two final data points:

For the sed use case, one should also consider lines where there are multiple symbols disabled on one line, as well as addressing possibly similar flake8 noqa comments as the pylint ones being removed. So in the big picture, being able to remove a line vs edit it doesn't really matter, more investigation/work is needed anyway. And I don't think sed is a proper tool for that job.

Top level disable=import-error in a file is a blanket permission for other bad imports to sneak into that file in addition to the ones it was originally added there for. The disable might be added there because there is a problem in CI with some imports, but the top level one may hide and let other ones through that blow up in user systems at runtime.

_LOGGER.exception("Failure while testing APCUPSd status retrieval.")
return False
return True
Expand Down
1 change: 1 addition & 0 deletions homeassistant/components/calendar/google.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def __init__(self, calendar_service, calendar_id, search,
self.event = None

def _prepare_query(self):
# pylint: disable=import-error
Copy link
Copy Markdown
Member

@fabaff fabaff Jul 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like that httplib should be a requirement for the platform.

Copy link
Copy Markdown
Member Author

@scop scop Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

from httplib2 import ServerNotFoundError

try:
Expand Down
5 changes: 2 additions & 3 deletions homeassistant/components/climate/eq3btsmart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,13 @@ def setup_platform(hass, config, add_entities, discovery_info=None):
add_entities(devices)


# pylint: disable=import-error
class EQ3BTSmartThermostat(ClimateDevice):
"""Representation of an eQ-3 Bluetooth Smart thermostat."""

def __init__(self, _mac, _name):
"""Initialize the thermostat."""
# We want to avoid name clash with this module.
import eq3bt as eq3
import eq3bt as eq3 # pylint: disable=import-error

self.modes = {
eq3.Mode.Open: STATE_ON,
Expand Down Expand Up @@ -176,7 +175,7 @@ def device_state_attributes(self):

def update(self):
"""Update the data from the thermostat."""
from bluepy.btle import BTLEException
from bluepy.btle import BTLEException # pylint: disable=import-error
try:
self._thermostat.update()
except BTLEException as ex:
Expand Down
2 changes: 0 additions & 2 deletions homeassistant/components/cover/zwave.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
For more details about this platform, please refer to the documentation
https://home-assistant.io/components/cover.zwave/
"""
# Because we do not compile openzwave on CI
# pylint: disable=import-error
import logging
from homeassistant.components.cover import (
DOMAIN, SUPPORT_OPEN, SUPPORT_CLOSE, ATTR_POSITION)
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/device_tracker/fritz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ def __init__(self, config):
self.password = config[CONF_PASSWORD]
self.success_init = True

# pylint: disable=import-error
import fritzconnection as fc
import fritzconnection as fc # pylint: disable=import-error

# Establish a connection to the FRITZ!Box.
try:
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/device_tracker/unifi_direct.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,9 @@ def _connect(self):

def _disconnect(self):
"""Disconnect the current SSH connection."""
# pylint: disable=broad-except
try:
self.ssh.logout()
except Exception:
except Exception: # pylint: disable=broad-except
pass
finally:
self.ssh = None
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/graphite.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ def run(self):
try:
self._report_attributes(
event.data['entity_id'], event.data['new_state'])
# pylint: disable=broad-except
except Exception:
except Exception: # pylint: disable=broad-except
# Catch this so we can avoid the thread dying and
# make it visible.
_LOGGER.exception("Failed to process STATE_CHANGED event")
Expand Down
12 changes: 4 additions & 8 deletions homeassistant/components/homekit_controller/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ def homekit_http_send(self, message_body=None, encode_chunked=False):

def get_serial(accessory):
"""Obtain the serial number of a HomeKit device."""
# pylint: disable=import-error
import homekit
import homekit # pylint: disable=import-error
for service in accessory['services']:
if homekit.ServicesTypes.get_short(service['type']) != \
'accessory-information':
Expand All @@ -85,8 +84,7 @@ class HKDevice():

def __init__(self, hass, host, port, model, hkid, config_num, config):
"""Initialise a generic HomeKit device."""
# pylint: disable=import-error
import homekit
import homekit # pylint: disable=import-error

_LOGGER.info("Setting up Homekit device %s", model)
self.hass = hass
Expand Down Expand Up @@ -132,8 +130,7 @@ def connect(self):

def accessory_setup(self):
"""Handle setup of a HomeKit accessory."""
# pylint: disable=import-error
import homekit
import homekit # pylint: disable=import-error

try:
data = self.get_json('/accessories')
Expand Down Expand Up @@ -185,8 +182,7 @@ def get_json(self, target):

def device_config_callback(self, callback_data):
"""Handle initial pairing."""
# pylint: disable=import-error
import homekit
import homekit # pylint: disable=import-error
pairing_id = str(uuid.uuid4())
code = callback_data.get('code').strip()
try:
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/homematic/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,7 @@ def link_homematic(self):
# Link events from pyhomematic
self._subscribe_homematic_events()
self._available = not self._hmdevice.UNREACH
# pylint: disable=broad-except
except Exception as err:
except Exception as err: # pylint: disable=broad-except
self._connected = False
_LOGGER.error("Exception while linking %s: %s",
self._address, str(err))
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/image_processing/dlib_face_detect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ def name(self):

def process_image(self, image):
"""Process image."""
# pylint: disable=import-error
import face_recognition
import face_recognition # pylint: disable=import-error

fak_file = io.BytesIO(image)
fak_file.name = 'snapshot.jpg'
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/keyboard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@

def setup(hass, config):
"""Listen for keyboard events."""
# pylint: disable=import-error
import pykeyboard
import pykeyboard # pylint: disable=import-error

keyboard = pykeyboard.PyKeyboard()
keyboard.special_key_assignment()
Expand Down
4 changes: 3 additions & 1 deletion homeassistant/components/keyboard_remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
For more details about this platform, please refer to the documentation at
https://home-assistant.io/components/keyboard_remote/
"""
# pylint: disable=import-error
import threading
import logging
import os
Expand Down Expand Up @@ -90,6 +89,7 @@ def __init__(self, hass, device_name, device_descriptor, key_value):
id_folder = '/dev/input/by-id/'

if os.path.isdir(id_folder):
# pylint: disable=import-error
from evdev import InputDevice, list_devices
device_names = [InputDevice(file_name).name
for file_name in list_devices()]
Expand All @@ -104,6 +104,7 @@ def __init__(self, hass, device_name, device_descriptor, key_value):

def _get_keyboard_device(self):
"""Get the keyboard device."""
# pylint: disable=import-error
from evdev import InputDevice, list_devices
if self.device_name:
devices = [InputDevice(file_name) for file_name in list_devices()]
Expand All @@ -121,6 +122,7 @@ def _get_keyboard_device(self):

def run(self):
"""Run the loop of the KeyboardRemote."""
# pylint: disable=import-error
from evdev import categorize, ecodes

if self.dev is not None:
Expand Down
3 changes: 1 addition & 2 deletions homeassistant/components/light/homekit_controller.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,7 @@ def __init__(self, *args):

def update_characteristics(self, characteristics):
"""Synchronise light state with Home Assistant."""
# pylint: disable=import-error
import homekit
import homekit # pylint: disable=import-error

for characteristic in characteristics:
ctype = characteristic['type']
Expand Down
5 changes: 2 additions & 3 deletions homeassistant/components/media_player/sonos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,16 +447,15 @@ def _set_favorites(self):
"""Set available favorites."""
# SoCo 0.16 raises a generic Exception on invalid xml in favorites.
# Filter those out now so our list is safe to use.
# pylint: disable=broad-except
try:
self._favorites = []
for fav in self.soco.music_library.get_sonos_favorites():
try:
if fav.reference.get_uri():
self._favorites.append(fav)
except Exception:
except Exception: # pylint: disable=broad-except
_LOGGER.debug("Ignoring invalid favorite '%s'", fav.title)
except Exception:
except Exception: # pylint: disable=broad-except
_LOGGER.debug("Ignoring invalid favorite list")

def _radio_artwork(self, url):
Expand Down
7 changes: 5 additions & 2 deletions homeassistant/components/raspihats.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
For more details about this component, please refer to the documentation at
https://home-assistant.io/components/raspihats/
"""
# pylint: disable=import-error,no-name-in-module
import logging
import threading
import time
Expand Down Expand Up @@ -125,7 +124,7 @@ def register_board(self, board, address):
with self._lock:
i2c_hat = self._i2c_hats.get(address)
if i2c_hat is None:
# pylint: disable=import-error
# pylint: disable=import-error,no-name-in-module
import raspihats.i2c_hats as module
constructor = getattr(module, board)
i2c_hat = constructor(address)
Expand All @@ -143,6 +142,7 @@ def register_board(self, board, address):

def run(self):
"""Keep alive for I2C-HATs."""
# pylint: disable=import-error,no-name-in-module
from raspihats.i2c_hats import ResponseException

_LOGGER.info(log_message(self, "starting"))
Expand Down Expand Up @@ -205,6 +205,7 @@ def register_online_callback(self, address, channel, callback):

def read_di(self, address, channel):
"""Read a value from a I2C-HAT digital input."""
# pylint: disable=import-error,no-name-in-module
from raspihats.i2c_hats import ResponseException

with self._lock:
Expand All @@ -217,6 +218,7 @@ def read_di(self, address, channel):

def write_dq(self, address, channel, value):
"""Write a value to a I2C-HAT digital output."""
# pylint: disable=import-error,no-name-in-module
from raspihats.i2c_hats import ResponseException

with self._lock:
Expand All @@ -228,6 +230,7 @@ def write_dq(self, address, channel, value):

def read_dq(self, address, channel):
"""Read a value from a I2C-HAT digital output."""
# pylint: disable=import-error,no-name-in-module
from raspihats.i2c_hats import ResponseException

with self._lock:
Expand Down
13 changes: 6 additions & 7 deletions homeassistant/components/rpi_gpio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
For more details about this component, please refer to the documentation at
https://home-assistant.io/components/rpi_gpio/
"""
# pylint: disable=import-error
import logging

from homeassistant.const import (
Expand All @@ -19,7 +18,7 @@

def setup(hass, config):
"""Set up the Raspberry PI GPIO component."""
from RPi import GPIO
from RPi import GPIO # pylint: disable=import-error

def cleanup_gpio(event):
"""Stuff to do before stopping."""
Expand All @@ -36,32 +35,32 @@ def prepare_gpio(event):

def setup_output(port):
"""Set up a GPIO as output."""
from RPi import GPIO
from RPi import GPIO # pylint: disable=import-error
GPIO.setup(port, GPIO.OUT)


def setup_input(port, pull_mode):
"""Set up a GPIO as input."""
from RPi import GPIO
from RPi import GPIO # pylint: disable=import-error
GPIO.setup(port, GPIO.IN,
GPIO.PUD_DOWN if pull_mode == 'DOWN' else GPIO.PUD_UP)


def write_output(port, value):
"""Write a value to a GPIO."""
from RPi import GPIO
from RPi import GPIO # pylint: disable=import-error
GPIO.output(port, value)


def read_input(port):
"""Read a value from a GPIO."""
from RPi import GPIO
from RPi import GPIO # pylint: disable=import-error
return GPIO.input(port)


def edge_detect(port, event_callback, bounce):
"""Add detection for RISING and FALLING events."""
from RPi import GPIO
from RPi import GPIO # pylint: disable=import-error
GPIO.add_event_detect(
port,
GPIO.BOTH,
Expand Down
6 changes: 2 additions & 4 deletions homeassistant/components/scsgate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,10 @@ def setup(hass, config):
device = config[DOMAIN][CONF_DEVICE]
global SCSGATE

# pylint: disable=broad-except
try:
SCSGATE = SCSGate(device=device, logger=_LOGGER)
SCSGATE.start()
except Exception as exception:
except Exception as exception: # pylint: disable=broad-except
_LOGGER.error("Cannot setup SCSGate component: %s", exception)
return False

Expand Down Expand Up @@ -101,10 +100,9 @@ def handle_message(self, message):
if new_device_activated:
self._activate_next_device()

# pylint: disable=broad-except
try:
self._devices[message.entity].process_event(message)
except Exception as exception:
except Exception as exception: # pylint: disable=broad-except
msg = "Exception while processing event: {}".format(exception)
self._logger.error(msg)
else:
Expand Down
5 changes: 2 additions & 3 deletions homeassistant/components/sensor/bh1750.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,11 @@
})


# pylint: disable=import-error
async def async_setup_platform(hass, config, async_add_entities,
discovery_info=None):
"""Set up the BH1750 sensor."""
import smbus
from i2csense.bh1750 import BH1750
import smbus # pylint: disable=import-error
from i2csense.bh1750 import BH1750 # pylint: disable=import-error

name = config.get(CONF_NAME)
bus_number = config.get(CONF_I2C_BUS)
Expand Down
5 changes: 2 additions & 3 deletions homeassistant/components/sensor/bme280.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,11 @@
})


# pylint: disable=import-error
async def async_setup_platform(hass, config, async_add_entities,
discovery_info=None):
"""Set up the BME280 sensor."""
import smbus
from i2csense.bme280 import BME280
import smbus # pylint: disable=import-error
from i2csense.bme280 import BME280 # pylint: disable=import-error

SENSOR_TYPES[SENSOR_TEMP][1] = hass.config.units.temperature_unit
name = config.get(CONF_NAME)
Expand Down
4 changes: 2 additions & 2 deletions homeassistant/components/sensor/bme680.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,15 @@ async def async_setup_platform(hass, config, async_add_entities,
return


# pylint: disable=import-error, no-member
def _setup_bme680(config):
"""Set up and configure the BME680 sensor."""
from smbus import SMBus
from smbus import SMBus # pylint: disable=import-error
import bme680
Copy link
Copy Markdown
Member

@fabaff fabaff Jul 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change will no longer cover bme680.

Copy link
Copy Markdown
Member Author

@scop scop Jul 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, but covering it does not appear to be necessary for the pylint tests to pass.


sensor_handler = None
sensor = None
try:
# pylint: disable=no-member
i2c_address = config.get(CONF_I2C_ADDRESS)
bus = SMBus(config.get(CONF_I2C_BUS))
sensor = bme680.BME680(i2c_address, bus)
Expand Down
Loading