Skip to content

Added more detailed docstrings to find_element() #14930

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Dec 27, 2024
Merged

Added more detailed docstrings to find_element() #14930

merged 6 commits into from
Dec 27, 2024
+135 −20

Conversation

shbenzer
Copy link
Contributor

@shbenzer shbenzer commented Dec 23, 2024 •
edited by qodo-merge-pro bot
Loading

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

    @qodo-merge-pro Qodo Merge Pro
    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')

    @qodo-merge-pro Qodo Merge Pro
    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

    @codecov Codecov
    Copy link

    codecov bot commented Dec 27, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 59.30%. Comparing base (57f8398) to head (12490d8).
    Report is 1064 commits behind head on trunk.

    Additional details and impacted files 
    @@            Coverage Diff             @@
##            trunk   #14930      +/-   ##
==========================================
+ Coverage   58.48%   59.30%   +0.82%     
==========================================
  Files          86       94       +8     
  Lines        5270     6040     +770     
  Branches      220      268      +48     
==========================================
+ Hits         3082     3582     +500     
- Misses       1968     2190     +222     
- Partials      220      268      +48

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    diemol
    diemol requested changes Dec 27, 2024
    View reviewed changes
    Copy link
    Member

    @diemol diemol left a comment

    Choose a reason for hiding this comment

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

    Could you please run the format script?

    @shbenzer
    Copy link
    Contributor Author

    @diemol should be good now

    diemol
    diemol approved these changes Dec 27, 2024
    View reviewed changes
    Copy link
    Member

    @diemol diemol left a comment

    Choose a reason for hiding this comment

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

    Thank you, @shbenzer!

    @diemol diemol merged commit 5508e80 into SeleniumHQ:trunk Dec 27, 2024
    17 checks passed
    @shbenzer shbenzer deleted the docstrings-py branch January 16, 2025 00:38
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @diemol diemol diemol approved these changes

    Assignees
    No one assigned
    Labels
    Review effort [1-5]: 1
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @shbenzer @diemol