Skip to content

Use nvidia-sphinx-theme for docs#528

Merged
rapids-bot[bot] merged 7 commits into
NVIDIA:branch-25.02from
benfred:nvidia-sphinx-theme
Dec 20, 2024
Merged

Use nvidia-sphinx-theme for docs#528
rapids-bot[bot] merged 7 commits into
NVIDIA:branch-25.02from
benfred:nvidia-sphinx-theme

Conversation

@benfred

@benfred benfred commented Dec 11, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@benfred benfred added doc Improvements or additions to documentation non-breaking Introduces a non-breaking change labels Dec 11, 2024
@benfred benfred requested review from a team as code owners December 11, 2024 17:16
@benfred benfred requested a review from jameslamb December 11, 2024 17:16
@benfred benfred marked this pull request as draft December 11, 2024 17:30
@copy-pr-bot

copy-pr-bot Bot commented Dec 11, 2024

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@benfred

benfred commented Dec 11, 2024

Copy link
Copy Markdown
Contributor Author

/ok to test

nvidia-sphinx-theme calls `sphinx.util.fileutil.copy_asset_file` with
a Path - but sphinx v7.* expects a `str` , which causes the build to fail.
Require sphinx v8 here to make this work
@benfred benfred marked this pull request as ready for review December 12, 2024 19:26
the docs build in CI is using an older version of breathe
(v4.15.0) - which is from 2020 and doesn't support
sphinx v8 (sphinx-doc/sphinx#11490).

Fix by requiring the latest version

@jameslamb jameslamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems fine to me, glad to see that it's on pypi.org: https://pypi.org/project/nvidia-sphinx-theme/#description

Breathe doesn't fully support sphinx v8 - it raises some warnings
in sphinx since its passing a `str` to Sphinx functions that
expect a `Path`. This still works for now (will be fully removed
in sphinx v9), but is deprecated - which is why breathe
indicates it only support sphinx<=v7.2 in conda.

However, nvidia-sphinx-theme only works with sphinx v8 afaict,
since it is passing a Path to `sphinx.util.fileutil.copy_asset_file`
- which breaks on sphinx v7 since it's expecting a `str`.

Hack around this dependency issue by installing breathe from pip,
which isn't as strict about supported sphinx versions.
@benfred

benfred commented Dec 20, 2024

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot rapids-bot Bot merged commit 89ebf15 into NVIDIA:branch-25.02 Dec 20, 2024
@benfred benfred deleted the nvidia-sphinx-theme branch December 20, 2024 20:06
rapids-bot Bot pushed a commit that referenced this pull request Dec 31, 2024
As part of #528 cuvs's doc builds were modified to pull Breathe from pip. That was necessary because the nvidia-sphinx-theme requires Sphinx 8 but [the conda-forge Breathe package was not compatible with that Sphinx version](conda-forge/breathe-feedstock#63). I fixed that in conda-forge/breathe-feedstock#64, so now we can go back to using Breathe from conda to avoid mixing pip and conda for dependency management in the same environment.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #554
rapids-bot Bot pushed a commit to rapidsai/cugraph-docs that referenced this pull request Jan 7, 2025
**This is exactly the same set of changes already approved in #69.**

`cugraph-docs` was switched from private to public during the lifetime of that PR, which put CI into a state where that PR couldn't be merged: #69 (comment)

---

Development and the vision for this project has stabilized here quite a bit over the last few weeks, so I think it's a good time to simplify things.

This proposes the following:

* removing configuration for codecov
  - *(there are no tests running here)*
* removing patterns in CODEOWNERS that don't match any files
* removing anything related to CUDA 11, pyproject.toml, or requirements.txt in `dependencies.yaml` and related files
  - *this repo exclusively uses conda, and only a single major version of CUDA*
* updating to `sphinx>=8` and `breathe>=4.35`
  - *to match the rest of RAPIDS, e.g. rapidsai/cugraph#4839, NVIDIA/cuvs#528
  - *floors also mean faster conda solves and fewer surprises at build time*
* removing unnecessary files in `ci/utils`
  - *these appear to have been copied from `cugraph`, but they're not needed as we don't do notebook testing here*

## Notes for Reviewers

### How I tested this

Tested the `update-version.sh` changes like this:

```shell
./ci/release/update-version.sh '25.04.00'

git grep -E '25\.2'
git grep -E '25\.02'
```

Authors:
  - James Lamb (https://github.com/jameslamb)

Approvers:
  - Don Acosta (https://github.com/acostadon)
  - Ray Douglass (https://github.com/raydouglass)
  - Bradley Dice (https://github.com/bdice)

URL: #71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Improvements or additions to documentation non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants