MAINT: Enforce additional ruff rules#416
MAINT: Enforce additional ruff rules#416DimitriPapadopoulos wants to merge 7 commits intodata-apis:mainfrom
Conversation
Loop control variable not used within loop body
Do not call `getattr` with a constant attribute value. It is not any safer than normal property access.
Unnecessary generator (rewrite as a set comprehension)
Unnecessary `list()` call within `sorted()`
PEP 484 prohibits implicit `Optional`
Unused `noqa` directive
There was a problem hiding this comment.
Pull request overview
This maintenance PR tightens linting by enabling additional Ruff rule sets and updates code/tests to comply with the newly enforced checks.
Changes:
- Expanded Ruff
selectto include additional rule families (e.g.,B,PERF,UP,FURB) and enabledRUF100to detect unusednoqadirectives. - Adjusted tests and library code to satisfy new lint expectations (avoid constant-string
getattr, simplify set/list constructions, improve type annotations). - Reduced/removed some
noqausage where it’s no longer needed.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Enables additional Ruff lint rules and adds targeted ignores. |
tests/test_no_dependencies.py |
Removes constant-string getattr calls to satisfy new lint rules. |
tests/test_isdtype.py |
Removes an inline noqa, but currently introduces a Ruff E721 lint failure. |
array_api_compat/torch/fft.py |
Improves optional type annotations for FFT wrappers; RUF100 may expose an unused noqa in the file. |
array_api_compat/torch/_aliases.py |
Replaces unused loop variable with _. |
array_api_compat/numpy/__init__.py |
Removes a file-level noqa directive now that it’s not needed. |
array_api_compat/cupy/linalg.py |
Simplifies __all__ de-duplication construction. |
array_api_compat/common/_helpers.py |
Uses set comprehension for clarity and lint compliance. |
array_api_compat/__init__.py |
Removes an unnecessary noqa code from the star import. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| res = dtype_ == kind | ||
| assert type(res) is bool # noqa: E721 | ||
| assert type(res) is bool |
There was a problem hiding this comment.
Removing the # noqa: E721 will cause Ruff to flag this line (E721 is included via the selected E7 rules). Either restore the noqa: E721 (optionally with a brief justification), or rewrite the assertion to avoid direct type(...) is ... comparison (e.g., use an isinstance-based check if that satisfies the intent).
| assert type(res) is bool | |
| assert isinstance(res, bool) |
There was a problem hiding this comment.
I don't see ruff flagging this line.
With that said, this line could be modified as suggested, but the suggestion is orthogonal to this PR.
No description provided.