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.

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

chore(doc): Add Sphinx build #77

Merged
merged 11 commits into from
Apr 17, 2024
Merged

chore(doc): Add Sphinx build #77

merged 11 commits into from
Apr 17, 2024

Conversation

KyleFromNVIDIA
Copy link
Contributor

No description provided.

bdice
bdice requested changes Apr 11, 2024
View reviewed changes
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Please use NumPy style docstrings for consistency with most of RAPIDS.

src/rapids_dependency_file_generator/__init__.py Outdated Show resolved Hide resolved
src/rapids_dependency_file_generator/rapids_dependency_file_generator.py Outdated Show resolved Hide resolved
vyasr
vyasr requested changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

We don't have to do it in this PR, but once this is merged can we replace all the current linters possible with ruff? When we do, we can also enable ruff's rules for docstring linting to help ensure that these docstrings stay in good shape (complementary to building with warnings as errors as I suggested in an inline comment). It might be worth trying to just add ruff in this PR for the purpose of docstring linting and seeing how extensive the changes are, in case they're not too bad.

Most of the changes that I suggested are optional, but it's worth trying them out before we merge in case they're easy and we can just knock them out here.

docs/source/_templates/class.rst Show resolved Hide resolved
docs/source/_templates/module.rst Outdated Show resolved Hide resolved
docs/Makefile Outdated Show resolved Hide resolved
docs/source/conf.py Outdated Show resolved Hide resolved
bdice
bdice reviewed Apr 17, 2024
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
vyasr
vyasr approved these changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Approved, assuming you add the remaining ruff rules we need. Thanks!

@KyleFromNVIDIA KyleFromNVIDIA merged commit f692b70 into rapidsai:main Apr 17, 2024
4 checks passed
@jameslamb jameslamb mentioned this pull request Apr 18, 2024
Open
@GPUtester
Copy link

🎉 This PR is included in version 1.13.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jakirkham jakirkham mentioned this pull request Sep 24, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdice bdice bdice approved these changes

@vyasr vyasr vyasr approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KyleFromNVIDIA @GPUtester @vyasr @bdice