Skip to content

Replace pylint and isort with ruff linter#1251

Open
DhavalGojiya wants to merge 5 commits into
meilisearch:mainfrom
DhavalGojiya:feat/ruff-linter
Open

Replace pylint and isort with ruff linter#1251
DhavalGojiya wants to merge 5 commits into
meilisearch:mainfrom
DhavalGojiya:feat/ruff-linter

Conversation

@DhavalGojiya

@DhavalGojiya DhavalGojiya commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Description

Replace Pylint and isort with Ruff for linting.

  • Remove stale pylint comments and references from the codebase
  • Add commonly used Ruff rule sets, including isort, Pylint, Pyflakes, and pyupgrade, and apply them in the codebase
  • Remove pylint from tox.ini.
  • Update the CI pipeline to use Ruff instead of Pylint and isort
  • Update CONTRIBUTING.md

Tests

% uv run --python=3.10 pytest -q
image

PR checklist

Please check if your PR fulfills the following requirements:

  • Did you use any AI tool while implementing this PR (code, tests, docs, etc.)? If yes, disclose it in the PR description and describe what it was used for. AI usage is allowed when it is disclosed.
  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Summary by CodeRabbit

  • Refactor
    • Modernized type annotations across the library using Python 3.10+ syntax, aligning public method signatures.
    • Updated a deprecation warning to report the more relevant caller.
  • Chores
    • Switched linting/formatting workflow and project guidance to Ruff, removing Pylint/Isort-based steps.
    • Updated tox/CI to run the updated lint/format checks.
  • Tests
    • Strengthened error expectations to use more specific exception types.
    • Improved assertion rigor (e.g., stricter comparisons) and warning behavior; added coverage for searchable attributes.
  • Documentation
    • Updated development instructions to use Ruff commands.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2f01ea51-435b-4f25-9294-6b31f3abb667

📥 Commits

Reviewing files that changed from the base of the PR and between 2d7c94b and 06c3da6.

📒 Files selected for processing (1)
  • .github/workflows/tests.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • .github/workflows/tests.yml

📝 Walkthrough

Walkthrough

Replaces Pylint and isort with Ruff for linting and formatting across CI, project config, and developer docs. It also modernizes type annotations in source and model modules, removes pylint directives, and updates tests to use stricter exception and value assertions.

Changes

Ruff Migration and Type Annotation Modernization

