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 requirements.txt to doc build environment #14171

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

emanlove
Copy link
Contributor

@emanlove emanlove commented Jun 22, 2024

User description

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.

Description

This adds the requirments.txt to the tox docs build environment.

Motivation and Context

The Sphinx Python API docs are built by "reading the code". That is as if Python were importing or running it. Thus the required
dependencies to run the Python bindings are also require to build the API documentation.

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, Documentation


Description

  • Added requirements.txt to the dependencies list in the tox configuration to ensure all necessary packages are available for building the API documentation with Sphinx.
  • This change addresses an issue where the Sphinx Python API documentation build process requires the dependent Python packages.

Changes walkthrough 📝

Relevant files
Configuration changes
tox.ini
Include `requirements.txt` in tox documentation build environment

py/tox.ini

  • Added requirements.txt to the dependencies list in the tox
    configuration.
  • +1/-0     

    💡 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.
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 1
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review None

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Ensure the path to requirements.txt is correct to avoid file not found errors

    Ensure that the requirements.txt file is located in the correct directory relative to the
    tox.ini file, or provide an absolute path to avoid potential file not found errors.

    py/tox.ini [9]

    --r requirements.txt
    +-r path/to/requirements.txt
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: This suggestion is relevant as it addresses a potential runtime error if the requirements.txt file is not found. It's a practical improvement for ensuring the build process does not fail.

    7
    Best practice
    Pin versions of dependencies in requirements.txt to ensure consistent builds

    Consider pinning the versions of the dependencies listed in requirements.txt to ensure
    consistent builds across different environments.

    py/tox.ini [9]

     -r requirements.txt
    +# Ensure versions are pinned in requirements.txt
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion to pin versions in requirements.txt is a good practice to ensure consistent environments, although it does not directly modify the PR code but rather the contents of another file. It's indirectly related to the PR changes.

    6

    @iampopovich
    Copy link
    Contributor

    @emanlove I applied the changes in my PR, and the documentation was generated without any dependency errors. Thank you.

    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