Skip to content
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.

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

Fix imports #27293

Closed
wants to merge 1 commit into from
Closed

Fix imports #27293

wants to merge 1 commit into from

Conversation

balloob
Copy link
Member

@balloob balloob commented Oct 7, 2019

(couldn't reopen old one)

Description:

Run script to move single line imports up. Then run isort on changed files to make the sorting correct.

Part of #27284

Hacked together with this script:

from pathlib import Path

component_dir = Path("./homeassistant/components")

import_lines = ("import", "from", ",", ")")

for fil in component_dir.glob("**/*.py"):
    # if "xiaomi_miio/fan.py" not in str(fil):
    #     continue

    lines = fil.read_text().split("\n")
    inject_import_pos = None
    to_push_up = []

    quotes_found = 0

    normal_code_started = False

    for no, line in enumerate(lines):
        if inject_import_pos is None:
            quotes_found += line.count('"""')

            if quotes_found >= 2:
                inject_import_pos = no + 1

            continue

        if not normal_code_started:
            if line == "":
                continue

            if any(s in line for s in import_lines):
                continue

        normal_code_started = True

        if " import " in line:
            to_push_up.append(no)

    if to_push_up:
        print(fil, to_push_up)

    for no in to_push_up:
        lines.insert(inject_import_pos, lines.pop(no).strip())

    # print("\n".join(lines))
    fil.write_text("\n".join(lines))

@@ -1,8 +1,11 @@
"""Support the ISY-994 controllers."""
# Don't import this node as a device at all
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the script moved this comment up to the top and shouldn't have

Clean,
PlaySound,
Spot,
Stop,
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels a bit risky to have some of these imports have such a generic imported name (like Stop()) at a higher scope than they were before. It certainly reduces readability in this case, as it won't be obvious what the Stop() method comes from when it's used in one method later on.

@balloob
Copy link
Member Author

balloob commented Oct 7, 2019

Going to close this again. @OverloadUT is right that this can better be looked at on a per-integration basis. Also comments should be filtered out, plus pylint comments should bre removed. That's more work than I can handle right now.

@balloob balloob closed this Oct 7, 2019
@lock lock bot locked and limited conversation to collaborators Oct 8, 2019
@balloob balloob deleted the fix-inline-imports branch July 4, 2020 06:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants