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

Documentation facelift #242

Merged
merged 13 commits into from
Oct 9, 2024
Merged

Documentation facelift #242

merged 13 commits into from
Oct 9, 2024

Conversation

wiseodd
Copy link
Collaborator

@wiseodd wiseodd commented Sep 13, 2024

Closes #226.

I decided to use mkdocs-material, which is the standard in Python (see Astral's docs etc.).

@aleximmer, I added a deploy workflow. Could you disable the current one for pdoc? (Seems like it's hidden in the repo settings page.)

To review:

uv sync --all-extras
uv run mkdocs serve

then open localhost:8000

@wiseodd wiseodd mentioned this pull request Sep 14, 2024
@wiseodd wiseodd self-assigned this Sep 15, 2024
@wiseodd wiseodd added documentation Improvements or additions to documentation enhancement New feature or request labels Sep 15, 2024
Copy link
Owner

@aleximmer aleximmer left a comment

Choose a reason for hiding this comment

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

Thanks a lot for updating the docs. They look super good now 💯

I got some warnings when running uv run mkdocs serve, not sure if these are serious:

SyntaxWarning: invalid escape sequence '\i'
             File "Laplace/.venv/lib/python3.12/site-packages/_griffe/agents/visitor.py", line 203, in get_module
               top_node = compile(self.code, mode="exec", filename=str(self.filepath), flags=ast.PyCF_ONLY_AST, optimize=1)
             File "Laplace/laplace/lllaplace.py", line 510, in
               """Here not much changes in terms of GP inference compared to FunctionalLaplace class.

It also shows us some docstring parameter descriptions that we should fix:

WARNING -  griffe: laplace/baselaplace.py:1957: Parameter 'num_data' does not appear in the function signature
WARNING -  griffe: laplace/baselaplace.py:1957: Parameter 'diagonal_kernel' does not appear in the function signature
WARNING -  griffe: laplace/baselaplace.py:1957: No types or annotations for parameters ['See']
WARNING -  griffe: laplace/baselaplace.py:1957: Parameter 'See' does not appear in the function signature
WARNING -  griffe: laplace/utils/matrix.py:296: Parameter 'dampen' does not appear in the function signature
WARNING -  griffe: laplace/utils/subnetmask.py:361: No types or annotations for parameters ['parameter_names']
WARNING -  griffe: laplace/utils/subnetmask.py:361: Parameter 'parameter_names' does not appear in the function signature

Should we open an issue to fix it after the PR?

For deploying the docs, I will disable the old docs once you merge.

docs/devs_guide.md Outdated Show resolved Hide resolved
docs/devs_guide.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@runame runame left a comment

Choose a reason for hiding this comment

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

Haven't looked at this in detail yet, but looks good! Feel free to just merge.

@wiseodd
Copy link
Collaborator Author

wiseodd commented Oct 9, 2024

@aleximmer fixed the issues. For the improper docstrings, I agree, let's open an issue for this. I also want to refactor the codebase to make it more extendible.

Feel free to merge and test whether the new docs site is properly deployed.

@aleximmer aleximmer merged commit 3b4cabb into main Oct 9, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite documentation site
3 participants