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

autodoc/docutils woes in Python 3.13: autodoc-before-process-signature (update_annotations_using_type_comments), Block quote ends without a blank line #13232

Closed
pllim opened this issue Jan 11, 2025 · 23 comments

Comments

@pllim
Copy link

pllim commented Jan 11, 2025

Describe the bug

Hello. astropy has run into two problems I am not sure how to debug when trying to build doc using Sphinx in Python 3.13. These problems do not exist in Python 3.11 and 3.12.

  1. 'autodoc-before-process-signature' threw an exception (exception: list index out of range) [autodoc] originally reported in Building docs on Python 3.13.1 gives 124 warnings astropy/astropy#17614
  2. WARNING: Block quote ends without a blank line; unexpected unindent. [docutils]

How to Reproduce

See log in astropy/astropy#17621

Environment Information

Tried it with latest stable sphinx 8.1.3 and also sphinx dev built from source.

Sphinx extensions

# https://github.com/astropy/sphinx-astropy/blob/82ca928d0b49470deb32a6de3820fa0c0fcba5f3/sphinx_astropy/conf/v2.py#L184-L202

extensions = [
    'sphinx.ext.autodoc',
    'sphinx.ext.coverage',
    'sphinx.ext.inheritance_diagram',
    'sphinx.ext.intersphinx',
    'sphinx.ext.mathjax',
    'sphinx.ext.todo',
    'sphinx.ext.viewcode',
    'sphinxcontrib.jquery',
    'numpydoc',
    'sphinx_copybutton',
    'pytest_doctestplus.sphinx.doctestplus',
    'sphinx_astropy.ext.changelog_links',
    'sphinx_astropy.ext.generate_config',
    'sphinx_astropy.ext.intersphinx_toggle',
    'sphinx_astropy.ext.missing_static',
    'sphinx_automodapi.automodapi',
    'sphinx_automodapi.smart_resolver',
]
# ...
extensions += ['matplotlib.sphinxext.plot_directive']

# https://github.com/astropy/astropy/blob/77bb87d50a9944806312cefab2b96608482277a2/docs/conf.py#L137-L142

extensions += [
    "matplotlib.sphinxext.roles",
    "sphinx_changelog",
    "sphinx_design",
    "sphinxcontrib.globalsubs",
]
# ...
extensions += ["sphinx_gallery.gen_gallery"]

Additional context

Ideas on how to debug downstream would be very nice. The warnings/exceptions are a bit too cryptic for me to understand.

For instance, the autodoc one points to update_annotations_using_type_comments but I am not sure what that is supposed to do or how to disable it.

Thank you!

@AA-Turner
Copy link
Member

Ideas on how to debug downstream would be very nice. The warnings/exceptions are a bit too cryptic for me to understand.

To start with, ensure you are using --show-traceback to get the full Python traceback.

The strategy I tend to adopt is crudely bisecting the documentation by iteratively deleting large parts of it but ensuring that the warnings being investigated remain (ignoring issues caused by deleting parts of the documentation). I then try and minimise conf.py until it is hopefully empty or contains only e.g. extensions = ['sphinx.ext.autodoc'].


The update_annotations_using_type_comments error does seem odd, as it hasn't changed in several years:

def update_annotations_using_type_comments(app: Sphinx, obj: Any, bound_method: bool) -> None:
"""Update annotations info of *obj* using type_comments."""
try:
type_sig = get_type_comment(obj, bound_method)
if type_sig:
sig = inspect.signature(obj, bound_method)
for param in sig.parameters.values():
if param.name not in obj.__annotations__:
annotation = type_sig.parameters[param.name].annotation
if annotation is not Parameter.empty:
obj.__annotations__[param.name] = ast_unparse(annotation)
if 'return' not in obj.__annotations__:
obj.__annotations__['return'] = type_sig.return_annotation
except KeyError as exc:
logger.warning(__("Failed to update signature for %r: parameter not found: %s"),
obj, exc)
except NotImplementedError as exc: # failed to ast.unparse()
logger.warning(__("Failed to parse type_comment for %r: %s"), obj, exc)

A

@electric-coder
Copy link

electric-coder commented Jan 11, 2025

@pllim I'm betting (looking at the pattern) the culprit is Sphinx issue #13178 that you didn't link in this report but in the Astropy issues.

@pllim
Copy link
Author

pllim commented Jan 11, 2025

Hello and thanks for your response. I linked #13178 downstream for WARNING: py:class reference target not found: pathlib._local.Path but it is unclear to me how that would also affect autodoc-before-process-signature (update_annotations_using_type_comments) and Block quote ends without a blank line problems. 🤔

Anyways, I can try to dig more next week. Thanks, all!

@electric-coder
Copy link

electric-coder commented Jan 11, 2025

@pllim that's really it: the whole thing is unclear! I looked at the apparently affected modules for a type comment and there wasn't any, I looked at your conf.py but there wasn't an autodoc-before-process-signature event override (and because of that there's also no obvious place to put a try/catch)... So the pathlib._local.Path bug has that in common with your case: an error that doesn't give an exact line and has no obvious problem (only very indirect symptoms). What's likely happening is that the pathlib._local.Path bug is triggering indirect error messages somewhere else in the machinery.

I'd follow AA-Turner's advice of using --show-traceback and crudely deleting .rst modules until you can narrow the problem down to a minimum, and afterwards stripping the conf.py down to autodoc if possible (Astropy is huge but it's still likely the most efficient way of narrowing it down).

P.S. And the Block quote ends without a blank line also seems like gibberish, usually such an error would give an exact line in the .rst (altough Astropy packs everything into the docstrings which also makes error more opaque) if you didn't change the .rst and the only change was a Sphinx version update then the most likely culprit is the pathlib._local.Path bug.

@AA-Turner
Copy link
Member

@pllim a minimal reproducer:

import inspect, astropy.modeling

obj = astropy.modeling.Fittable1DModel.__call__
print(obj)
source = inspect.getsource(obj)
print(source)

In Python 3.11 and 3.12, this reports

            def __call__(self, *inputs, **kwargs):
                """Evaluate this model on the supplied inputs."""
                return super(cls, self).__call__(*inputs, **kwargs)

In Python 3.13, source == '\n'. As such, autosummary and autodoc can't document anything because we can't fetch the source. This may need to be reported upstream as a bug.

In Sphinx, we could just gracefully fail here rather than emmitting a warning, though this would hide a real problem. What do you think?

A

@pllim
Copy link
Author

pllim commented Jan 13, 2025

we could just gracefully fail here rather than emmitting a warning, though this would hide a real problem

As a downstream user, warning is better than silently failing, so I would rather Sphinx keep the warning until upstream fix can be done. But I do wonder if the warning can be clearer or that is hard to do in this situation. 🤔

Thanks for tracking that down! That is very weird indeed. inspect.getsource(astropy.modeling.Fittable1DModel.__mul__) seems to work so it is not affecting all dunder methods. And astropy.modeling.Fittable1DModel.__call__.__doc__ does exist. 🤯

FWIW this is what it is supposed to look like when successful: https://docs.astropy.org/en/v7.0.0/api/astropy.modeling.Fittable1DModel.html#astropy.modeling.Fittable1DModel.__call__

Image

@AA-Turner
Copy link
Member

AA-Turner commented Jan 13, 2025

I think the problem is the metaclass. Perhaps related to the new __firstlineno__ attribute (python/cpython#118465). I've not had time to fully minimise but this is reproducable without astropy:

# lasagna/__init__.py
import abc
import inspect
import keyword
import os


def make_function_with_signature(func, args=(), kwargs={}, varargs=None, varkwargs=None, name=None):
    pos_args = []
    key_args = []

    if isinstance(kwargs, dict):
        iter_kwargs = kwargs.items()
    else:
        iter_kwargs = iter(kwargs)

    # Check that all the argument names are valid
    for item in (*args, *iter_kwargs):
        if isinstance(item, tuple):
            argname = item[0]
            key_args.append(item)
        else:
            argname = item
            pos_args.append(item)

        if keyword.iskeyword(argname) or not argname.isascii() or not argname.isidentifier():
            raise SyntaxError(f"invalid argument name: {argname}")

    for item in (varargs, varkwargs):
        if item is not None:
            if keyword.iskeyword(item) or not argname.isascii() or not argname.isidentifier():
                raise SyntaxError(f"invalid argument name: {item}")

    def_signature = [", ".join(pos_args)]

    if varargs:
        def_signature.append(f", *{varargs}")

    call_signature = def_signature[:]

    if name is None:
        name = func.__name__

    global_vars = {f"__{name}__func": func}
    local_vars = {}
    # Make local variables to handle setting the default args
    for idx, item in enumerate(key_args):
        key, value = item
        default_var = f"_kwargs{idx}"
        local_vars[default_var] = value
        def_signature.append(f", {key}={default_var}")
        call_signature.append(f", {key}={key}")

    if varkwargs:
        def_signature.append(f", **{varkwargs}")
        call_signature.append(f", **{varkwargs}")

    def_signature = "".join(def_signature).lstrip(", ")
    call_signature = "".join(call_signature).lstrip(", ")

    frm = inspect.currentframe()
    depth = 2
    for i in range(depth):
        frm = frm.f_back
        if frm is None:
            return None

    mod = inspect.getmodule(frm)
    assert mod is not None, frm
    frm = inspect.currentframe().f_back

    if mod:
        filename = mod.__file__
        modname = mod.__name__
        if filename.endswith(".pyc"):
            filename = os.path.splitext(filename)[0] + ".py"
    else:
        filename = "<string>"
        modname = "__main__"

    # Subtract 2 from the line number since the length of the template itself
    # is two lines.  Therefore we have to subtract those off in order for the
    # pointer in tracebacks from __{name}__func to point to the right spot.
    lineno = frm.f_lineno - 2

    # The lstrip is in case there were *no* positional arguments (a rare case)
    # in any context this will actually be used...
    template = """{0}\
def {name}({sig1}):
    return __{name}__func({sig2})
""".format("\n" * lineno, name=name, sig1=def_signature, sig2=call_signature)

    code = compile(template, filename, "single")

    eval(code, global_vars, local_vars)

    new_func = local_vars[name]
    new_func.__module__ = modname
    new_func.__doc__ = func.__doc__

    return new_func


class _ModelMeta(abc.ABCMeta):
    def __init__(cls, name, bases, members, **kwds):
        super().__init__(name, bases, members, **kwds)

        # Handle init creation from inputs
        def update_wrapper(wrapper, cls):
            # Set up the new __call__'s metadata attributes as though it were
            # manually defined in the class definition
            # A bit like functools.update_wrapper but uses the class instead of
            # the wrapped function
            wrapper.__module__ = cls.__module__
            wrapper.__doc__ = getattr(cls, wrapper.__name__).__doc__
            if hasattr(cls, "__qualname__"):
                wrapper.__qualname__ = f"{cls.__qualname__}.{wrapper.__name__}"

        # THIS BIT!!!!
        if (
            "__call__" not in members
            and "n_inputs" in members
            and isinstance(members["n_inputs"], int)
            and members["n_inputs"] > 0
        ):
            # Don't create a custom __call__ for classes that already have one
            # explicitly defined (this includes the Model base class, and any
            # other classes that manually override __call__

            def __call__(self, *inputs, **kwargs):
                """Evaluate this model on the supplied inputs."""
                return super(cls, self).__call__(*inputs, **kwargs)

            # When called, models can take two optional keyword arguments:
            #
            # * model_set_axis, which indicates (for multi-dimensional input)
            #   which axis is used to indicate different models
            #
            # * equivalencies, a dictionary of equivalencies to be applied to
            #   the input values, where each key should correspond to one of
            #   the inputs.
            #
            # The following code creates the __call__ function with these
            # two keyword arguments.

            args = ("self",)
            kwargs = {
                "model_set_axis": None,
                "with_bounding_box": False,
                "equivalencies": None,
                "inputs_map": None,
            }

            new_call = make_function_with_signature(
                __call__, args, kwargs, varargs="inputs", varkwargs="new_inputs"
            )

            # The following makes it look like __call__
            # was defined in the class
            update_wrapper(new_call, cls)

            cls.__call__ = new_call

        # THIS BIT!!!!
        if (
            "__init__" not in members
            and not inspect.isabstract(cls)
            #and cls._parameters_
        ):
            # Build list of all parameters including inherited ones

            # If *all* the parameters have default values we can make them
            # keyword arguments; otherwise they must all be positional
            # arguments
            args = ("self",)
            kwargs = []

            def __init__(self, *params, **kwargs):
                return super(cls, self).__init__(*params, **kwargs)

            new_init = make_function_with_signature(
                __init__, args, kwargs, varkwargs="kwargs"
            )
            update_wrapper(new_init, cls)
            cls.__init__ = new_init


class Model(metaclass=_ModelMeta):
    def __init_subclass__(cls, **kwargs):
        super().__init_subclass__()

    def __call__(self, *args, **kwargs):
        """
        Evaluate this model using the given input(s) and the parameter values
        that were specified when the model was instantiated.
        """


class FittableModel(Model):
    """
    Base class for models that can be fitted using the built-in fitting
    algorithms.
    """

    linear = False
    fit_deriv = None
    col_fit_deriv = True
    fittable = True


class Fittable1DModel(FittableModel):
    """
    Base class for one-dimensional fittable models.

    This class provides an easier interface to defining new models.
    Examples can be found in `astropy.modeling.functional_models`.
    """

    n_inputs = 1
    n_outputs = 1
    _separable = True
# docs/bug.py
import sys
sys.path.append('.')
import inspect, lasagna


obj = lasagna.Fittable1DModel.__call__
print(obj)
source = inspect.getsource(obj)
print(source)
print(source == '\n')

Then run either py -3.13 docs/bug.py or py -3.12 docs/bug.py.

A

@electric-coder
Copy link

@pllim yep, that's what happens someone strays into too much metaclass hackery. It's fine and dandy at first, but eventually things break and just the diagnosis alone is mind boggling.

@pllim
Copy link
Author

pllim commented Jan 13, 2025

All I can say in its defense is... I didn't write it. 😆

@pllim
Copy link
Author

pllim commented Jan 13, 2025

More seriously, let me ping astropy.modeling maintainers and see if they have insight. Thanks for helping us debug this! 🙏

@nden @perrygreenfield @WilliamJamieson

@WilliamJamieson
Copy link

This is likely going to be a pain to fix, thanks for the insight into what we may have to do!

(Maybe this will push us to finally clean up all the metaclass stuff in modeling, its something I've wanted to do since I started working on it)

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 21, 2025

Workaround in the meantime 👍

from typing import TYPE_CHECKING

from sphinx.domains.python import PythonDomain

if TYPE_CHECKING:
    from docutils.nodes import Element
    from sphinx.application import Sphinx
    from sphinx.builders import Builder
    from sphinx.environment import BuildEnvironment

def setup(app: Sphinx) -> None:  # noqa: D103
    class PatchedPythonDomain(PythonDomain):
        def resolve_xref(  # noqa: PLR0913,PLR0917
            self,
            env: BuildEnvironment,
            fromdocname: str,
            builder: Builder,
            type: str,  # noqa: A002
            target: str,
            node: resolve_xref,
            contnode: Element,
        ) -> Element:
            # fixup some wrongly resolved mappings
            mapping = {
                "pathlib._local.Path": "pathlib.Path",
            }
            if target in mapping:
                target = node["reftarget"] = mapping[target]
            return super().resolve_xref(env, fromdocname, builder, type, target, node, contnode)

    app.add_domain(PatchedPythonDomain, override=True)

pllim added a commit to pllim/astropy that referenced this issue Jan 22, 2025
@pllim
Copy link
Author

pllim commented Jan 23, 2025

Thanks, @gaborbernat ! I tried your workaround in astropy/astropy#17621 but alas, most of the warnings persisted. Is your workaround only for the Path warning?

@gaborbernat
Copy link
Contributor

Yes, you can add others in the mapping as you need it.

@AA-Turner
Copy link
Member

A better workaround for the pathlib._local.Path problem (though still hacky) is:

import sys
import sphinx

if sys.version_info[:2] >= (3, 13) and sphinx.version_info[:2] < (8, 2):
    import pathlib

    from sphinx.util.typing import _INVALID_BUILTIN_CLASSES

    _INVALID_BUILTIN_CLASSES[pathlib.Path] = 'pathlib.Path'  # type: ignore

A

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 23, 2025 via email

@AA-Turner
Copy link
Member

AA-Turner commented Jan 23, 2025

@gaborbernat please could you elaborate? The method I showed means that the links to pathlib.Path are created, _INVALID_BUILTIN_CLASSES is the method Sphinx uses for Python types with __module__ attributes that aren't equal to the import module.

A

@gaborbernat
Copy link
Contributor

Ah, I'm mistaken; they're likely equaly good. Note sure how forward compatible your is, but I believe should work (small downside needing a type ignore).

@AA-Turner
Copy link
Member

Note sure how forward compatible your is

Committed in #13261, so the patch reflects behaviour that will be in Sphinx 8.2.

A

@pllim
Copy link
Author

pllim commented Jan 23, 2025

Thank you for the suggestions. However the Path thing was the least of my problem and also a separate issue already.

Back to the __call__ inspection failure, it sounds like we have no fix except having to refactor astropy.modeling internals?

As for the 2 docstring weirdness from Table about block quote, does not look like the suggestions can help in any way.

@AA-Turner
Copy link
Member

@pllim see astropy/astropy#17659 which I think fixes the codegen issue.

A

@AA-Turner
Copy link
Member

Closing, nothing to resolve in Sphinx.

A

@AA-Turner AA-Turner closed this as not planned Won't fix, can't repro, duplicate, stale Jan 23, 2025
@pllim
Copy link
Author

pllim commented Jan 24, 2025

Thanks, @AA-Turner !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants