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

[INF] update some mkdocs infrastructure #1231

Merged
merged 9 commits into from
Jan 29, 2023
Merged

[INF] update some mkdocs infrastructure #1231

merged 9 commits into from
Jan 29, 2023

Conversation

thatlittleboy
Copy link
Contributor

@thatlittleboy thatlittleboy commented Jan 22, 2023

PR Description

Couple of minor infrastructure changes:

  • I've deleted most of the linting tools in environment-dev.yml to reduce the bloat (esp for our CIs), and to promote the use of pre-commit (which sets up its own isolated environments for each tool). Two exceptions:
    • black: since the vscode container uses the black binary to format on save
    • isort: this isn't in pre-commit config yet, to be fixed in the future
  • Since we don't pin mkdocs and friends, there're some new compatibility changes with v0.9 mkdocs-material and mkdocs v1.4. E.g. the watch option is recommended to use the global mkdocs one. pymdownx navigation extension names were wrong... etc.
  • Added some more mkdocs-material features which I think will help the navigation experience
  • I moved the Development Guide section to below API reference, since the latter is most likely what our users want to see (first).

PR Checklist

Please ensure that you have done the following:

  1. Add a line to CHANGELOG.md under the latest version header (i.e. the one that is "on deck") describing the contribution.
    • Do use some discretion here; if there are multiple PRs that are related, keep them in a single line.

Relevant Reviewers

Please tag maintainers to review.

`environment-dev.yml` is too bloated, for one: most formatting/linting tools
should be controlled by `pre-commit` (which sets up its own isolated environments
for each tool).

Two exceptions:
- `black`: since the vscode container uses the black binary to format on save
- `isort`: this isn't in pre-commit config yet, to be fixed in the future
We are now using v0.9 mkdocs-material and mkdocs v1.4, along with it comes several
compatibility changes.

https://squidfunk.github.io/mkdocs-material/upgrade/ for CHANGELOG

E.g. the `watch` option is recommended to use the global mkdocs one. `pymdownx`
navigation extension names were wrong...
@ericmjl
Copy link
Member

ericmjl commented Jan 22, 2023

@codecov
Copy link

codecov bot commented Jan 22, 2023

Codecov Report

Merging #1231 (3b6b00b) into dev (161c290) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev    #1231      +/-   ##
==========================================
+ Coverage   97.71%   97.77%   +0.05%     
==========================================
  Files          78       78              
  Lines        3767     3767              
==========================================
+ Hits         3681     3683       +2     
+ Misses         86       84       -2     

Copy link
Collaborator

@samukweku samukweku left a comment

Choose a reason for hiding this comment

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

any ideas on the mkdocs-legacy? ok to remove and just have the new mkdocs?

@thatlittleboy
Copy link
Contributor Author

thatlittleboy commented Jan 22, 2023

any ideas on the mkdocs-legacy? ok to remove and just have the new mkdocs?

@samukweku

Yea, I was thinking about this recently as well, since mkdocstrings recently just got updated to v0.20. I don't think it's okay to remove it just yet, it hinges on #1032 being resolved -- because the new python parser that mkdocstrings uses (griffe) doesn't support the sphinx style that well.

So I'm imagining that we'll need to:

  • update all doc styles to google style (it's the most feature complete one)
  • update mkdocstrings to v0.20+, along with relevant changes to mkdocs.yml -- selection/rendering options are now in one single list of options, etc.
  • and remove the version pinning (jinja2 to use 3.10+, etc.)

Correction: hmm nvm, ignore me. It might be possible to just do an upgrade first (while still using the legacy parser), I'll investigate and report back.

@thatlittleboy thatlittleboy added the infrastructure Infrastructure-related issues label Jan 22, 2023
@thatlittleboy
Copy link
Contributor Author

thatlittleboy commented Jan 23, 2023

any ideas on the mkdocs-legacy? ok to remove and just have the new mkdocs?

@samukweku

Yea, I was thinking about this recently as well, since mkdocstrings recently just got updated to v0.20. I don't think it's okay to remove it just yet, it hinges on #1032 being resolved -- because the new python parser that mkdocstrings uses (griffe) doesn't support the sphinx style that well.

So I'm imagining that we'll need to:

* update all doc styles to google style (it's the most feature complete one)

* update mkdocstrings to v0.20+, along with relevant changes to `mkdocs.yml` -- selection/rendering options are now in one single list of options, etc.

* and remove the version pinning (jinja2 to use 3.10+, etc.)

Correction: hmm nvm, ignore me. It might be possible to just do an upgrade first (while still using the legacy parser), I'll investigate and report back.

We can remove the pin on mkdocstrings to use v0.19+ (not quite, need an upstream bug to be fixed first), but I wouldn't make the jump to drop the "mkdocstrings-python-legacy" handler just yet. I rather we move to Google style along (#1032) with the upgrade to the new "mkdocstrings-python" handler together as a whole.

I'll create a separate PR to upgrade our mkdocstrings once the aforementioned bug is fixed.

@thatlittleboy
Copy link
Contributor Author

Just realized there's been multiple related PRs to this current one (some overlap), #1133, #1147 . I'll like your opinion on the changes I'm making, if possible @ericmjl @Zeroto521 🙏🏽

@thatlittleboy thatlittleboy mentioned this pull request Jan 23, 2023
3 tasks
@ericmjl
Copy link
Member

ericmjl commented Jan 29, 2023

@thatlittleboy thank you for handling this PR!

I've reviewed the changes and I'm good to go as well.

Regarding docstring style, I'm also happy to make a move to Google-style docstrings now that AI coding tools are more generally available, as they can help with the migration. I have paid for Tabnine, but we can also edit ChatGPT's output. Without them, reformatting would be a chore.

@ericmjl ericmjl merged commit f8fb11a into dev Jan 29, 2023
@thatlittleboy thatlittleboy deleted the infra-mkdocs branch January 29, 2023 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Infrastructure-related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants