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

Updates the py api doc build #14173

Merged
merged 8 commits into from
Jun 23, 2024
Merged

Conversation

emanlove
Copy link
Contributor

@emanlove emanlove commented Jun 22, 2024

User description

Description

These changes regenerate the api stub files - those template files which give the base structure for the api docs - each time instead of using outdated checked in version from previous builds. Essentially we treat these like new builds which need to be regenerated versus updating and storing old build artifacts.

The base API template (py/docs/source/api.rst) has been updated to match current high level python modules which the api doc is built from. Ideally this should be autogenerated itself simply matching the Python codebase. Leaving that for another body of work to do sometime in the future.

Also updated the versions of the documentation tool (sphinx) and template packages (jinja) to more recent version. (Due to using Python 3.8 the latest sphinx version can't be used but this is ok.).

Motivation and Context

The API changes with each release as it matches the code. So essentially each time a new release comes out a new build of the documentation should be generated.

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

Bug fix, Enhancement


Description

  • Updated Sphinx version in py/docs/requirements.txt from 1.8.2 to 7.1.2 to support Python 3.8.
  • Enhanced py/docs/source/api.rst by adding new modules and reordering them alphabetically.
  • Modified py/tox.ini to use docs/requirements.txt for dependencies, added a step to regenerate autodoc stub pages, and set the PYTHONPATH environment variable.
  • Removed previously generated API doc stub files to ensure they are auto-generated during the build process.

Changes walkthrough 📝

Relevant files
Dependencies
requirements.txt
Update Sphinx version in documentation requirements           

py/docs/requirements.txt

  • Updated Sphinx version from 1.8.2 to 7.1.2.
+1/-1     
Documentation
api.rst
Update API documentation structure and modules                     

py/docs/source/api.rst

  • Added new modules to the API documentation.
  • Reordered modules alphabetically.
  • +36/-14 
    Configuration changes
    tox.ini
    Update tox configuration for documentation build                 

    py/tox.ini

  • Updated to use docs/requirements.txt for dependencies.
  • Added command to regenerate autodoc stub pages.
  • Set PYTHONPATH environment variable.
  • +9/-4     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    For the API documentation as sphinx autogenerates the
    api docs from the code it needs the dependent Python
    packages. This change adds those to the tox docs build
    environment.
    Bring Sphinx version up to the latest Python 3.8 suported version,
    v7.1.2. This change was made within py/docs/requirements.txt. It
    should be noted that there is a seperate version requirement given
    within py/tox.ini which I need to test and see which one is grabed
    using the selenium build process. (I supsect it will be the one
    within py/tox.ini.)
    Instead of using the commited stub files which contain "pre-compiled"
    outlines for the api doc this will regenrate them. It should be noted
    this might add some time to the build process so a future change might
    be to see how store and update either commited files or previous
    build artifacts.
    
    I also and working though the output directory which is currently set to
    `-o docs/source`. This seem to dump all the sub files into the main source
    directory insted of neatly organied into child directories. Going to try
    to reowrk this but wanted to commit so as not to lose how I did this the
    first time ;)
    Credit goes to @iampopovich for these changes
    Credit goes to @iampopovich for these changes
    Prefer instead to auto-generate these as part of the build.
    Also ignore those sub-directories.
    - Use the docs/requirements.txt file instead of repeating the build
      versions for jinja and sphinx
    - Removed the output directory option on sphinx-autogen as that was
      putting everything in a flat directory structure at the root instead
      of the well organized folders
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Documentation Structure:
    The PR involves significant changes to the documentation structure, including the removal of several .rst files and the addition of new modules in the API documentation. It's crucial to ensure that all new modules are correctly documented and that no essential information is lost in the transition.
    Sphinx Version Compatibility:
    The update to a newer Sphinx version (from 1.8.2 to 7.1.2) may introduce changes in how documents are built or displayed. It's important to verify that the new Sphinx version is fully compatible with the existing documentation setup and that the documentation builds without errors or significant changes in formatting.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure all new modules are correctly linked and documented

    Verify that all new modules added to the api.rst file are correctly linked and documented
    in their respective .rst files to ensure proper documentation generation.

    py/docs/source/api.rst [79-103]

    +selenium.webdriver.chrome.remote_connection
    +selenium.webdriver.chromium.service
    +selenium.webdriver.edge.remote_connection
     
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: This suggestion is crucial for ensuring that the documentation is complete and functional, preventing broken links and ensuring that all new modules are accessible and well-documented.

    8
    Maintainability
    Ensure consistent formatting and alignment for new module entries

    Ensure that all new modules added to the api.rst file are correctly formatted and aligned
    with the existing entries to maintain consistency and readability.

    py/docs/source/api.rst [27-37]

    +selenium.webdriver.common.driver_finder
    +selenium.webdriver.common.options
    +selenium.webdriver.common.selenium_manager
    +selenium.webdriver.common.utils
    +selenium.webdriver.common.virtual_authenticator
     
    -
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion is relevant as it addresses the maintainability and readability of the documentation by ensuring new entries are well-formatted and aligned. This is important for large documentation files to remain manageable and user-friendly.

    7
    Possible issue
    Add a check to ensure sphinx-autogen runs successfully before sphinx-build

    Add a check to ensure that the sphinx-autogen command runs successfully before proceeding
    to the sphinx-build command to prevent build failures.

    py/tox.ini [10-14]

     commands = 
       ; regenerate autodoc stub pages
    -  sphinx-autogen docs/source/api.rst
    +  sphinx-autogen docs/source/api.rst || { echo 'sphinx-autogen failed'; exit 1; }
       ; build api docs 
       sphinx-build -b html -d ../build/docs/doctrees docs/source ../build/docs/api/py {posargs}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Adding a check for the successful execution of sphinx-autogen before proceeding with sphinx-build is a good practice to avoid build failures. This suggestion enhances the robustness of the build process.

    6

    @emanlove
    Copy link
    Contributor Author

    If one wants to easily test or reproduce the steps this is doing, the following commands will setup an virtualenv, grab the latest code and run the doc generation through tox; similar to what the official build does.

    virtualenv test-py38-env
    source test-py38-env/bin/activate
    pip install tox
    git clone git@github.com:emanlove/selenium.git se-api-docs-test
    cd se-api-docs-test/
    git switch -c py-api-doc-13910 origin/py-api-doc-13910
    tox -c py/tox.ini

    @titusfortner
    Copy link
    Member

    Oh wow, yeah, this looks great. When looking at the diff I'm seeing several things in the current docs that still reference Selenium 4.17

    @titusfortner
    Copy link
    Member

    @emanlove can you fix the conflict (looks like adding the line from #14171 isn't part of this PR)

    @titusfortner
    Copy link
    Member

    Also can you create another issue for auto-generating the api.rst file so we can track it?

    @emanlove
    Copy link
    Contributor Author

    @titusfortner Fixed the conflict. Sorry about that one.

    Copy link
    Member

    @titusfortner titusfortner 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 verified this works and is an improvement on what we have now. Thanks.

    @titusfortner
    Copy link
    Member

    @iampopovich I know you did the work to update the api.rst file here, do you want me to merge your file and rebase this on top of it, or can I just merge this? Also are you in our chat room? (https://www.selenium.dev/support/#ChatRoom) We can discuss more there if you'd like.

    @iampopovich
    Copy link
    Contributor

    These changes are better than mine. It's fine to use them, no worries. They also include part of my work, so it's all fair 🤝

    @titusfortner titusfortner merged commit 6936f64 into SeleniumHQ:trunk Jun 23, 2024
    10 checks passed
    @titusfortner
    Copy link
    Member

    Now I just need to see about retroactively generating docs from the 4.22 tag😁

    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.

    3 participants