[py] fix D105 warn for ruff format rule

[iampopovich]

User description

🔗 Related Issues

relates #11442

💥 What does this PR do?

This pull request adds comprehensive docstrings to many magic methods and context manager methods throughout the codebase, improving code readability and maintainability. Additionally, it updates the pyproject.toml configuration to require docstrings for magic methods by removing the D105 ignore directive. The most important changes are grouped below.

Docstring Enhancements for Magic Methods and Context Managers:

• Added detailed docstrings to __eq__, __ne__, __hash__, __repr__, and other magic methods in key classes such as WebElement, Color, ScriptKey, ShadowRoot, WebDriver, and WebDriverWait, describing their behavior, arguments, and return values. [1] [2] [3] [4] [5] [6]
• Added docstrings to context manager methods (__enter__ and __exit__) in classes like WebDriver, ActionChains, and others, clarifying their purpose and usage within with statements. [1] [2]

Configuration Update:

• Removed "D105" from the extend-ignore list in pyproject.toml, enforcing the requirement for docstrings in magic methods as per the docstring linter configuration.

Improved Exception and Utility Method Documentation:

• Added docstrings to exception classes and utility methods, such as __str__ in various exception classes and protocol/container classes, to clarify their output and usage. [1] [2] [3] [4]

Dataclass and Attribute Accessor Documentation:

• Introduced docstrings for dataclass __post_init__ methods and attribute accessors, explaining validation logic and side effects. [1] [2] [3]

Event Firing Wrapper Improvements:

• Added docstrings to attribute access and mutation methods in EventFiringWebDriver and EventFiringWebElement, specifying delegation logic and exception handling. [1] [2] [3] [4]

These changes collectively improve the codebase's documentation quality, making it easier for developers to understand the behavior of core classes and methods.

💡 Additional Considerations

🔄 Types of changes

• Cleanup (formatting, renaming)

---

PR Type

Documentation

---

Description

• Add comprehensive docstrings to magic methods across codebase

• Document context manager methods (enter, exit)

• Add validation docstrings to dataclass post_init methods

• Remove D105 rule from ruff configuration to enforce magic method documentation

---

Diagram Walkthrough

flowchart LR
  A["Magic Methods<br/>(__eq__, __hash__, etc.)"] -->|"Add docstrings"| B["Enhanced Documentation"]
  C["Context Managers<br/>(__enter__, __exit__)"] -->|"Add docstrings"| B
  D["Validation Methods<br/>(__post_init__)"] -->|"Add docstrings"| B
  E["pyproject.toml"] -->|"Remove D105 ignore"| F["Enforce Documentation"]
  B --> G["Improved Code Quality"]
  F --> G

Loading

File Walkthrough

	Relevant files
Documentation	13 files conftest.py Add type hint and docstring to ContainerProtocol.__contains__ +9/-1      exceptions.py Add docstrings to exception __str__ methods                            +11/-0    action_chains.py Add docstrings to context manager methods                                +12/-0    input.py Add validation docstrings to dataclass __post_init__ methods +14/-0    service.py Add docstring to Service.__del__ cleanup method                    +7/-0      virtual_authenticator.py Add docstring to Credential.__str__ method                              +5/-0      script_key.py Add docstrings to ScriptKey comparison and repr methods    +14/-1    shadowroot.py Add docstrings to ShadowRoot magic methods                              +19/-0    webdriver.py Add docstrings to WebDriver repr and context manager methods +19/-0    webelement.py Add docstrings to WebElement comparison and hash methods  +29/-2    color.py Add docstrings to Color comparison and representation methods +33/-0    event_firing_webdriver.py Add docstrings to attribute access magic methods                  +62/-0    wait.py Add docstring to WebDriverWait.__repr__ method                      +5/-0
Configuration changes	1 files pyproject.toml Remove D105 rule from ruff extend-ignore list                        +0/-1

---

[selenium-ci]

Thank you, @iampopovich for this code suggestion.

The support packages contain example code that many users find helpful, but they do not necessarily represent
the best practices for using Selenium, and the Selenium team is not currently merging changes to them.

After reviewing the change, unless it is a critical fix or a feature that is needed for Selenium
to work, we will likely close the PR.

We actively encourage people to add the wrapper and helper code that makes sense for them to their own frameworks.
If you have any questions, please contact us