Layer / File(s) Summary
CI, config, and docs
pyproject.toml, tox.ini, .github/workflows/tests.yml, CONTRIBUTING.md, docs/conf.py
Updates lint dependencies and Ruff configuration, changes CI to run Ruff lint and format checks separately, revises contributor commands, and type-annotates html_static_path.
Core module typing
meilisearch/config.py, meilisearch/_utils.py, meilisearch/errors.py, meilisearch/_httprequests.py, meilisearch/task.py
Replaces typing aliases with PEP 604 unions and built-in generics, and moves collection typing imports to collections.abc in core modules.
Client and index API typing
meilisearch/client.py, meilisearch/index.py, meilisearch/__init__.py
Updates Client and Index signatures, return types, helpers, and warning attribution to use modern typing forms, while removing file-level pylint directives.
Pydantic model typing
meilisearch/models/document.py, meilisearch/models/embedders.py, meilisearch/models/index.py, meilisearch/models/key.py, meilisearch/models/task.py, meilisearch/models/webhook.py
Modernizes model field and validator annotations across documents, embedders, index metadata, keys, tasks, and webhooks.
Test assertions and cleanup
tests/__init__.py, tests/conftest.py, tests/client/*, tests/errors/*, tests/index/*, tests/models/*, tests/settings/*
Removes pylint directives, narrows exception expectations, adds stricter assertions and searchable-attributes coverage, and updates helpers and fixtures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • sanders41
  • Strift

🐇 I hopped through types and lint,
Ruff now purrs without a hint.
No Pylint grumbles in the night,
Just clean checks and code that's right.
Built-ins, unions, tidy trails—
Carrot-bright, and no more fails!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 77.49% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: replacing Pylint and isort with Ruff for linting.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot added the maintenance Anything related to maintenance (CI, tests, refactoring...) label Jun 23, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/client/test_client_tenant_token.py (1)

130-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

The “no api key” test is currently asserting the wrong failure path.

This test now triggers a missing-argument TypeError (no api_key_uid) instead of exercising the intended missing-api-key validation in generate_tenant_token.

Proposed fix
-def test_generate_tenant_token_with_no_api_key(client):
+def test_generate_tenant_token_with_no_api_key(get_private_key):
     """Tests create a tenant token with no api key."""
     client = meilisearch.Client(BASE_URL)

-    with pytest.raises(TypeError):
-        client.generate_tenant_token(search_rules=["*"])
+    with pytest.raises(ValueError):
+        client.generate_tenant_token(api_key_uid=get_private_key.uid, search_rules=["*"])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/client/test_client_tenant_token.py` around lines 130 - 135, The test
test_generate_tenant_token_with_no_api_key is missing the required api_key_uid
parameter when calling client.generate_tenant_token(), which causes a TypeError
for the missing argument instead of testing the intended missing-api-key
validation. Add the api_key_uid parameter to the generate_tenant_token() call
with a valid test value (like a non-empty string) so the function executes far
enough to reach and validate the missing API key condition in the client that
was initialized without an API key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/index/test_index_search_meilisearch.py`:
- Line 12: The assertions at lines 12, 446, and 459 are comparing a string
directly to the response dict (e.g., assert "hitsPerPage" != response), which
doesn't properly validate key absence and allows incorrect passes. Replace these
assertions with membership checks using the `not in` operator to verify that the
key is not present in the response dict. For example, change the comparison
pattern to check if the string key is not in the response dictionary.

In `@tox.ini`:
- Line 2: The envlist configuration is missing Ruff for linting, causing the
default tox run to skip lint checks even though documentation describes linting
as part of tox execution. Either add a Ruff environment configuration section to
tox.ini (defining how to run ruff checks) and include it in the envlist on line
2, or update the project documentation to accurately reflect that the default
tox run only performs type checking and testing without linting.

---

Outside diff comments:
In `@tests/client/test_client_tenant_token.py`:
- Around line 130-135: The test test_generate_tenant_token_with_no_api_key is
missing the required api_key_uid parameter when calling
client.generate_tenant_token(), which causes a TypeError for the missing
argument instead of testing the intended missing-api-key validation. Add the
api_key_uid parameter to the generate_tenant_token() call with a valid test
value (like a non-empty string) so the function executes far enough to reach and
validate the missing API key condition in the client that was initialized
without an API key.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bb75ce00-d167-44cd-a038-065f4d10b206

📥 Commits

Reviewing files that changed from the base of the PR and between 1464823 and db87649.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (49)
  • .github/workflows/tests.yml
  • CONTRIBUTING.md
  • meilisearch/__init__.py
  • meilisearch/_httprequests.py
  • meilisearch/_utils.py
  • meilisearch/client.py
  • meilisearch/config.py
  • meilisearch/errors.py
  • meilisearch/index.py
  • meilisearch/models/document.py
  • meilisearch/models/embedders.py
  • meilisearch/models/index.py
  • meilisearch/models/key.py
  • meilisearch/models/task.py
  • meilisearch/models/webhook.py
  • meilisearch/task.py
  • pyproject.toml
  • tests/__init__.py
  • tests/client/test_chat_completions.py
  • tests/client/test_client.py
  • tests/client/test_client_dumps.py
  • tests/client/test_client_exports.py
  • tests/client/test_client_key_meilisearch.py
  • tests/client/test_client_swap_meilisearch.py
  • tests/client/test_client_task_meilisearch.py
  • tests/client/test_client_tenant_token.py
  • tests/client/test_clinet_snapshots.py
  • tests/client/test_multimodal.py
  • tests/client/test_version.py
  • tests/conftest.py
  • tests/errors/test_api_error_meilisearch.py
  • tests/errors/test_communication_error_meilisearch.py
  • tests/errors/test_timeout_error_meilisearch.py
  • tests/index/test_index.py
  • tests/index/test_index_document_meilisearch.py
  • tests/index/test_index_facet_search_meilisearch.py
  • tests/index/test_index_search_meilisearch.py
  • tests/index/test_index_task_meilisearch.py
  • tests/index/test_index_wait_for_task.py
  • tests/models/test_document.py
  • tests/settings/test_settings.py
  • tests/settings/test_settings_displayed_attributes_meilisearch.py
  • tests/settings/test_settings_embedders.py
  • tests/settings/test_settings_filterable_attributes_meilisearch.py
  • tests/settings/test_settings_localized_attributes_meilisearch.py
  • tests/settings/test_settings_searchable_attributes_meilisearch.py
  • tests/settings/test_settings_sortable_attributes_meilisearch.py
  • tests/settings/test_settings_typo_tolerance_meilisearch.py
  • tox.ini
💤 Files with no reviewable changes (14)
  • tests/errors/test_communication_error_meilisearch.py
  • tests/settings/test_settings_sortable_attributes_meilisearch.py
  • tests/settings/test_settings_displayed_attributes_meilisearch.py
  • tests/client/test_clinet_snapshots.py
  • tests/client/test_client_dumps.py
  • tests/client/test_client_swap_meilisearch.py
  • tests/index/test_index_facet_search_meilisearch.py
  • tests/models/test_document.py
  • tests/settings/test_settings_searchable_attributes_meilisearch.py
  • tests/client/test_version.py
  • tests/client/test_client.py
  • tests/errors/test_api_error_meilisearch.py
  • tests/settings/test_settings_filterable_attributes_meilisearch.py
  • tests/index/test_index_wait_for_task.py

Comment thread tests/index/test_index_search_meilisearch.py Outdated
Comment thread tox.ini
…n` operator

- These assertions always passed without actually checking anything
- Use `not in` so they properly verify the key is absent from the response

@sanders41 sanders41 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In general this looks good to me. I just have a question about linting in CI because the output looks off to me (detailed in the review).

Comment thread .github/workflows/tests.yml Outdated
Comment thread tests/index/test_index_search_meilisearch.py Outdated
Use `--output-format=full` so ruff check prints complete diagnostics
with source context, making linter failures easier to debug in CI logs.
@DhavalGojiya DhavalGojiya requested a review from sanders41 June 24, 2026 06:24

@sanders41 sanders41 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good!

@Strift we need to remove pylint from the required checks for this. It is covered by the ruff check now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Anything related to maintenance (CI, tests, refactoring...)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants