Copilot commented on code in PR #1143: URL: https://github.com/apache/mahout/pull/1143#discussion_r2900052705
########## pyproject.toml: ########## @@ -96,6 +96,25 @@ exclude = [ # api: benchmark module loaded via sys.path in tests allowed-unresolved-imports = ["_qdp", "_qdp.*", "api", "api.*"] +[tool.ruff.lint] +select = [ + # pyupgrade + "UP", + # isort + "I", + # pygrep-hooks + "PGH003", + "PGH004", + # unsorted-dunder-all + "RUF022", + # unused-noqa + "RUF100", + # flake8-pytest-style + "PT", +] + +ignore = ["PT011"] Review Comment: PR description mentions ignoring `.ipynb` files, but the Ruff configuration here doesn’t exclude notebooks. Since the repo contains notebooks (e.g. under `examples/` and `qdp/qdp-python/benchmark/notebooks/`), `ruff check` may start linting them depending on how it’s invoked. If the intent is to keep notebooks out of linting, add an explicit Ruff exclude for `*.ipynb` (or configure notebook linting intentionally and ensure they pass). ########## testing/conftest.py: ########## @@ -32,25 +32,25 @@ # Check if QDP extension is available at module load time _QDP_AVAILABLE = False -_QDP_IMPORT_ERROR: Optional[str] = "No module named '_qdp'" +_QDP_IMPORT_ERROR: str | None = "No module named '_qdp'" try: - import _qdp # noqa: F401, PLC0415 + import _qdp Review Comment: `Optional` is imported here but no longer used after switching annotations to `X | None`. Removing unused imports keeps the module clean and avoids future lint failures if `F401` (unused imports) gets enabled later. ########## pyproject.toml: ########## @@ -96,6 +96,25 @@ exclude = [ # api: benchmark module loaded via sys.path in tests allowed-unresolved-imports = ["_qdp", "_qdp.*", "api", "api.*"] +[tool.ruff.lint] +select = [ + # pyupgrade + "UP", + # isort + "I", + # pygrep-hooks + "PGH003", + "PGH004", + # unsorted-dunder-all + "RUF022", + # unused-noqa + "RUF100", + # flake8-pytest-style + "PT", +] + +ignore = ["PT011"] Review Comment: Consider setting Ruff’s `target-version` to match `requires-python` (>=3.10). Several fixes in this PR rely on Python 3.10+ (e.g., PEP 604 unions), and pinning the target version keeps `UP` behavior consistent across developer environments and CI. ########## qdp/qdp-python/qumat_qdp/loader.py: ########## @@ -29,11 +29,12 @@ from __future__ import annotations +from collections.abc import Iterator from functools import lru_cache -from typing import TYPE_CHECKING, Iterator, Optional +from typing import TYPE_CHECKING, Optional Review Comment: `Optional` is imported but not used (the module now uses `int | None` / `str | None` instead). Dropping the unused import will avoid noise and potential failures if unused-import checks are enabled. ```suggestion from typing import TYPE_CHECKING ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
