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

Handler autodoc throw exceptions after >= 7.1.0 release #11543

Closed
johnnv1 opened this issue Jul 31, 2023 · 16 comments · Fixed by #11550
Closed

Handler autodoc throw exceptions after >= 7.1.0 release #11543

johnnv1 opened this issue Jul 31, 2023 · 16 comments · Fixed by #11550
Milestone

Comments

@johnnv1
Copy link

johnnv1 commented Jul 31, 2023

Describe the bug

After the 7.1 release, the kornia docs started to show warnings for throw exceptions from autodocs for some dataclass. I think is some issue with autodoc for dataclass classes.

WARNING: error while formatting arguments for kornia.contrib.models.SegmentationResults: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.contrib.models.Prompts: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.contrib.models.rt_detr.RTDETRConfig: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.contrib.models.sam.SamConfig: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.feature.DISKFeatures: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.image.ImageSize: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.image.PixelFormat: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for kornia.image.ImageLayout: Handler <function update_defvalue at 0x7fef18c267a0> for event 'autodoc-before-process-signature' threw an exception (exception: )

How to Reproduce

I could not reduce it to something small and reproducible -- but all the warnings on kornia docs occurs on data classes

$ git clone git@github.com:kornia/kornia.git
$ virtualvenv venv -p python3.10
$ pip install -e .[docs]
$ make build-docs SPHINXOPTS="-W"

Environment Information

Platform:              linux; (Linux-5.19.0-50-generic-x86_64-with-glibc2.35)
Python version:        3.10.10 (main, Mar 21 2023, 18:45:11) [GCC 11.2.0])
Python implementation: CPython
Sphinx version:        7.1.0
Docutils version:      0.20.1
Jinja2 version:        3.1.2
Pygments version:      2.15.1

Sphinx extensions

i belive the issue is from: ['sphinx.ext.autodoc']

Additional context

list complete the extensions:
[
'sphinx.ext.autodoc',
'sphinx.ext.autosummary',
'sphinx.ext.doctest',
'sphinx.ext.intersphinx',
'sphinx.ext.mathjax',
'sphinx.ext.napoleon',
'sphinx_autodoc_typehints',
'sphinx_autodoc_defaultargs',
'sphinx_copybutton',
'sphinx.ext.viewcode',
'sphinx.ext.githubpages',
'sphinxcontrib.bibtex',
"sphinxcontrib.gtagjs",
'sphinxcontrib.youtube',
'sphinx_design',
]

this issue can also be checked on the kornia CI -- https://github.com/kornia/kornia/actions/workflows/docs.yml

@picnixz
Copy link
Member

picnixz commented Aug 2, 2023

I was afraid that the cause was what I implemented for PEP 695 but it appears that the issue is with the default values. In particular, I suspect that sphinx_autodoc_typehints or sphinx_autodoc_defaultargs are actually the one responsible for that. Can you disable these extensions and check whether it fails again?

@johnnv1
Copy link
Author

johnnv1 commented Aug 2, 2023

I was afraid that the cause was what I implemented for PEP 695 but it appears that the issue is with the default values. In particular, I suspect that sphinx_autodoc_typehints or sphinx_autodoc_defaultargs are actually the one responsible for that. Can you disable these extensions and check whether it fails again?

The issue still exists even without sphinx_autodoc_typehints and sphinx_autodoc_defaultargs

@picnixz
Copy link
Member

picnixz commented Aug 2, 2023

Can you test with only built-in Sphinx extensions ? (only those in sphinx.ext.*)

@wangenau
Copy link

wangenau commented Aug 2, 2023

I have encountered the same warnings when creating docs for dataclasses.
For me, I only get these warnings when autodoc_preserve_defaults is enabled.
If I set autodoc_preserve_defaults to False, the warnings disappear in my case.

@picnixz
Copy link
Member

picnixz commented Aug 2, 2023

Ok, I confirm it. The issue stems from the fact that we cannot retrieve properly the function definition AST for a dataclass. Even this simple example fails:

from __future__ import annotations

from dataclasses import dataclass

@dataclass
class Foo:
    """pouet"""
    bar: int = 1

The reason is that we fail to extract the code source for Foo.__init__. Unfortunately, there is little that we can do but what surprises me is that it should have never worked before. So I'm wondering what happened ?

@picnixz
Copy link
Member

picnixz commented Aug 3, 2023

I've bisected the culprit commit 4de540e which included some assert function is not None for mypy checks. Previously, we allowed function to be None because we would then catch AttributeError and TypeError exceptions silently. This case happens when the object being documented is an extension type or a built-in (such as a dataclass).

In particular, because we don't catch the assertion, it propagates. I'll write a patch for that today (and document why we are allowed to ignore the assertion!).

@jaraco
Copy link

jaraco commented Aug 12, 2023

In addition to dataclasses, a namedtuple subclass is also affected.

@jaraco
Copy link

jaraco commented Aug 12, 2023

I tried applying the potential fix to jaraco/irc, but it introduced a new error not present in 7.1:

Extension error (furo):
Handler <function _html_page_context at 0x107ec2700> for event 'html-page-context' threw an exception (exception: This documentation is not using `furo.css` as the stylesheet. If you have set `html_style` in your conf.py file, remove it.)

Perhaps that's by design, but it does mean that my best hope for a workaround until a proper fix is introduced will be to pin to sphinx<7.1.

@picnixz
Copy link
Member

picnixz commented Aug 12, 2023

This error is likely due to furo theme. We deprecated some theme API so it might be the reason (check the latest CHANGES).

I'll try to understand why it fails with namedtuple in the upcoming days.

EDIT: I don't know why what I did could affect themes btw.

johnnv1 added a commit to johnnv1/kornia that referenced this issue Aug 14, 2023
warnings after >= 7.1.0 release of sphinx
- sphinx-doc/sphinx#11543
@picnixz
Copy link
Member

picnixz commented Aug 15, 2023

@jaraco I just saw that your error is coming from another function so it's likely an issue on furo.

edgarriba pushed a commit to kornia/kornia that referenced this issue Aug 16, 2023
warnings after >= 7.1.0 release of sphinx
- sphinx-doc/sphinx#11543
@randolf-scholz
Copy link

randolf-scholz commented Aug 21, 2023

@AA-Turner I am still getting this issue (sphinx 7.2.2) for a very specific signature: @classmethod decorated function with both positional-only and keyword-only separator.

Example:

class Foo:
    @classmethod
    def foo(cls, bar, /, *, baz: int) -> None:
        print(cls, bar, baz)
WARNING: error while formatting arguments for tsdm.utils.data.datasets.Foo.foo: Handler <function update_defvalue at 0x7fac942d28e0> for event 'autodoc-before-process-signature' threw an exception (exception: wrong parameter order: positional or keyword parameter before positional-only parameter)

@AA-Turner
Copy link
Member

AA-Turner commented Aug 22, 2023

@randolf-scholz I'm having trouble reproducing this. Please could you add your conf.py and index.rst?

A

@AA-Turner
Copy link
Member

xref #10427, #10421

@AA-Turner
Copy link
Member

@randolf-scholz sorry for the earlier note, I've found the error.

In sphinx.ext.autodoc.preserve_defaults.update_defvalue() we insert a cls parameter for bound methods (i.e. classmethods) defined as inspect.Parameter('cls', inspect.Parameter.POSITIONAL_OR_KEYWORD).

In your example this clearly fails, as cls is POSITIONAL_ONLY.

The challenge comes to fixing this. To take this tortured but legal case:

class Lobster:
    @classmethod
    def spam(cls, ham, *, eggs: int) -> None:  # note: no '/'
        print(cls, ham, eggs)

Lobster.spam(1, eggs=1)
# <class '__main__.Lobster'> 1 1
Lobster.spam.__get__(Lobster(), Lobster).__func__(cls=Lobster, ham=1, eggs=1)
# <class '__main__.Lobster'> 1 1

I believe this means that we can't unilaterally switch to POSITIONAL_ONLY for the cls parameter.

A

@AA-Turner AA-Turner modified the milestones: some future version, 7.2.0 Aug 22, 2023
@randolf-scholz
Copy link

randolf-scholz commented Aug 22, 2023

@AA-Turner I think one simply needs a branching logic in

if bound_method and inspect.ismethod(obj):
# classmethods
cls = inspect.Parameter('cls', inspect.Parameter.POSITIONAL_OR_KEYWORD)
parameters.insert(0, cls)

something like:

KEYWORD_ONLY = Parameter.KEYWORD_ONLY
POSITIONAL_ONLY = Parameter.POSITIONAL_ONLY
POSITIONAL_OR_KEYWORD = Parameter.POSITIONAL_OR_KEYWORD
VAR_KEYWORD = Parameter.VAR_KEYWORD
VAR_POSITIONAL = Parameter.VAR_POSITIONAL

def has_po_args(func: Callable, /) -> bool:
    sig = inspect.signature(func)
    return POSITIONAL_ONLY in {p.kind for p in sig.parameters.values()}

[...]

if bound_method and inspect.ismethod(obj):
    # classmethods
    kind = POSITIONAL_ONLY if has_po_args(obj) else POSITIONAL_OR_KEYWORD
    cls = inspect.Parameter('cls', kind)
    parameters.insert(0, cls)

Talking about tortured cases, there is one edge-case not covered by this approach:

class Lobster:
    @classmethod
    def spam(cls, /, ham, *, eggs: int) -> None:  # note: no '/'
        print(cls, ham, eggs)

I think this is solved by inspecting Lobster.spam.__func__ directly instead of Lobster.spam, so:

def has_po_args(func: Callable, /) -> bool:
    if is_classmethod(func):
        sig = inspect.signature(func.__func__)
    else:
        sig = inspect.signature(func)
    return POSITIONAL_ONLY in {p.kind for p in sig.parameters.values()}

Implementing is_classmethod can be a bit tricky since many approaches might false-positives on otherwise decorated methods.
This one could work.

@AA-Turner
Copy link
Member

Sphinx 7.2.3 has been released with a fix for the issue mentioned by @randolf-scholz.

A

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants