Skip to content

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Oct 15, 2025

User description

🔗 Related Issues

relates to #16427

💥 What does this PR do?

https://github.com/google/styleguide/blob/gh-pages/pyguide.md

This pull request focuses on improving the clarity and consistency of docstrings across several Selenium WebDriver Python modules, especially for Chromium, Chrome, Edge, and Firefox classes. The changes modernize docstring formats to follow a more standardized "Args"/"Returns"/"Example" style, making the documentation easier to read and understand for both users and contributors.

Docstring Standardization and Clarification

  • Updated constructor and method docstrings in chromium/webdriver.py and chrome/webdriver.py to use consistent "Args" and "Returns" sections, replacing older ":Args:" and ":Returns:" formats. This includes clearer descriptions and examples for parameters and return values. [1] [2]
  • Improved docstrings in chromium/options.py for properties and methods, ensuring each parameter and return value is clearly documented using the "Args" and "Returns" format. [1] [2] [3] [4] [5] [6] [7]

Edge-Specific Documentation Improvements

  • Enhanced docstrings in edge/options.py and edge/service.py to clarify initialization parameters, property purposes, and method return values, all using the updated documentation style. [1] [2] [3] [4] [5]

Other Browser-Specific Updates

  • Updated the FirefoxBinary class constructor in firefox/firefox_binary.py to use the new docstring format for parameter documentation.

Minor Enhancements

  • Added explicit docstrings to methods that previously lacked them, such as those raising NotImplementedError in chromium/webdriver.py, to clarify their current status.

These changes collectively make the codebase's documentation more accessible and maintainable, which will help both new and experienced developers understand and use these APIs more effectively.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Documentation


Description

  • Standardized Python docstrings to Google style guide format

  • Updated Args/Returns/Example sections across WebDriver modules

  • Enhanced clarity for Chromium, Chrome, Edge, and Firefox classes

  • Improved parameter descriptions and usage examples


Diagram Walkthrough

flowchart LR
  A["Old docstring format"] --> B["Google style format"]
  B --> C["Chrome/Chromium modules"]
  B --> D["Edge modules"]
  B --> E["Firefox modules"]
  C --> F["Improved Args/Returns"]
  D --> F
  E --> F
Loading

File Walkthrough

Relevant files
Documentation
webdriver.py
Standardize Chrome WebDriver docstrings                                   

py/selenium/webdriver/chrome/webdriver.py

  • Updated __init__ docstring to Google style format
  • Changed :Args: to Args: with proper indentation
  • Improved parameter descriptions clarity
+7/-6     
options.py
Refactor ChromiumOptions docstrings to Google style           

py/selenium/webdriver/chromium/options.py

  • Added __init__ docstring
  • Converted all property and method docstrings to Google style
  • Updated binary_location, debugger_address, extensions properties
  • Improved add_extension, add_encoded_extension method docs
  • Enhanced enable_webextensions with detailed notes
+38/-37 
webdriver.py
Standardize ChromiumDriver method docstrings                         

py/selenium/webdriver/chromium/webdriver.py

  • Updated __init__ to use Args: format
  • Converted launch_app, get_network_conditions, set_network_conditions
    docs
  • Improved set_permissions, execute_cdp_cmd with Example sections
  • Updated Cast-related methods and log methods
  • Added docstrings for download_file, get_downloadable_files,
    delete_downloadable_files
+53/-54 
options.py
Update Edge Options docstrings to Google style                     

py/selenium/webdriver/edge/options.py

  • Added __init__ docstring
  • Updated use_webview property with Returns section
  • Enhanced to_capabilities and default_capabilities docs
+17/-2   
service.py
Refactor Edge Service docstrings                                                 

py/selenium/webdriver/edge/service.py

  • Converted class docstring from :param: to Args: format
  • Added __init__ docstring
  • Updated service_args property with Returns and setter docs
+19/-6   
firefox_binary.py
Standardize Firefox binary docstrings                                       

py/selenium/webdriver/firefox/firefox_binary.py

  • Updated __init__ docstring from :Args: to Args: format
  • Improved parameter descriptions for firefox_path and log_file
+5/-5     
webdriver.py
Refactor Firefox WebDriver docstrings to Google style       

py/selenium/webdriver/firefox/webdriver.py

  • Updated __init__ to Google style format
  • Added docstring for set_context method
  • Converted context, install_addon, uninstall_addon docs
  • Updated screenshot methods with Args/Returns/Example sections
  • Added docstrings for unimplemented download methods
+45/-36 

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

qodo-merge-pro bot commented Oct 15, 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 <!-- /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.

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

@iampopovich
Copy link
Contributor Author

@cgoldberg one more batch of files

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 15, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Correct docstring example for network conditions

Update the example in the get_network_conditions docstring to include the
offline key for accuracy.

py/selenium/webdriver/chromium/webdriver.py [86-92]

 def get_network_conditions(self):
     """Gets Chromium network emulation settings.
 
     Returns:
-        A dict. For example: {'latency': 4, 'download_throughput': 2, 'upload_throughput': 2}
+        A dict. For example: {'offline': False, 'latency': 4, 'download_throughput': 2, 'upload_throughput': 2}
     """
     return self.execute("getNetworkConditions")["value"]
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the PR's change to the docstring for get_network_conditions made the example less accurate by removing the offline key, which is a valid part of the returned dictionary.

Low
Remove redundant information from docstring

Refactor the install_addon docstring to remove redundant information about the
return value, consolidating it into the Returns: section for clarity.

py/selenium/webdriver/firefox/webdriver.py [118-133]

 def install_addon(self, path, temporary=False) -> str:
-    """Installs Firefox addon.
-
-    Returns identifier of installed addon. This identifier can later
-    be used to uninstall addon.
+    """Installs a Firefox addon.
 
     Args:
         path: Absolute path to the addon that will be installed.
         temporary: Allows you to load browser extensions temporarily during a session.
 
     Returns:
-        Identifier of installed addon.
+        Identifier of the installed addon, which can be used to uninstall it.
 
     Example:
         driver.install_addon("/path/to/firebug.xpi")
     """

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion correctly points out redundancy in the docstring for install_addon and proposes a cleaner version that better aligns with the Google style guide, which is the goal of this PR.

Low
  • Update

Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

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

For the docstrings that don't contain a description and only have the Returns/Args sections, they need to start on the second line and have indentation, or else they get smashed into a single line when we build the docs.

For example, if you change:

    """Returns:
    The address of the remote devtools instance.
    """

to this:

    """
    Returns:
        The address of the remote devtools instance.
    """

... it will render correctly when we build the docs.

I don't know if having a blank first line breaks a ruff rule or style recommendation, but it seems to look good in the docs.

@iampopovich iampopovich requested a review from cgoldberg October 16, 2025 14:59
@iampopovich
Copy link
Contributor Author

I don't know if having a blank first line breaks a ruff rule or style recommendation, but it seems to look good in the docs.

RUFF doesn't have any available options for finding and fixing such line breaks in docstrings. It seems that formatting decisions should be made based on the docstring's semantics.

Copy link
Member

@cgoldberg cgoldberg left a comment

Choose a reason for hiding this comment

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

Thanks... those look really good. 👍

@cgoldberg cgoldberg merged commit b3fccd0 into SeleniumHQ:trunk Oct 16, 2025
22 checks passed
@iampopovich
Copy link
Contributor Author

Thanks , see you tomorrow :D

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