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

Fix all warnings in Python docs #13789

Merged
merged 31 commits into from
Aug 7, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jul 31, 2023

Description

The Sphinx documentation has historically been warning-filled, which makes it difficult to identify when there are real issues like missing APIs. This PR fixes all the current issues and converts warnings to errors during the build, ensuring that doc builds are reliable indicators of issues in the future.

I will say that there are a few changes that may not be exactly what we want, particularly in cases of including APIs that may not be documented in exactly the same way in pandas. However, I think we'd be better off merging this PR so that we can get to a 0 warnings state and then work through further improvements in follow-ups where the build will be more robust.

Here is an inexhaustive list of the most significant changes:

  • Adds all missing BaseIndex APIs. The goal of BaseIndex is to provide an abstract interface defining all functions that should match pandas.Index, but up until now some methods were missing. This PR does not implement any new ones, it either lifts existing implementations up from subclasses (where those implementations are generic for all Index types) or it simply defines them as returning NotImplemented. The result is that all methods at least exist so that docs don't complain.
  • Cleans up the listed APIs in rst files so that all existing APIs are included somewhere and no nonexistent APIs are listed anywhere. APIs that don't have an exact equivalent in the pandas docs are given a new home in these docs. That includes pieces like extension dtypes, which were previously documented in the user guide and therefore weren't part of any summary list (causing warnings).
  • Fixed missing dependencies for doc notebooks.
  • Fixed various formatting issues with docstrings, especially around bulleted lists that were missing the requisite spacing to be rendered correctly.
  • Fixing header ordering (going from level 1 to level 3 headings is a warning in Sphinx) and links in notebooks.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@vyasr vyasr added 3 - Ready for Review Ready for review by team doc Documentation non-breaking Non-breaking change labels Jul 31, 2023
@vyasr vyasr self-assigned this Jul 31, 2023
@vyasr vyasr requested review from a team as code owners July 31, 2023 20:46
@github-actions github-actions bot added the conda label Jul 31, 2023
ci/build_docs.sh Outdated Show resolved Hide resolved
@galipremsagar galipremsagar self-requested a review August 1, 2023 20:40
@vyasr vyasr requested a review from bdice August 2, 2023 16:19
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

blocking while I investigate some build path changes resulting from this PR

@ajschmidt8
Copy link
Member

It looks like the build paths are different as a result of the changes in this PR.

For example, compare the preview environments below:

I see the same problem on the rmm PR.

@@ -3,8 +3,9 @@

# You can set these variables from the command line, and also
# from the environment for the first two.
SPHINXOPTS ?=
SPHINXOPTS ?= -n -v
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why these flags don't match the cudf Makefile flags?

Will warnings be treated as errors for dask-cudf in a follow-up PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I didn't want to also try and fix dask-cudf issues in the same PR. This one is large enough already. I also don't know dask-cudf as well so may want to get someone more familiar to work through those issues.

docs/cudf/Makefile Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor Author

vyasr commented Aug 7, 2023

/merge

@rapids-bot rapids-bot bot merged commit e92de81 into rapidsai:branch-23.10 Aug 7, 2023
54 checks passed
@vyasr vyasr deleted the feat/cleanup_docs branch August 7, 2023 16:15
@vyasr vyasr mentioned this pull request Aug 7, 2023
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Aug 8, 2023
This PR is a follow-up to #13789 that adds specified lists of methods/attributes to some classes; removes redundancy in the autosummary templates we are using and adds documentation explaining how they work; and removes various pieces of outdated code in our conf.py to make it easier to maintain going forward.

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

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

URL: #13826
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team doc Documentation non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants