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

Correctly handle case where "See Also" section follows "Examples" in global_enable_try_examples #251

Merged
merged 7 commits into from
Jan 13, 2025
Merged

Correctly handle case where "See Also" section follows "Examples" in global_enable_try_examples #251

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f24cd2d
Add `..seealso` section as a pattern
agriyakhetarpal Jan 10, 2025
10abc82
Fix escaping of regex characters
agriyakhetarpal Jan 11, 2025
5f187f3
Reword comment about SymPy's special case
agriyakhetarpal Jan 12, 2025
79e864e
Add subset of docutils + Sphinx directives to end at
agriyakhetarpal Jan 13, 2025
08e70bd
Revert "Add subset of docutils + Sphinx directives to end at"
agriyakhetarpal Jan 13, 2025
5397180
Reword comment about "See Also" section
agriyakhetarpal Jan 13, 2025
631dfeb
Remove references to historical PRs
agriyakhetarpal Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions jupyterlite_sphinx/_try_examples.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,10 @@ def _process_literal_blocks(md_text):
+ [r"\!\! processed by numpydoc \!\!"]
# Attributes section sometimes has no directive.
+ [r":Attributes:"]
# The See Also section shows up for old numpydoc versions where ordering
# is neither guaranteed nor enforced; such as SymPy, therefore we check
# for it as a special case.
Copy link
Collaborator

@steppi steppi Jan 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

"See Also" is included in _non_example_docstring_section_headers, so I don't think the issue can hinge just on the ability of "See Also" sections to appear after an "Examples" sections in older numpydocs versions. It looks like in SymPy, after numpydoc preprocessing, the line for the See Also section is ".. seealso::", but the existing logic expects it to look like ".. rubric:: See Also". I think the real issue is that the older numpydoc version uses a different format for the directives that get inserted during processing. It just so happens that SymPy has a "See Also" section after each "Examples" section, but I think any section appearing afterwards would fail to be detected.

If we want to support older numpydoc versions, then I think the principled solution would then be to either:

  1. Add the format used by older numpydoc versions to _next_section_pattern
    or
  2. Detect the numpydoc version at runtime, and have _next_section_start_pattern depend on that version

update: or it could even be that the directives even for newer numpydoc versions are different from what I expected, but I had it right for every section that can appear after "Examples" with newer numpydoc versions.

Copy link
Member Author

@agriyakhetarpal agriyakhetarpal Jan 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It just so happens that SymPy has a "See Also" section after each "Examples" section, but I think any section appearing afterwards would fail to be detected.

Ah, you're right – thanks for pointing that out! I think it would be nice to support older numpydoc versions in whatever manner we can, but I think option 2) is not possible with SymPy's numpydoc at the moment, as it is present as a custom Sphinx extension in docs/sphinxext/ across two files, and not as an installable package fetched from PyPI or a separate location that we can compare or detect versions for. So we are left with 1), which I've done here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, the See Also section seems to be a case that has been handled specially: https://github.com/sympy/sympy/blob/3c33a8283cd02e2e012b57435bd6212ed915a0bc/doc/ext/docscrape_sphinx.py#L155-L161

And it doesn't seem to be the case for the "Notes", "Examples", or "References" sections: https://github.com/sympy/sympy/blob/3c33a8283cd02e2e012b57435bd6212ed915a0bc/doc/ext/docscrape.py#L424, which are not included in Try Ex

I haven't dived into the code in detail, but I infer that the difference is because the "See Also" section is an admonition in SymPy's numpydoc, and not a rubric. I'd suggest that we stick with the current approach for now, and add any other cases here if someone notices them, until I can migrate SymPy to the upstream numpydoc and remove their implementation (that is, I'm short of ideas, unless you have some).

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is working, it's fine to do this hack to get SymPy working specifically, but the comment is incorrect and should be updated. It's not about the ordering but because of the form the directive takes in sympy's version of numpydocs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, makes sense to me. I hope 5f187f3 addresses it!

+ [r".. seealso::"]
)
)

Expand Down
Loading