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

Update docs version #154

Merged
merged 9 commits into from
Mar 7, 2023
Merged

Update docs version #154

merged 9 commits into from
Mar 7, 2023

Conversation

adamgayoso
Copy link
Member

@adamgayoso adamgayoso commented Mar 2, 2023

built docs

  1. lower bound the theme version so that we get dark mode!
  2. Use sphinx ext autodoc for typehint processing instead of sphinx-autodoc-typehints which we borrowed from scanpy at a time when sphinx didn't have this functionality. The third-party package also started processing numpydoctring return sections differently resulting in a colon in front of our custom freeform return
  3. Edited the autosummary class template to remove headers for attr/meth names, which aren't necessary. Without this it looks like this on the right sidebar:

Screenshot 2023-03-02 at 10 55 02 AM

@adamgayoso adamgayoso requested review from flying-sheep and giovp March 2, 2023 18:55
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

A PR has been generated to the instance repo:
scverse/cookiecutter-scverse-instance#42

You can check out the PR to preview your changes
in an instance of the cookiecutter template. The PR will be kept in sync with
this PR automatically.

@adamgayoso adamgayoso changed the title Update docs theme Update docs version, drop sphinx-autodoc-typehints for sphinx.ext.autodoc Mar 3, 2023
@adamgayoso adamgayoso requested a review from grst March 3, 2023 20:53
@flying-sheep
Copy link
Member

flying-sheep commented Mar 4, 2023

sphinx.ext.autodoc’s typehint support is not yet ready for prime time, as it obtains type hints by parsing signatures and can’t deal with parameter defaults whose reprs aren’t valid Python code (e.g. every function, every object of a user defined class without __repr__ method, enum members, …). I’m not sure if that only applies to autodoc_typehints = 'signature' (which we don’t use) or all of them, in which case it’s a dealbreaker. If it breaks, we should continue to use sphinx-autodoc-typehints (without scanpydoc’s by-now-unnecessary customization).

I used autodoc_typehints = 'signature' for a little package I maintain, and had to hack around its limitations in two instances, which is too much. E.g. I had to turn this wonderfully reasonable definition:

from operator import add
from collections.abc import Iterable, Callable
from typing import TypeVar

A = TypeVar("A")
T = TypeVar("T")

class accumulate(...)
    def __init__(iterable: Iterable[A], func: Callable[[A, A], T] = add) -> None:
        ...

into this monstrosity to keep sphinx.ext.autodocs from choking:

...

class _Adder:
    """This just exists to allow Sphinx to parse the function signature."""

    def __call__(self, a: A, b: A) -> T:
        a + b

    def __repr__(self) -> str:
        return "add"  # pragma: no cover

class accumulate(...)
    def __init__(iterable: Iterable[A], func: Callable[[A, A], T] = _Adder()) -> None:
        ...

@adamgayoso
Copy link
Member Author

@flying-sheep happy to keep sphinx autodoc typehints, but would you be able to update the typed returns extension on this repo?

This PR on sphinx autodoc typehints tox-dev/sphinx-autodoc-typehints#311 causes an issue where : precedes all return descriptions.

@flying-sheep
Copy link
Member

flying-sheep commented Mar 5, 2023

I’ll check out the return problem. But before I do that: I can’t find the issue you filed to sphinx-autodoc-typehints’ repo about this problem, can you link it to me?

I investigated further, and while using 'description' helps with the problem I described, it doesn’t fix other problems like spurios “class not found” errors when using TypeVars. sphinx-autodoc-typehints is just more mature right now.

Comparison:

sphinx-autodoc-typehints

autodoc_typehints = "description"

grafik

grafik

no log errors

grafik

I think neither style is good. sphinx-autodoc-typehints is too visually busy, and autodoc_typehints = "description" breaks the otherwise consistent visual style for references to functions, classes, …

@adamgayoso
Copy link
Member Author

I didn't file an issue because the problem seems to be more specific to our custom typed returns extension, which is in this repo/scanpy/etc. You can see the "bug" in the left image above: : Some integer value

I can remove the change in this PR, but we should figure something out with the : issue.

@flying-sheep
Copy link
Member

flying-sheep commented Mar 7, 2023

Ah I see! Regarding the colon, since we know which PR it comes from and that it’s just there instead of being indicative of a deeper problem, we can simply remove it, right?

We should move that little typed-returns customization to scanpydoc and test it properly, but for now, I’ll just fix it here.

@flying-sheep flying-sheep changed the title Update docs version, drop sphinx-autodoc-typehints for sphinx.ext.autodoc Update docs version Mar 7, 2023
@grst grst merged commit f44f5ee into main Mar 7, 2023
@grst grst deleted the docs_version branch March 7, 2023 17:02
@giovp giovp mentioned this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants