Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added more detailed docstrings to find_element() #14930

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Dec 23, 2024

User description

Added more detailed doc strings to find_element() per #14746

Description

added more detailed docstrings to find_element() in 3 files

Motivation and Context

#14746

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Documentation


Description

  • Enhanced documentation for element finding methods across three core Selenium Python classes:
    • WebDriver
    • WebElement
    • ShadowRoot
  • Modernized docstring format to include detailed sections for:
    • Parameters with comprehensive descriptions of all locator strategies
    • Return values with clear type information
    • Usage examples
  • Replaced old-style docstrings with more detailed and structured documentation
  • Improved developer experience by providing clearer guidance on element location methods

Changes walkthrough 📝

Relevant files
Documentation
shadowroot.py
Enhanced docstrings for shadow root element finding methods

py/selenium/webdriver/remote/shadowroot.py

  • Added detailed docstrings to find_element() and find_elements()
    methods
  • Documented all supported By locator strategies and their usage
  • Added parameters section with detailed descriptions
  • Added example usage and return value documentation
  • +50/-0   
    webdriver.py
    Modernized docstrings for WebDriver element finding methods

    py/selenium/webdriver/remote/webdriver.py

  • Updated docstrings for find_element() and find_elements() methods
  • Replaced old-style docstrings with modern format including Parameters
    and Returns sections
  • Added comprehensive list of supported By locator strategies
  • Added clear examples of method usage
  • +42/-10 
    webelement.py
    Updated docstrings for WebElement element finding methods

    py/selenium/webdriver/remote/webelement.py

  • Enhanced docstrings for find_element() and find_elements() methods
  • Updated documentation format to include detailed parameters and return
    values
  • Added comprehensive list of supported locator strategies
  • Improved example usage documentation
  • +42/-10 

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Incorrect Example

    The example in find_elements() shows find_element() usage instead of find_elements(). Should show plural usage with a list of elements.

            element = driver.find_element(By.ID, 'foo')

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Fix incorrect return type documentation to properly indicate a list return type

    The return type description for find_elements() incorrectly states "WebElement"
    instead of "List[WebElement]". Update to accurately reflect that it returns a list.

    py/selenium/webdriver/remote/shadowroot.py [103-104]

     Returns:
         -------
    -    WebElement
    -        list of `WebElements` matching locator strategy found on the page.
    +    List[WebElement]
    +        List of `WebElement` objects matching the locator strategy found on the page.
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion addresses an important documentation error where the return type is incorrectly documented as WebElement instead of List[WebElement], which could lead to incorrect usage of the API.

    8
    Update example code to use correct class context instead of incorrect driver reference

    The example in the docstring shows driver.find_element() but should show
    shadowroot.find_element() since this is a ShadowRoot class method. Update the
    example to reflect the correct context.

    py/selenium/webdriver/remote/shadowroot.py [59-61]

     Example:
         --------
    -    element = driver.find_element(By.ID, 'foo')
    +    element = shadowroot.find_element(By.ID, 'foo')
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion correctly identifies a misleading example that could confuse users. Using driver.find_element() in ShadowRoot's documentation is incorrect and should be shadowroot.find_element() to accurately reflect the class context.

    7

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

    Successfully merging this pull request may close these issues.

    1 participant