Fix Linter Errors from ruff check dinov3 #233
Description
Running the ruff check dinov3 command on the current main branch reveals several linting errors that cause the linting CI workflow to fail. These errors include unused imports, ambiguous variable names, and incorrect type comparisons.
Summary of Linter Errors
The following issues were identified by Ruff:
- Unused Imports (F401):
- dinov3/eval/results.py: openpyxl is imported but unused (in a try...except block).
- dinov3/hub/classifiers.py: typing.Optional is imported but unused.
- dinov3/hub/depthers.py: urllib.parse.urlparse and pathlib.Path are imported but unused.
- Incorrect Type Comparison (E721):
- dinov3/layers/fp8_linear.py: The code uses type(module) == ... for an intentional exact type check. This should be flagged with # noqa: E721 to acknowledge the specific use case.
- dinov3/layers/sparse_linear.py: Similar to the above, an assert uses an exact type check that needs a noqa flag.
- Ambiguous Variable Name (E741):
- dinov3/eval/detection/models/detr.py: The variable l is used in a list comprehension, which Ruff flags as ambiguous. It should be renamed to something more descriptive like label.
- Invalid # noqa Directive:
- dinov3/eval/results.py: An existing noqa comment (# noqa: 401) is incorrectly formatted and should be # noqa: F401.
Proposed Solution
A pull request will be submitted to address all the identified issues. The fixes will include removing unused code, renaming variables for clarity, and adding correctly formatted noqa directives to lines where the linter's suggestion is intentionally bypassed.
Full Linter Output
Click to expand Ruff output
warning: Invalid `# noqa` directive on dinov3/eval/results.py:24: expected a comma-separated list of codes (e.g., `# noqa: F401, F841`).
dinov3/eval/detection/models/detr.py:427:66: E741 Ambiguous variable name: `l`
|
425 | boxes = boxes * scale_fct[:, None, :]
426 |
427 | results = [{"scores": s, "labels": l, "boxes": b} for s, l, b in zip(scores, labels, boxes)]
| ^ E741
428 |
429 | return results
|
dinov3/eval/results.py:24:12: F401 `openpyxl` imported but unused; consider using `importlib.util.find_spec` to test for availability
|
23 | try:
24 | import openpyxl # noqa: 401
| ^^^^^^^^ F401
25 |
26 | HAS_OPENPYXL = True
|
= help: Remove unused import: `openpyxl`
dinov3/hub/classifiers.py:8:20: F401 [*] `typing.Optional` imported but unused
|
6 | import os
7 | from enum import Enum
8 | from typing import Optional
| ^^^^^^^^ F401
9 |
10 | import torch
|
= help: Remove unused import: `typing.Optional`
dinov3/hub/depthers.py:11:26: F401 [*] `urllib.parse.urlparse` imported but unused
|
9 | import torch
10 | from dinov3.eval.dense.depth.models import build_depther
11 | from urllib.parse import urlparse
| ^^^^^^^^ F401
12 | from pathlib import Path
|
= help: Remove unused import: `urllib.parse.urlparse`
dinov3/hub/depthers.py:12:21: F401 [*] `pathlib.Path` imported but unused
|
10 | from dinov3.eval.dense.depth.models import build_depther
11 | from urllib.parse import urlparse
12 | from pathlib import Path
| ^^^^ F401
13 |
14 | from .utils import DINOV3_BASE_URL
|
= help: Remove unused import: `pathlib.Path`
dinov3/layers/fp8_linear.py:107:12: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
105 | if not isinstance(module, torch.nn.Linear) or not filter_re.search(name):
106 | return module
107 | if type(module) == torch.nn.Linear:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
108 | new_cls = Fp8Linear
109 | elif type(module) == LinearKMaskedBias:
|
dinov3/layers/fp8_linear.py:109:14: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
107 | if type(module) == torch.nn.Linear:
108 | new_cls = Fp8Linear
109 | elif type(module) == LinearKMaskedBias:
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E721
110 | new_cls = Fp8LinearKMaskedBias
111 | else:
|
dinov3/layers/sparse_linear.py:54:16: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks
|
52 | if not isinstance(module, nn.Linear) or not filter_fn(name):
53 | return module
54 | assert type(module) == nn.Linear, "Subtypes not supported"
| ^^^^^^^^^^^^^^^^^^^^^^^^^ E721
55 | new_module = LinearW24(
56 | in_features=module.in_features,
|
Found 8 errors.
[*] 3 fixable with the `--fix` option.