-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add special case for NumpyDoc warnings section so they render as warning admonitions #236
Add special case for NumpyDoc warnings section so they render as warning admonitions #236
Conversation
970e722
to
919e6e8
Compare
I'm not sure what the deal with the (macos-latest, 3.8) tests is, but I don't think that's my fault? 😅 |
Indeed, not sure what these Regarding the change: I don't like modifying kinds in the generic admonition class. Ideally, Griffe is never aware of how admonition kinds are used, which ones are available, etc. It all depends on the downstream docs system. For all we know, maybe someone has defined a custom With this in mind, I'd prefer that we handle this special case in the Numpy parser itself, rather than in the generic admonition class, and only because Warnings section are actually specified in the style-guide (https://numpydoc.readthedocs.io/en/latest/format.html#warnings). Looks like def _append_section(sections: list, current: list[str], admonition_title: str) -> None:
if admonition_title:
kind = admonition_title.lower().replace(" ", "-")
# Numpydoc style-guide defines a Warnings section,
# so we special-case it here to avoid breaking user expectations.
if kind == "warnings":
kind = "warning"
sections.append(
DocstringSectionAdmonition(
kind=kind,
text="\n".join(current).rstrip("\n"),
title=admonition_title,
),
)
elif current and any(current):
sections.append(DocstringSectionText("\n".join(current).rstrip("\n"))) WDYT? By the way, maybe we should also modify |
Wrote this comment, made the changes, then realised I forgot to press send lmao. I like your suggestion though, didn't realise there were NumpyDoc specific sections so deffo better to handle the special cases in those. I've also added a special case for the Notes section as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, LGTM! Just two suggestions that I'll commit right away, and we can merge 🙂
Leaving the branch up as I need to build the docs from it until the next Griffe version is released. |
I wanted to pack a few more things in the next release, but they've been delayed, so I'll release one today 🙂 |
v0.41.0 released. |
Awesome, thanks! 🎉 |
This PR fixes a bug in the NumpyDoc parser where the Warnings section would render as a note admonition. They now render as warning admonitions.
I felt bad opening an issue about this after accusing the NumpyDoc parser of being buggy yesterday, so thought I'd at least contribute this one myself 😄