Skip to content

Conversation

@calumy
Copy link
Contributor

@calumy calumy commented Aug 26, 2024

Summary

Now that docs are formatted by the Ruff formatter, #13087. It is also possible to format pyi-style examples with the pyi-style format fixing #11568.

One concern I have with this update is that syntax highlighting is currently not implemented in MK docs for PYI examples. Therefore, we need to decide if it is better that the docs are formatted correctly or if syntax highlighting is more important. If the latter is the case, I can investigate this further and draft this PR for now.

Test Plan

  • CI to check that docs are formatted correctly

CC @AlexWaygood, I would appreciate it if you could review this PR as you opened the issue that prompted this update. I think it would be useful to sense-check the examples that I updated to PYI, as well as let me know and further examples that would benefit from pyi style formatting and I can update as required.

@calumy calumy requested a review from AlexWaygood as a code owner August 26, 2024 21:57
@calumy calumy marked this pull request as draft August 26, 2024 21:59
@calumy calumy marked this pull request as ready for review August 26, 2024 22:01
@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_salesforce.ipynb:14:1:1: Expected an expression

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_salesforce.ipynb:14:1:1: Expected an expression

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

Nice, this looks great!

One concern I have with this update is that syntax highlighting is currently not implemented in MK docs for PYI examples.

Hmm, that is a shame (and might indeed be a blocker here, sadly ☹️). Did you find this out from reading the docs, or by experimenting? I can't immediately find anywhere where this is documented :/

@calumy
Copy link
Contributor Author

calumy commented Aug 27, 2024

Nice, this looks great!

One concern I have with this update is that syntax highlighting is currently not implemented in MK docs for PYI examples.

Hmm, that is a shame (and might indeed be a blocker here, sadly ☹️). Did you find this out from reading the docs, or by experimenting? I can't immediately find anywhere where this is documented :/

This was found with experimentation. Hopefully, the lack of highlighting of .pyi files should be fixed with this PR and a subsequent release of pygments.

I tested this by installing locally with pip install git+https://github.com/calumy/pygments.git@add-pyi-short-name (sorry, I have not had a chance to use UV yet). Then running mkdocs serve -f mkdocs.public.yml and checking one of the updated pyi docs, e.g. http://127.0.0.1:8000/ruff/rules/unprefixed-type-param/. Which resulted in the following:
image

Do you know of any other places where .pyi file syntax highlighting is missing that would be a blocker?

Should I draft until the next release of pygments or would you consider pinning to a commit of pygments in the requirements? If so, which requirements file would be best for this, as there appear to be a couple in the docs directory?

@AlexWaygood
Copy link
Member

Hopefully, the lack of highlighting of .pyi files should be fixed with this PR and a subsequent release of pygments.

Woah, nice! Thanks so much for figuring out the root cause there and filing a PR to solve it — that's awesome 😃

(It's late here so I'll get to your other questions tomorrow :)

@dhruvmanila dhruvmanila added the documentation Improvements or additions to documentation label Aug 28, 2024
@calumy
Copy link
Contributor Author

calumy commented Aug 28, 2024

pygments/pygments#2773 has been merged and added to the next release milestone. I am unsure of the schedule of the next release of pygments.

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 28, 2024

pygments/pygments#2773 has been merged and added to the next release milestone.

I saw! I'm currently looking into whether it's feasible for us to pin pygments to a commit on their main branch -- if so, we can land this immediately. If not, we'll need to wait. I'll get back to you soon.

If we do need to wait, we should pick up the latest release of pygments in CI as soon as it's released. The depdendency comes from mkdocs-material's pyproject.toml file here: https://github.com/squidfunk/mkdocs-material/blob/6f3c05b6fa7e42279ea7604aedc4e4333131fe57/requirements.txt#L26. And the dependency is permissive enough that it should allow pygments==2.19.0 as soon as it's released.

@AlexWaygood
Copy link
Member

I think pinning to a main branch commit works -- I pushed the new pins to this branch, so we should be good to go. Thanks again for this -- really great contribution 😃

@AlexWaygood AlexWaygood changed the title Format PYI files in docs correctly Format PYI examples in docs as .pyi-file snippets Aug 28, 2024
@AlexWaygood AlexWaygood merged commit 2e75cfb into astral-sh:main Aug 28, 2024
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants