sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.45k stars 2.1k forks source link

Sphinx 7.4: autodoc: ``typing.Annotated`` failure with pydantic #12601

Closed AA-Turner closed 2 months ago

AA-Turner commented 2 months ago

I think I found a regression about this. We're using the Annotated class + pydantic to validate types of a user-defined config in conf.py...

CSSClassType = Annotated[str, pydantic.AfterValidator(nodes.make_id)]

class CustomAdmonitionConfig(pydantic.BaseModel):
    title: Annotated[Optional[str], pydantic.Field(validate_default=True)] = None
    classes: List[CSSClassType] = []

But autodoc-ing CustomAdmonitionConfig's title and classes attributes results in errors trying to reference the metadata:

/opt/hostedtoolcache/Python/3.12.4/x64/lib/python3.12/site-packages/sphinx_immaterial/custom_admonitions.py:docstring of sphinx_immaterial.custom_admonitions.CustomAdmonitionConfig.title:1: WARNING: py:obj reference target not found: typing.Annotated[str | None, FieldInfo(annotation=NoneType, required=True, validate_default=True)]

/opt/hostedtoolcache/Python/3.12.4/x64/lib/python3.12/site-packages/sphinx_immaterial/custom_admonitions.py:docstring of sphinx_immaterial.custom_admonitions.CustomAdmonitionConfig.classes:1: WARNING: py:obj reference target not found: typing.List[~typing.Annotated[str, AfterValidator(func=<function make_id at 0x7fd2a7b5cae0>)]]

I think lesson here is that not all metadata is compatible with autodoc. Is there a way to disable it?

Originally posted by @2bndy5 in https://github.com/sphinx-doc/sphinx/issues/11785#issuecomment-2230045019

2bndy5 commented 2 months ago

I'm not sure there's anything autodoc or intersphinx can do about this. Pydantic doesn't use sphinx to generate the their docs (sadly).

Honestly, this "feature" should be hidden behind a config option until if/when it is stable to use in autodoc.

AA-Turner commented 2 months ago

Hi @2bndy5 are you able to help with a reproducer? I currently have the below, which doesn't generate the cited warnings/errors:

conf.py

extensions = ['sphinx.ext.autodoc']
nitpicky = True

index.rst

.. automodule:: bug
   :members:

bug.py

from typing import List, Optional, Annotated

from docutils import nodes
import pydantic

CSSClassType = Annotated[str, pydantic.AfterValidator(nodes.make_id)]

class CustomAdmonitionConfig(pydantic.BaseModel):
    title: Annotated[Optional[str], pydantic.Field(validate_default=True)] = None
    classes: List[CSSClassType] = []

Output:

PS> python -m sphinx build -M html . _build -T
<...output elided...>
...\bug.py:docstring of bug.CustomAdmonitionConfig.model_computed_fields:1: WARNING: py:class reference target not found: ComputedFieldInfo
...\bug.py:docstring of bug.CustomAdmonitionConfig.model_config:1: WARNING: py:class reference target not found: ConfigDict
...\bug.py:docstring of bug.CustomAdmonitionConfig.model_fields:1: WARNING: py:class reference target not found: FieldInfo
<...output elided...>

A

2bndy5 commented 2 months ago

Example was taken straight from sphinx-immaterial docs. Maybe the theme does something extra to return types... I'll look into it.

2bndy5 commented 2 months ago

I haven't narrowed it down yet, but I was able to suppress the warnings using autoattribute's :annotation: option:

   .. autoclass:: sphinx_immaterial.custom_admonitions.CustomAdmonitionConfig
      :exclude-members: __new__, __init__

      .. autoattribute:: name
      .. autoattribute:: title
         :annotation: : str | None
      .. autoattribute:: icon
      .. autoattribute:: color
      .. autoattribute:: classes
         :annotation: : List[str]
      .. autoattribute:: override
2bndy5 commented 2 months ago
Some theme-specific details. So, the sphinx-immaterial theme (in `sphinx_immaterial.apidoc.python.attribute_style`) monkey-patches the `handle_signature()` function of `sphinx.domains.python.PyAttribute` (and `PyProperty`) to 1. add a punctuation to the signature - `sphinx.addnodes.desc_sig_punctuation("", " : ")` for a type - `sphinx.addnodes.desc_sig_punctuation("", " = ")` for a value 2. convert the signature's type into xrefs using a monkey-patched `sphinx.domains.python._parse_annotation()` I'll discuss a solution downstream. It doesn't seem like the problem is specific to our monkey patches (when comparing the original code with the patched code).

@AA-Turner The example you posted is sufficient to reproduce, but we build our docs with both --nitpicky --fail-on-warning options enabled.

I can't just ignore a nitpicky warning that shows a function's memory offset (the __str__() output of docutils.nodes.make_id()) because the offset is different on every build, That is unless the nitpicky_ignore field supports a glob or regex pattern of which I'm unaware.

AA-Turner commented 2 months ago

That is unless the nitpicky_ignore field supports a glob or regex pattern of which I'm unaware.

Today is your lucky day: https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-nitpick_ignore_regex

A

AA-Turner commented 2 months ago

I would still be interested in a reproducer that has new failures on 7.4 compared to 7.3 so that I can release a fix, if you have one.

A

2bndy5 commented 2 months ago

I ended up factoring out our use of the Annotated class as an attribute type (see https://github.com/jbms/sphinx-immaterial/pull/370/commits/a1b0bfd29dfc98965c6394ed7eef46eff81f1c3c), at least for the documented API. Pydantic is refreshingly flexible. It just took a little more code to achieve the same behavior.

Today is your lucky day

I'm glad to see I have multiple options to recommend others that may come across this (or a similar) problem. 👍🏼

I would still be interested in a reproducer

You're example does reproduce it but it needs the --fail-on-warning option added to sphinx-build command. Just to be proper, I packaged it up into a zip file (with a requirements.txt file) and added to :members: for conciseness.

Steps: to reproduce

  1. extract the zip: issue#12601.zip
  2. navigate to "issue#12601" directory
  3. pip install -r requirements.txt
  4. sphinx-build . ./_build -W --keep-going

release a fix, if you have one.

I don't have a fix other than ways to avoid the warning. I just don't know the domain API well enough. Downstream, we were considering a theme-specific config option to help filter the transformations of type annotations (see our docs for options already implemented), but nitpicky warnings are often ignored in the wild.

AA-Turner commented 2 months ago

Edit: The below is wrong, there are mitigations we can apply within Sphinx.


I've made some progress and there is a mitigation we can apply, but part of the problem is with Pydantic.

Sphinx tries to parse a function annotation and from there convert the components within autodoc.

>>> import ast
>>> annotation ='~typing.List[~typing.Annotated[str, AfterValidator(func=<function make_id at 0x0000020883233100>)]]'
>>> ast.parse(annotation, type_comments=True)
[snip]
                                                            ^
SyntaxError: invalid syntax
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "S:\Micromamba\envs\sphinx\Lib\site-packages\IPython\core\interactiveshell.py", line 3577, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-7-ce0ec65a4b87>", line 1, in <module>
    ast.parse('~typing.List[~typing.Annotated[str, AfterValidator(func=<function make_id at 0x0000020883233100>)]]', type_comments=True)
  File "S:\Micromamba\envs\sphinx\Lib\ast.py", line 52, in parse
    return compile(source, filename, mode, flags,
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<unknown>", line 1
    ~typing.List[~typing.Annotated[str, AfterValidator(func=<function make_id at 0x0000020883233100>)]]
                                                            ^
SyntaxError: invalid syntax

Ideally, given that pydantic ought know that func is a function or callable, they would implement a custom __repr__() that returns f'{func.__module__}.{func.__qualname__}'.

A

2bndy5 commented 2 months ago

Ideally for sphinx maybe, but is it really pydantic's responsibility to provide representation of function pointers specified from user-space?

I don't think falling back to __repr__() is the best last resort, especially if Python's default implementation does not include the info that ast.parse() is looking for. It my understanding that the ast module's source is beyond the scope of Sphinx (as with the inspect module).


There is a similar problem with using an instantiated object as a parameter default value.

# in defaults.py
class A:
    """Some class A doc"""
    pass

def some_fn(param: A = A()):
    """Some function doc"""
    pass

image

We've come across yet another seemingly similar problem in https://github.com/jbms/sphinx-immaterial/issues/140

AA-Turner commented 2 months ago

https://github.com/AA-Turner/sphinx/tree/fix-annotated-autodoc has my working progress.

There are two parts to fix; autodoc's generation of reStructuredText source, and then the Python domain's parsing and resolution of those targets. I have the second fixed, but not the first.

A

AA-Turner commented 2 months ago

Sphinx 7.4.7 has been released with a fix.

A