[qodo-merge-pro]

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
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No auditing: The new docstrings and minor logic do not add or modify any audit logging of critical actions, and it's unclear from the diff whether such actions are logged elsewhere. Referred Code def __del__(self) -> None: """Cleanup and terminate the service process when the object is destroyed. Attempts to gracefully stop the service process when the instance is garbage collected. `subprocess.Popen` doesn't send signals on `__del__`, so we explicitly call stop() here. Note: globals are not referenced here as they may be None during interpreter shutdown. """ # `subprocess.Popen` doesn't send signal on `__del__`;
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Empty exit: The context manager exit method is a pass with no cleanup or handling, which may miss error handling or graceful degradation if exceptions occur in with blocks. Referred Code def __exit__(self, _type, _value, _traceback) -> None: """Exit the context manager. Args: _type: The exception type if an error occurred in the with block. _value: The exception value if an error occurred in the with block. _traceback: The traceback if an error occurred in the with block. """ pass # Do nothing, does not require additional cleanup.
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Error detail exposure: The enhanced str includes message, screenshot availability, and stacktrace content which could expose internal details if surfaced to end users; usage context is not visible. Referred Code def __str__(self) -> str: """Return a formatted string representation of the exception. Returns: A formatted string containing the error message, screenshot info, and stacktrace if available. """ exception_msg = f"Message: {self.msg}\n" if self.screen: exception_msg += "Screenshot: available via screen\n" if self.stacktrace: stacktrace = "\n".join(self.stacktrace)
Update

Compliance status legend

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

[iampopovich]

@cgoldberg check please
i've added docstrings and type hints for magic method , please verify the docstrings contain correct descriptions

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect equality comparison logic Fix the eq method in ScriptKey to correctly compare with another ScriptKey instance by checking its _id attribute, while retaining comparison with raw ID values. py/selenium/webdriver/remote/script_key.py [29-38] def __eq__(self, other) -> bool: """Compare this ScriptKey with another object for equality. Args: other: The object to compare with. Returns: True if the script key ID equals the other object, False otherwise. """ + if isinstance(other, ScriptKey): + return self._id == other._id return self._id == other Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a logical flaw in the __eq__ method where a ScriptKey object would be incorrectly compared to a UUID, and provides a robust fix.	Medium
Learned best practice	Add context manager for cleanup Do not depend on del for cleanup; provide and document an explicit context manager so the process is stopped deterministically. py/selenium/webdriver/common/service.py [192-208] +def __enter__(self): + # Allow using Service in a with-statement for deterministic cleanup + return self + +def __exit__(self, exc_type, exc, tb): + try: + self.stop() + except Exception: + # Swallow exceptions to mirror current __del__ behavior + pass + def __del__(self) -> None: - """Cleanup and terminate the service process when the object is destroyed. - - Attempts to gracefully stop the service process when the instance is - garbage collected. `subprocess.Popen` doesn't send signals on `__del__`, - so we explicitly call stop() here. Note: globals are not referenced here - as they may be None during interpreter shutdown. - """ - # `subprocess.Popen` doesn't send signal on `__del__`; - # so we attempt to close the launched process when `__del__` - # is triggered. - # do not use globals here; interpreter shutdown may have already cleaned them up - # and they would be `None`. This goes for anything this method is referencing internally. + # Fallback only; prefer explicit stop() or context manager use. try: self.stop() except Exception: pass Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Ensure resource cleanup and cancellation using context managers/try-finally; avoid relying on del for critical cleanup.	Low
Update

[cgoldberg]

The changes look fine, but I'd prefer we leave this rule ignored and not add all these docstrings. I think it's kind of ridiculous to have docstrings on every dunder method that's overridden from standard library classes. Everyone familiar with Python knows exactly what they do and this just adds noise to the code.

Forcing you to include a docstring on every __str__ method stating that it returns a string representation of the object doesn't add any value... That's what every __str__ method does on every single object.

@navin772 @shbenzer any thoughts?

[navin772]

@navin772 @shbenzer any thoughts?

Yup, we shouldn't have docstrings for such trivial dunder methods.

[iampopovich]

Hmm
on the other hand:

• dunder methods aren't added frequently - if one magic method gets added, it won't be difficult to write a docstring for it
• some dunder methods like str add additional details to the string representation, which might not be obvious without a docstring until the moment of use
• Adding a method description is also easy with an LLM. In my humble opinion, we have two choices: either ignore all warnings about undocumented methods, classes, and packages, or agree to follow this rule with minimal effort by leveraging LLM autocomplete.

What's your opinion?

[cgoldberg]

I think we should ignore all missing docstrings... but if they are added, they should follow our formatting rules.

[iampopovich]

well, let's close this PR and ignore codes

    "D100", # Missing docstring in public module
    "D101", # Missing docstring in public class
    "D102", # Missing docstring in public method
    "D103", # Missing docstring in public function
    "D104", # Missing docstring in public package
    "D105", # Missing docstring in magic method
    "D107", # Missing docstring in `__init__`