Core: Migrate integrations to use top-level imports

[frenck]

Pylint 2.4 introduced a new check: import-outside-toplevel.

This check warns when modules are imported from places other than a module top-level, e.g., inside a function or a class.

With the upcoming upgrade of Pylint in our codebase, we are going to disable this check. However, it would have been nice to be able to enable it, so we can prevent imports outside of the top-level in our builds.

Since we started using our manifests for integration requirements, these function-level imports are no longer required or needed, and thus, can now be removed or moved to the top-level.

This issue is a placeholder to keep track of the status of this migration. If you resolve this issue for one of the integrations listed below, please refer to this issue in the PR you are creating (so we can keep track).

What has to be done?

While this is a pretty small and simple change, it takes a bit of time to do right and neat.

• Move the imports to the top.
• Combine duplicates and multiple imports from the same module.
• Make sure imports are ordered correctly, including the ones already there. (e.g., using isort).
• Remove undeeded empty lines caused by moving the import from the old location.
• Pay attention to comments placed around the old import locations. You might need to move those with the import or adjust if needed.
• Make sure, pylint, black and mypy are happy.
• Make sure tests still pass.

More information can be found in the developer documentation:
https://developers.home-assistant.io

Integrations to adjust

I've generated a list of all integrations that still use import outside of the top-level imports using the following command:

pylint homeassistant/components | grep import-outside-toplevel | cut -d'/' -f 3 | sort | uniq

Migrated integrations

- [x] abode - [x] acer_projector - [x] ads - [x] amazon_polly - [x] anthemav - [x] apple_tv - [x] aprs - [x] aquostv - [x] arduino - [x] aruba - [x] auth - [x] automatic - [x] aws - [x] axis - [x] bbox - [x] bh1750 - [x] bluesound - [x] bluetooth_le_tracker - [x] bme280 - [x] bme680 - [x] broadlink - [x] brottsplatskartan - [x] browser - [x] bt_home_hub_5 - [x] bt_smarthub - [x] caldav - [x] cisco_ios - [x] co2signal - [x] config - [x] crimereports - [x] darksky - [x] decora - [x] denonavr - [x] deutsche_bahn - [x] device_tracker - [x] dht - [x] digital_ocean - [x] digitalloggers - [x] discogs - [x] discord - [x] dlib_face_detect - [x] dlib_face_identify - [x] dnsip - [x] dovado - [x] dsmr - [x] dte_energy_bridge - [x] dweet - [x] ebusd - [x] eliqonline - [x] elkm1 - [x] epson - [x] eufy - [x] everlights - [x] feedreader - [x] fitbit - [x] flic - [x] flux_led - [x] fritz - [x] fritzbox_callmonitor - [x] fritzbox_netmonitor - [x] frontend - [x] frontier_silicon - [x] futurenow - [x] gc100 - [x] geo_rss_events - [x] github - [x] gitlab_ci - [x] gntp - [x] goalfeed - [x] google - [x] google_travel_time - [x] gpsd - [x] greenwave - [x] gtfs - [x] harman_kardon_avr - [x] harmony - [x] hikvisioncam - [x] hp_ilo - [x] html5 - [x] http - [x] htu21d - [x] hue - [x] ifttt - [x] imap - [x] imap_email_content - [x] insteon - [x] iperf3 - [x] iss - [x] isy994 - [x] itach - [x] juicenet - [x] keyboard - [x] kira - [x] knx - [x] kodi - [x] konnected - [x] lastfm - [x] lg_soundbar - [x] lifx - [x] lifx_legacy - [x] linode - [x] linux_battery - [x] lirc - [x] liveboxplaytv - [x] logbook - [x] loopenergy - [x] lupusec - [x] lw12wifi - [x] magicseaweed - [x] mcp23017 - [x] melissa - [x] message_bird - [x] metoffice - [x] miflora - [x] mitemp_bt - [x] mobile_app - [x] mopar - [x] mpd - [x] mqtt - [x] mvglive - [x] mychevy - [x] mythicbeastsdns - [x] namecheapdns - [x] nest - [x] netatmo - [x] netgear - [x] netgear_lte - [x] neurio_energy - [x] niko_home_control - [x] nilu - [x] nissan_leaf - [x] norway_air - [x] nuheat - [x] oasa_telematics - [x] ohmconnect - [x] onkyo - [x] onvif - [x] opencv - [x] openevse - [x] openweathermap - [x] orangepi_gpio - [x] osramlightify - [x] otp - [x] owntracks - [x] panasonic_bluray - [x] panasonic_viera - [x] pandora - [x] piglow - [x] pocketcasts - [x] proliphix - [x] prometheus - [x] proxy - [x] ptvsd - [x] pushbullet - [x] pushover - [x] qrcode - [x] raspihats - [x] recollect_waste - [x] recorder - [x] reddit - [x] rejseplanen - [x] remember_the_milk - [x] repetier - [x] rflink - [x] rfxtrx - [x] route53 - [x] rpi_pfio - [x] samsungtv - [x] season - [x] serial - [x] sesame - [x] seven_segments - [x] shiftr - [x] shodan - [x] skybeacon - [x] slack - [x] sma - [x] smappee - [x] smarthab - [x] snapcast - [x] snmp - [x] socialblade - [x] sonos - [x] sony_projector - [x] speedtestdotnet - [x] spotcrime - [x] spotify - [x] sql - [x] squeezebox - [x] startca - [x] statsd - [x] steam_online - [x] stream - [x] switchmate - [x] synology_srm - [x] syslog - [x] systemmonitor - [x] ted5000 - [x] telegram_bot - [x] tellstick - [x] tensorflow - [x] thermoworks_smoke - [x] thingspeak - [x] tibber - [x] tikteck - [x] tplink_lte - [x] transport_nsw - [x] trend - [x] tts - [x] upcloud - [x] updater - [x] uscis - [x] vasttrafik - [x] venstar - [x] vera - [x] verisure - [x] vizio - [x] vlc - [x] w800rf32 - [x] wake_on_lan - [x] waqi - [x] waterfurnace - [x] watson_iot - [x] waze_travel_time - [x] wemo - [x] wink - [x] workday - [x] wunderlist - [x] xmpp - [x] yamaha - [x] yamaha_musiccast - [x] yeelight - [x] yeelightsunflower - [x] yr - [x] zengge - [x] zestimate - [x] zha - [x] zigbee

Round 2!

⚠️ Please be sure to follow the procedure written at the top of this issue
⚠️ It is possible that the change was revered on an integration/component before! Please be sure to check the history of PR's to see if that was the case.

• [x] aftership #28860
• [x] alarmdecoder #28862
• [x] ambient_station
• [x] asuswrt
• [x] canary
• [x] clementine
• [x] daikin
• [x] danfoss_air
• [x] datadog
• [x] deconz
• [x] decora_wifi
• [x] deluge
• [x] directv
• [x] discovery
• [x] dlink
• [x] dlna_dmr
• [x] dominos
• [x] doorbird
• [x] duke_energy
• [x] dunehd
• [x] dyson
• [x] ebox
• [x] ecoal_boiler
• [x] econet
• [x] ecovacs
• [x] eddystone_temperature
• [x] edimax
• [x] ee_brightbox
• [x] egardia
• [x] eight_sleep
• [x] emby
• [x] emulated_roku
• [x] enigma2
• [x] enocean
• [x] enphase_envoy
• [x] entur_public_transport
• [x] environment_canada
• [x] envisalink
• [x] epsonworkforce
• [x] eq3btsmart
• [x] esphome
• [x] etherscan
• [x] familyhub
• [x] fastdotcom
• [x] ffmpeg_motion
• [x] ffmpeg_noise
• [x] fibaro
• [x] fido
• [x] fints
• [x] fixer
• [x] fleetgo
• [x] flexit
• [x] flunearyou
• [x] folder_watcher
• [x] foobot
• [x] fortigate
• [x] free_mobile
• [x] freebox
• [x] frontend
• [x] gearbest
• [x] geizhals
• [x] geo_json_events
• [x] gitter
• [x] gogogate2
• [x] google_assistant
• [x] google_pubsub
• [x] google_translate
• [x] gpmdp
• [x] greeneye_monitor
• [x] group
• [x] gstreamer
• [x] habitica
• [x] hangouts
• [x] hdmi_cec
• [x] heatmiser
• [x] hikvision
• [x] history
• [x] hlk_sw16
• [x] homekit
• [x] homekit_controller
• [x] homematic
• [x] homeworks
• [x] horizon
• [x] http
• [x] hue
• [x] hunterdouglas_powerview
• [x] hydrawise
• [x] ialarm
• [x] icloud
• [x] idteck_prox
• [x] iglo
• [x] ign_sismologia
• [x] ihc
• [x] influxdb
• [x] iota
• [x] ipma
• [x] irish_rail_transport
• [x] islamic_prayer_times
• [x] izone
• [x] joaoapps_join
• [x] keenetic_ndms2
• [x] kiwi
• [x] kwb
• [x] lacrosse
• [x] lametric
• [x] launch_library
• [x] lg_netcast
• [x] lightwave
• [x] limitlessled
• [x] litejet
• [x] logi_circle
• [x] london_underground
• [x] luci
• [x] lutron
• [x] lutron_caseta
• [x] lyft
• [x] mailgun
• [x] matrix
• [x] maxcube
• [x] media_extractor
• [x] mediaroom
• [x] meteo_france
• [x] meteoalarm
• [x] mfi
• [x] mhz19
• [x] microsoft
• [x] mobile_app
• [x] mochad
• [x] modbus
• [x] modem_callerid
• [x] monoprice
• [x] moon
• [x] mycroft
• [x] mysensors
• [x] mystrom
• [x] n26
• [x] nad
• [x] nanoleaf
• [x] nederlandse_spoorwegen
• [x] nello
• [x] ness_alarm
• [x] netdata
• [x] netio
• [x] nextbus
• [x] nmap_tracker
• [x] nmbs
• [x] noaa_tides
• [x] notion
• [x] nsw_fuel_station
• [x] nsw_rural_fire_service_feed
• [x] nuimo_controller
• [x] nut
• [x] nx584
• [x] onboarding
• [x] openhome
• [x] opensensemap
• [x] openuv
• [x] opple
• [x] orvibo
• [x] owlet
• [x] pencom
• [x] philips_js
• [x] pilight
• [x] pjlink
• [x] plant
• [x] plum_lightpad
• [x] point
• [x] postnl
• [x] prezzibenzina
• [x] ps4
• [x] pushetta
• [x] python_script
• [x] qbittorrent
• [x] qnap
• [x] quantum_gateway
• [x] qwikswitch
• [x] rachio
• [x] radarr
• [x] raincloud
• [x] rainmachine
• [x] random
• [x] raspyrfm
• [x] recswitch
• [x] remote_rpi_gpio
• [x] ring
• [x] ripple
• [x] rocketchat
• [x] roku
• [x] roomba
• [x] route53
• [x] rova
• [x] rpi_gpio_pwm
• [x] rpi_rf
• [x] russound_rio
• [x] russound_rnet
• [x] sabnzbd
• [x] satel_integra
• [x] scsgate
• [x] sense
• [x] sensehat
• [x] serial_pm #28861
• [x] seventeentrack
• [x] sht31
• [x] simplepush
• [x] simplisafe
• [x] sisyphus
• [x] skybell
• [x] sleepiq
• [x] smarty
• [x] smhi
• [x] sochain
• [x] solaredge
• [x] somfy_mylink
• [x] sonarr
• [x] soundtouch
• [x] spc
• [x] spider
• [x] starlingbank
• [x] statistics
• [x] stiebel_eltron
• [x] stream
• [x] streamlabswater
• [x] supla
• [x] swiss_hydrological_data
• [x] swiss_public_transport
• [x] switchbot
• [x] switcher_kis
• [x] syncthru
• [x] synology
• [x] synologydsm
• [x] system_log
• [x] tado
• [x] tahoma
• [x] tank_utility
• [x] tapsaff
• [x] tautulli
• [x] tellduslive
• [x] temper
• [x] tensorflow
• [x] thinkingcleaner
• [x] todoist
• [x] tof
• [x] toon
• [x] touchline
• [x] traccar
• [x] trackr
• [x] tradfri
• [x] trafikverket_weatherstation
• [x] travisci
• [x] tuya
• [x] twilio_call
• [x] ubee
• [x] unifi_direct
• [x] upnp
• [x] uptimerobot
• [x] usgs_earthquakes_feed
• [x] uvc
• [x] velux
• [x] version
• [x] vizio
• [x] volkszaehler
• [x] volvooncall
• [x] vultr
• [x] watson_tts
• [x] webostv
• [x] websocket_api
• [x] wirelesstag
• [x] xeoma
• [x] xfinity
• [x] xiaomi
• [x] xiaomi_aqara
• [x] xiaomi_tv
• [x] yale_smart_alarm
• [x] yi
• [x] zabbix
• [x] zha
• [x] zhong_hong
• [x] ziggo_mediabox_xl
• [x] zoneminder
• [ ] zwave

[frenck]

🆘 Marked as "Help wanted"

If you want to help out on this, go ahead, it is really appreciated!

[ggravlingen]

@frenck there were a few sneaky lines in the Tradfri integration too. Couldn't spot them on the list above though. :eyes:

[frenck]

@ggravlingen I've seen a couple of integrations more that are not on this list.
Why Pylint is not triggering on those, is unclear to me. The above list is simply generated. 🤷‍♂

[Bouni]

@frenck I've seen that you updated the list just minutes ago, but some components are still marked as to be done despite they are already done.
For example anthemav, the PR #27468 was merged 4days ago and pylint 2.4.2 gives me no error if I do pylint homeassistant/components/anthemav | grep import-outside-toplevel on the dev branch!?

This is also true for aprs and brottsplatskartan

[frenck]

Thanks, @Bouni! 👍

I've updated the list for those. I try to keep it up to date based on merge notifications, but it is quite easy to miss one.

[Bouni]

Thanks!

[Misiu]

What about rpi_gpio? there are an import in the setup and inside other functions.
Does that count? If not then sorry, I'm new to HA and Python 🙂

[Bouni]

@Misiu ist right!

pylint --version
pylint 2.4.2
astroid 2.3.1
Python 3.7.4 (default, Jul 16 2019, 07:12:58)
[GCC 9.1.0]

pylint  --disable R,C,W,E --enable C0415 ~/projects/home-assistant/homeassistant/components/rpi_gpio

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

For what ever reason pylint does not trigger on these imports!? I even tried to remove the # pylint: disable=import-error but that doesn't change anything.
Also if I run pylint without the --disable and --enable options no import-outside-toplevel is found

[Bouni]

Oh snap, its even worse!

The aftership component for example

pylint --disable R,C,W,E --enable C0415 ~/projects/home-assistant/homeassistant/components/aftership/sensor.py

------------------------------------
Your code has been rated at 10.00/10

rg "\s+from\s+[\w_.]+\s+import" aftership/sensor.py
59:    from pyaftership.tracker import Tracking

https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/aftership/sensor.py#L59

ripgrep search

rg -c "\s+from\s+[\w_.]+\s+import" * | wc -l
487
rg "\s{4,}import" * | wc -l
263

For sure there is overlap with pylint, but that gave just 239 results in the first place!

Edit:

rg --line-number "(\s{4,}import)|(\s{4,}from\s+[\w_.]+)" * in the homeassistant/components folder gave me ~1400 results:

https://gist.github.com/Bouni/e8abaab8c264328e8813b3f50426697f

[frenck]

@Bouni @Misiu I've answered on that 3 days ago: https://github.com/home-assistant/home-assistant/issues/27284#issuecomment-541432046

You guys are absolutely right, and I am aware of this. However, the goal of this issue is to get the rule enabled for Pylint, let's focus on that first.

[Bouni]

Sorry, totally overlooked that

[Misiu]

@Bouni @frenck I've created PR that moves imports in rpi_gpio. Small fix, but one more integration can be marked as fixed 🙂

[Misiu]

@bouni what about this one? https://github.com/home-assistant/home-assistant/blob/dev/homeassistant/components/mcp23017/switch.py
Should imports from L34-L36 and L57 also be moved to top of the file?

[Bouni]

@Misiu Yes, these should also be on top level.
You can find a list of all components that need to be checked/modified in this gist

[Misiu]

@Bouni thanks for the list, it is long!
BTW I've created PR for rpi_gpio, but I don't know why Azure Pipelines give me this error:

homeassistant/components/rpi_gpio/__init__.py:4:0: E0401: Unable to import 'RPi' (import-error)

same with other PR's. Do I have to pay attention to this? Old code give the exact same error.

[Bouni]

@Misiu Thats because you removed the pylint comment after the import

It was from RPi import GPIO # pylint: disable=import-error initially and you put jut from RPi import GPIO into the top level.
Append the pylint comment and you're fine.
By the way, you should run tests before you submit a PR that way you can spot these errors locally!

[Misiu]

@Bouni thanks for the hint and sorry for the trouble. Already fixed in my two PR.

[briglx]

@cgtobi I think this is still open because there are more components that need to be merged.

[Quentame]

luftdaten component is missing on the list
qrcode seems nice
venstar done

[frenck]

@Quentame I am aware there are more imports to fix. However, this list is generated from Pylint. This list indicates what is needed to enable the rule in Pylint.

The other are nice to fix, but not directly the goal of this issue.

[djpremier]

@frenck remove discogs, it hasn't imports to put on top-level

[frenck]

@djpremier Is has been moved already in #27679

[djpremier]

@frenck Thanks, I don't know why but I had not found this PR

[djpremier]

@frenck I had closed this PR improperly, now it has the wrong labels, is it possible to correct? And also parse to possibly be merged. #27974

[djpremier]

@frenck qrcode has no imports to move, has it already been done (found no pull request on this)?

[djpremier]

Hi @frenck,
venstar fixed by: #27478
ZHA fixed by: #28071

[frenck]

Current status: Seems like the last items that need to be checked off, are all open as a PR.

This means all work for this issue has been done

_Unless a PR drops off of course._

[Bouni]

@frenck There are many more if you do a scan using ripgrep instead of pylint!
I expect pylint to fix the issue of not finding some imports in the future.

Here's a list I made using ripgrep on the latest dev branch (379 matches): https://gist.github.com/Bouni/d0e24db83d0a06130de20149925a9d13

I'm aware that some of them are already have an open PR but still there are many to do.

[frenck]

As stated before, I'm aware, however, that was not the scope of this issue.

[Bouni]

OK, do you want to get PRs that move imports to toplevel for components not listet here?

[frenck]

The goal is to enable the Pylint rule. Once all the above integrations are adjusted en merged, we can try starting to enable it. Depending on that outcome we might need to adjust the list.

Regarding other integrations, sure go ahead I would say.

[springstan]

FYI the imports in the Melissa component are already at the top :)

[Quentame]

Only 3 components are maked as undone, but almost 0 !

See the last PR states :

• owntracks https://github.com/home-assistant/home-assistant/pull/27780 (merged)
• piglow https://github.com/home-assistant/home-assistant/pull/28046 (merged)
• samsungtv https://github.com/home-assistant/home-assistant/pull/27775 (merged)

[frenck]

Thanks all for helping out!

I'll be picking this up later tonight to see what the current state is.

[Bouni]

There are currently two open PR's which I opend, but unfortunately one of them #27880 clementine runs the tests fine with Python3.7 (which I tested with) but fails with 3.6. The other one #27808 asuswrt fails tests but I'm not experienced enough to figure out how to fix the tests.
Should I just revoke both PR's and let somebody else try to move the imports?

[Quentame]

I replace the « fix » by « f!x » now

[springstan]

Btw @frenck could you please mark media_extractor, moon, nsw_rural_fire_service_feed and tensorflow as solved? Their imports are already at the top or are wrapped in a try-catch statement. Thanks :)

[frenck]

done.

[Quentame]

@frenck : The last PR #28999 (about zwave) has been closed, so not merged. I am letting someone else taking a look at this.