Skip to content

Conversation

yvsvarma
Copy link
Contributor

@yvsvarma yvsvarma commented Oct 15, 2025

User description

Summary

Goal: Add Ruff configuration to lint Python docstrings using the Google convention.
Issues: #16432, refs #11442


What’s in this PR

  • Ruff docstrings enabled: add D codes to extend-select.
  • Transitional ignores: add a focused extend-ignore list so existing code won’t fail immediately.
  • Convention: set pydocstyle convention to google.

Why

  • Consistency: enforce a clear, automated standard for docstrings across the Python package.
  • Scalability: allow gradual enforcement by tightening ignores as we fix legacy docs.

Scope / Impact

  • Lint-only: configuration change; no runtime or public API impact.


PR Type

Enhancement


Description

  • Enable Ruff docstring linting with Google convention for Python package

  • Add transitional ignore rules for 17 docstring violations to prevent immediate failures

  • Configure pydocstyle convention to enforce Google-style docstrings

  • Extend Ruff lint selection to include all "D" (docstring) rules


Diagram Walkthrough

flowchart LR
  A["pyproject.toml"] --> B["Add 'D' to extend-select"]
  B --> C["Configure pydocstyle convention='google'"]
  C --> D["Add 17 transitional ignore rules"]
Loading

File Walkthrough

Relevant files
Configuration changes
pyproject.toml
Configure Ruff for Google-style docstring linting               

py/pyproject.toml

  • Enable Ruff docstring linting by adding "D" codes to extend-select
  • Add 17 transitional extend-ignore rules for existing docstring
    violations
  • Configure pydocstyle convention to "google" for Google-style
    docstrings
+26/-1   

@selenium-ci selenium-ci added the C-py Python Bindings label Oct 15, 2025
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Reconsider globally ignoring missing docstring rules

The PR globally disables rules for missing docstrings (D100-D107), allowing new
code to lack documentation. Instead, these rules should be enabled, and existing
violations should be ignored using a baseline or # noqa comments to enforce
docstrings on new code.

Examples:

py/pyproject.toml [149-156]
extend-ignore = [
    "D103", # Missing docstring in public function
    "D102", # Missing docstring in public method
    "D100", # Missing docstring in public module
    "D205", # 1 blank line required between summary line and description
    "D107", # Missing docstring in `__init__`
    "D101", # Missing docstring in public class
    "D104", # Missing docstring in public package

Solution Walkthrough:

Before:

# py/pyproject.toml

[tool.ruff.lint]
extend-select = [..., "D"]
extend-ignore = [
    "D103", # Missing docstring in public function
    "D102", # Missing docstring in public method
    "D100", # Missing docstring in public module
    "D107", # Missing docstring in `__init__`
    "D101", # Missing docstring in public class
    # ... and other rules
]

[tool.ruff.lint.pydocstyle]
convention = "google"

After:

# py/pyproject.toml

# D100-D107 rules are no longer globally ignored.
# Existing violations should be handled via a baseline file
# or inline `# noqa` comments, generated by `ruff --add-noqa`.

[tool.ruff.lint]
extend-select = [..., "D"]
extend-ignore = [
    "D205", # 1 blank line required between summary line and description
    "D209", # Multi-line docstring closing quotes should be on a separate line
    # ... other rules, but not D100-D107
]

[tool.ruff.lint.pydocstyle]
convention = "google"
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical flaw in the PR's strategy; globally ignoring missing docstring rules (D100-D107) prevents enforcement on new code, undermining the PR's primary goal.

High
Possible issue
Enforce raw docstrings for correctness

Remove the D301 rule from the ignore list to enforce the use of raw string
literals for docstrings that contain backslashes, preventing incorrect
rendering.

py/pyproject.toml [164]

-"D301", # Use `r\"\"\"` if any backslashes in a docstring
+# "D301", # Use `r\"\"\"` if any backslashes in a docstring
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: This is a valid suggestion that improves documentation quality by preventing potential rendering issues from unescaped backslashes, which is a subtle but important correctness issue.

Low
General
Disallow empty placeholder docstrings

Remove the D419 rule from the ignore list to disallow empty docstrings,
encouraging developers to either provide meaningful documentation or remove the
placeholders.

py/pyproject.toml [167]

-"D419", # Docstring is empty
+# "D419", # Docstring is empty
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: This is a good suggestion for improving code quality by flagging empty docstrings, which are often unhelpful placeholders and add noise to the code.

Low
  • More

@cgoldberg
Copy link
Member

The feature request states that this should be done once all docstrings are fixed.

@cgoldberg cgoldberg closed this Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants