Skip to content

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Oct 31, 2025

User description

🔗 Related Issues

relates to #11442 and probably completes #16432

💥 What does this PR do?

🔧 Implementation Notes

This pull request primarily refines and standardizes docstrings across several Python modules, improving clarity and consistency in documentation. Additionally, minor code style improvements are made, such as explicit return type annotations and configuration updates. The changes are grouped below by theme.

Docstring and Documentation Improvements

  • Shortened and clarified docstrings for exception classes in py/selenium/common/exceptions.py, making them more concise and easier to read. [1] [2] [3] [4] [5] [6] [7] [8] [9]
  • Updated and condensed docstrings for service, driver, and options classes and methods in py/selenium/webdriver/chrome/service.py, py/selenium/webdriver/chromium/service.py, py/selenium/webdriver/chromium/webdriver.py, and py/selenium/webdriver/chromium/options.py. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13]
  • Improved docstrings for ActionChains and related methods in py/selenium/webdriver/common/action_chains.py, enhancing clarity for automation actions. [1] [2] [3]
  • Refined the script-level docstring in py/generate_api_module_listing.py for better readability and structure.

Code Style and Typing Improvements

  • Added explicit return type annotations to property methods in py/generate.py for better type safety and code clarity. [1] [2] [3]

Configuration Updates

  • Removed the D205 docstring linting rule from py/pyproject.toml, aligning linting configuration with the new docstring style.

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Documentation, Enhancement


Description

  • Standardize and shorten docstrings across Python modules for clarity

  • Add explicit return type annotations to methods and properties

  • Improve docstring formatting with proper blank lines and structure

  • Refactor exception class docstrings for conciseness

  • Update configuration to remove D205 ruff rule for docstring formatting


Diagram Walkthrough

flowchart LR
  A["Docstring Refactoring"] --> B["Shortened & Clarified"]
  A --> C["Added Blank Lines"]
  D["Type Annotations"] --> E["Return Types Added"]
  F["Configuration"] --> G["Removed D205 Rule"]
  B --> H["Improved Readability"]
  C --> H
  E --> H
Loading

File Walkthrough

Relevant files
Documentation
50 files
webdriver.py
Add return types and refactor docstrings                                 
+77/-72 
expected_conditions.py
Shorten and clarify expectation condition docstrings         
+23/-52 
exceptions.py
Condense exception class docstrings                                           
+15/-42 
action_chains.py
Improve ActionChains docstring formatting                               
+19/-17 
options.py
Simplify property docstrings                                                         
+8/-27   
webelement.py
Refactor WebElement method docstrings                                       
+14/-16 
webdriver.py
Clarify Firefox driver docstrings                                               
+13/-15 
select.py
Improve Select class docstring clarity                                     
+8/-12   
webdriver.py
Shorten ChromiumDriver docstrings                                               
+5/-15   
firefox_profile.py
Condense profile-related docstrings                                           
+4/-11   
event_firing_webdriver.py
Refactor EventFiringWebDriver docstrings                                 
+6/-10   
relative_locator.py
Improve RelativeBy class docstrings                                           
+5/-8     
options.py
Enhance options class docstrings                                                 
+8/-7     
options.py
Clarify Firefox options docstrings                                             
+8/-9     
virtual_authenticator.py
Shorten decorator docstrings                                                         
+3/-6     
options.py
Add return types and simplify docstrings                                 
+5/-13   
switch_to.py
Improve switch_to method docstrings                                           
+6/-6     
service.py
Refactor Service class docstrings                                               
+8/-7     
generate_api_module_listing.py
Improve module docstring formatting                                           
+7/-6     
service.py
Shorten Edge service docstring                                                     
+2/-6     
browsing_context.py
Improve BrowsingContext method docstrings                               
+4/-3     
page_loader.py
Refactor test module docstring                                                     
+4/-4     
remote_connection_tests.py
Improve test docstring formatting                                               
+7/-6     
log.py
Clarify Log class docstrings                                                         
+2/-4     
options.py
Add return types and simplify docstrings                                 
+3/-7     
options.py
Improve Edge options docstrings                                                   
+4/-2     
firefox_binary.py
Shorten binary path method docstring                                         
+1/-3     
service.py
Improve Safari service docstring                                                 
+3/-2     
service.py
Shorten Firefox service docstring                                               
+1/-2     
interactions_with_device_tests.py
Condense test docstring                                                                   
+1/-3     
interactions_tests.py
Condense test docstring                                                                   
+1/-3     
service.py
Shorten ChromiumService docstring                                               
+1/-2     
service.py
Maintain WPEWebKit service docstring                                         
+1/-2     
file_detector.py
Shorten FileDetector docstring                                                     
+1/-3     
webdriver.py
Shorten IE driver docstring                                                           
+1/-3     
service.py
Maintain WebKitGTK service docstring                                         
+1/-2     
service.py
Shorten Chrome service docstring                                                 
+1/-2     
remote_connection.py
Improve certificate bundle docstring                                         
+3/-3     
driver_finder.py
Shorten DriverFinder docstring                                                     
+1/-2     
webdriver.py
Shorten WPEWebKit quit method docstring                                   
+1/-3     
webdriver.py
Shorten WebKitGTK quit method docstring                                   
+1/-3     
proxy.py
Shorten Proxy class docstring                                                       
+1/-3     
webdriverwait_tests.py
Improve test docstring formatting                                               
+4/-2     
print_page_options.py
Improve print options docstring                                                   
+2/-1     
options.py
Improve IE options docstrings                                                       
+4/-2     
webdriver.py
Shorten Safari driver docstring                                                   
+1/-2     
utils.py
Improve URL connectivity check docstring                                 
+1/-2     
shadowroot.py
Maintain shadow root find element docstring                           
+1/-2     
errorhandler.py
Improve error handler docstring                                                   
+1/-2     
webserver.py
Improve test webserver module docstring                                   
+1/-0     
Enhancement
1 files
generate.py
Add return type annotations to properties                               
+3/-3     
Configuration changes
1 files
pyproject.toml
Remove D205 ruff rule from configuration                                 
+0/-1     

@selenium-ci selenium-ci added C-py Python Bindings B-devtools Includes everything BiDi or Chrome DevTools related B-support Issue or PR related to support classes labels Oct 31, 2025
@selenium-ci
Copy link
Member

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

@iampopovich
Copy link
Contributor Author

@cgoldberg
I opened a draft pull request to make it clear that I'm still reviewing the changes myself. I reassembled some docstrings in a shorter form so they fit in the one-liner. I'm willing to correct any comments in the review.

Copy link
Contributor Author

@iampopovich iampopovich left a comment

Choose a reason for hiding this comment

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

i've finished self-review

@iampopovich iampopovich marked this pull request as ready for review November 1, 2025 18:10
@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 1, 2025

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: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

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

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

@iampopovich
Copy link
Contributor Author

the last one commit fixes some leftovers after self-review. no more changes from me in this branch

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Nov 1, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Consider reverting some over-simplified docstrings

Review the docstring modifications, especially in
py/selenium/common/exceptions.py. Some changes, while making the text more
concise, have removed valuable context and details, potentially harming
documentation quality.

Examples:

py/selenium/common/exceptions.py [210-213]
    """Thrown when the selector used to find an element does not return a WebElement.

    Currently this only happens when the XPath expression is syntactically invalid or does not select WebElements.
    """
py/selenium/common/exceptions.py [244]
    """Thrown when no cookie matching the given path name was found."""

Solution Walkthrough:

Before:

class InvalidSelectorException(WebDriverException):
    """Thrown when the selector which is used to find an element does not
    return a WebElement.

    Currently this only happens when the selector is an xpath expression
    and it is either syntactically invalid (i.e. it is not a xpath
    expression) or the expression does not select WebElements (e.g.
    "count(//input)").
    """
    ...

class NoSuchCookieException(WebDriverException):
    """No cookie matching the given path name was found amongst the associated
    cookies of the current browsing context's active document.
    """
    ...

After:

class InvalidSelectorException(WebDriverException):
    """Thrown when the selector used to find an element does not return a WebElement.

    Currently this only happens when the selector is an xpath expression
    and it is either syntactically invalid (i.e. it is not a xpath
    expression) or the expression does not select WebElements (e.g.
    "count(//input)").
    """
    ...

class NoSuchCookieException(WebDriverException):
    """Thrown when no cookie matching the given path name was found amongst the
    associated cookies of the current browsing context's active document.
    """
    ...
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that in the pursuit of conciseness, some docstrings, particularly in py/selenium/common/exceptions.py, have lost important details and clarifying examples, which impacts documentation quality.

Medium
Possible issue
Improve new window detection logic

Improve the new_window_is_opened function by using set operations instead of
len() to robustly detect new windows, and update the type hint for
current_handles to set[str].

py/selenium/webdriver/support/expected_conditions.py [673-681]

-def new_window_is_opened(current_handles: list[str]) -> Callable[[WebDriver], bool]:
-    """An expectation that a new window will be opened and have the number of
-    windows handles increase.
+def new_window_is_opened(current_handles: set[str]) -> Callable[[WebDriver], bool]:
+    """Check that a new window has been opened (window handles count increased).
 
     Args:
         current_handles: The current window handles.
 
     Returns:
         True when a new window is opened, False otherwise.
 
     Example:
         >>> from selenium.webdriver.support.ui import By
         >>> from selenium.webdriver.support.ui import WebDriverWait
         >>> from selenium.webdriver.support import expected_conditions as EC
-        >>> is_new_window_opened = WebDriverWait(driver, 10).until(EC.new_window_is_opened(driver.window_handles))
+        >>> is_new_window_opened = WebDriverWait(driver, 10).until(EC.new_window_is_opened(set(driver.window_handles)))
     """
 
     def _predicate(driver: WebDriver):
-        return len(driver.window_handles) > len(current_handles)
+        return set(driver.window_handles) > current_handles
 
     return _predicate

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential edge case where simply comparing handle counts is insufficient and proposes a more robust implementation using set operations.

Medium
Learned
best practice
Narrow exception handling scope

Narrow the exception handling to expected network errors so unrelated issues
aren't silently masked.

py/selenium/webdriver/common/utils.py [150-154]

 try:
     with urllib.request.urlopen(f"{scheme}://{host}:{port}/status") as res:
         return res.getcode() == 200
-except Exception:
+except (urllib.error.URLError, TimeoutError, ConnectionError, OSError):
     return False

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Ensure resource cleanup by using context managers for network I/O to avoid leaking resources on exceptions.

Low
  • More

@iampopovich iampopovich changed the title [py] Fix D205 ruf fwarns for python package formatting [py] Fix D205 ruff warns for python package formatting Nov 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related B-support Issue or PR related to support classes C-py Python Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants