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

test: Remove unused Python imports #12896

Merged
merged 2 commits into from
Dec 23, 2024

Conversation

cclauss
Copy link
Contributor

@cclauss cclauss commented Dec 23, 2024

% ruff check --output-format=concise # https://docs.astral.sh/ruff

test/e2e-v2/cases/browser/docker/provider.py:19:20: F401 [*] `urllib.request` imported but unused
test/e2e-v2/cases/python/consumer.py:18:8: F401 [*] `urllib.parse` imported but unused
test/e2e-v2/cases/python/provider-kafka.py:20:20: F401 [*] `urllib.request` imported but unused
test/e2e-v2/cases/python/provider.py:20:20: F401 [*] `urllib.request` imported but unused
Found 4 errors.
[*] 4 fixable with the `--fix` option.

% ruff check --fix

Found 4 errors (4 fixed, 0 remaining).

% ruff rule F401

unused-import (F401)

Derived from the Pyflakes linter.

Fix is sometimes available.

What it does

Checks for unused imports.

Why is this bad?

Unused imports add a performance overhead at runtime, and risk creating
import cycles. They also increase the cognitive load of reading the code.

If an import statement is used to check for the availability or existence
of a module, consider using importlib.util.find_spec instead.

If an import statement is used to re-export a symbol as part of a module's
public interface, consider using a "redundant" import alias, which
instructs Ruff (and other tools) to respect the re-export, and avoid
marking it as unused, as in:

from module import member as member

Alternatively, you can use __all__ to declare a symbol as part of the module's
interface, as in:

# __init__.py
import some_module

__all__ = ["some_module"]

Fix safety

Fixes to remove unused imports are safe, except in __init__.py files.

Applying fixes to __init__.py files is currently in preview. The fix offered depends on the
type of the unused import. Ruff will suggest a safe fix to export first-party imports with
either a redundant alias or, if already present in the file, an __all__ entry. If multiple
__all__ declarations are present, Ruff will not offer a fix. Ruff will suggest an unsafe fix
to remove third-party and standard library imports -- the fix is unsafe because the module's
interface changes.

Example

import numpy as np  # unused import


def area(radius):
    return 3.14 * radius**2

Use instead:

def area(radius):
    return 3.14 * radius**2

To check the availability of a module, use importlib.util.find_spec:

from importlib.util import find_spec

if find_spec("numpy") is not None:
    print("numpy is installed")
else:
    print("numpy is not installed")

Options

  • lint.ignore-init-module-imports
  • lint.pyflakes.allowed-unused-imports

References

Fix

  • Add a unit test to verify that the fix works.
  • Explain briefly why the bug exists and how to fix it.

Improve the performance of <class or module or ...>

  • Add a benchmark for the improvement, refer to the existing ones
  • The benchmark result.
<Paste the benchmark results here>
  • Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
  • Update the CHANGES log.

@wu-sheng wu-sheng requested a review from kezhenxu94 December 23, 2024 12:28
@wu-sheng wu-sheng added the test Test requirements about performance, feature or before release. label Dec 23, 2024
@wu-sheng wu-sheng added this to the 10.2.0 milestone Dec 23, 2024
@cclauss cclauss force-pushed the remove-unused-python-imports branch from 74ad480 to b61c6df Compare December 23, 2024 12:28
@wu-sheng wu-sheng merged commit 6fb8f23 into apache:master Dec 23, 2024
165 checks passed
@cclauss cclauss deleted the remove-unused-python-imports branch December 23, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Test requirements about performance, feature or before release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants