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

bug: napoleon: numpydoc: incorrect section ordering for classes #13180

Open
lucascolley opened this issue Dec 16, 2024 · 5 comments
Open

bug: napoleon: numpydoc: incorrect section ordering for classes #13180

lucascolley opened this issue Dec 16, 2024 · 5 comments
Labels
Milestone

Comments

@lucascolley
Copy link

lucascolley commented Dec 16, 2024

Describe the bug

As noted in the release notes for numpydoc 1.8.0, the section ordering was updated in numpy/numpydoc#571 for classes, to move the Attributes and Methods sections directly below the Parameters section.

Using sphinx.ext.napoleon attributes and methods are still generated after the Notes and Examples sections.

How to Reproduce

"""Sphinx config."""

import importlib.metadata
from typing import Any

project = "array-api-extra"
version = release = importlib.metadata.version("array_api_extra")

extensions = [
    "myst_parser",
    "sphinx.ext.autodoc",
    "sphinx.ext.autosummary",
    "sphinx.ext.intersphinx",
    "sphinx.ext.mathjax",
    "sphinx.ext.napoleon",
    "sphinx_autodoc_typehints",
    "sphinx_copybutton",
]

source_suffix = [".rst", ".md"]
exclude_patterns = [
    "_build",
    "**.ipynb_checkpoints",
    "Thumbs.db",
    ".DS_Store",
    ".env",
    ".venv",
]

html_theme = "furo"

html_theme_options: dict[str, Any] = {
    "footer_icons": [
        {
            "name": "GitHub",
            "url": "https://github.com/data-apis/array-api-extra",
            "html": """
                <svg stroke="currentColor" fill="currentColor" stroke-width="0" viewBox="0 0 16 16">
                    <path fill-rule="evenodd" d="M8 0C3.58 0 0 3.58 0 8c0 3.54 2.29 6.53 5.47 7.59.4.07.55-.17.55-.38 0-.19-.01-.82-.01-1.49-2.01.37-2.53-.49-2.69-.94-.09-.23-.48-.94-.82-1.13-.28-.15-.68-.52-.01-.53.63-.01 1.08.58 1.23.82.72 1.21 1.87.87 2.33.66.07-.52.28-.87.51-1.07-1.78-.2-3.64-.89-3.64-3.95 0-.87.31-1.59.82-2.15-.08-.2-.36-1.02.08-2.12 0 0 .67-.21 2.2.82.64-.18 1.32-.27 2-.27.68 0 1.36.09 2 .27 1.53-1.04 2.2-.82 2.2-.82.44 1.1.16 1.92.08 2.12.51.56.82 1.27.82 2.15 0 3.07-1.87 3.75-3.65 3.95.29.25.54.73.54 1.48 0 1.07-.01 1.93-.01 2.2 0 .21.15.46.55.38A8.013 8.013 0 0 0 16 8c0-4.42-3.58-8-8-8z"></path>
                </svg>
            """,  # noqa: E501
            "class": "",
        },
    ],
    "source_repository": "https://github.com/data-apis/array-api-extra",
    "source_branch": "main",
    "source_directory": "docs/",
}

myst_enable_extensions = [
    "colon_fence",
]

intersphinx_mapping = {
    "python": ("https://docs.python.org/3", None),
    "jax": ("https://jax.readthedocs.io/en/latest", None),
}

nitpick_ignore = [
    ("py:class", "_io.StringIO"),
    ("py:class", "_io.BytesIO"),
]

always_document_param_types = True
class at:  # pylint: disable=invalid-name  # numpydoc ignore=PR02
    """
    Update operations for read-only arrays.

    This implements ``jax.numpy.ndarray.at`` for all writeable
    backends (those that support ``__setitem__``) and routes
    to the ``.at[]`` method for JAX arrays.

    Parameters
    ----------
    x : array
        Input array.
    idx : index, optional
        Only `array API standard compliant indices
        <https://data-apis.org/array-api/latest/API_specification/indexing.html>`_
        are supported.

        You may use two alternate syntaxes::

          at(x, idx).set(value)  # or add(value), etc.
          at(x)[idx].set(value)
    copy : bool, optional
        True (default)
            Ensure that the inputs are not modified.
        False
            Ensure that the update operation writes back to the input.
            Raise ``ValueError`` if a copy cannot be avoided.
        None
            The array parameter *may* be modified in place if it is
            possible and beneficial for performance.
            You should not reuse it after calling this function.
    xp : array_namespace, optional
        The standard-compatible namespace for `x`. Default: infer.

    Returns
    -------
    Updated input array.

    Warnings
    --------
    (a) When you use ``copy=None``, you should always immediately overwrite
    the parameter array::

        x = at(x, 0).set(2, copy=None)

    The anti-pattern below must be avoided, as it will result in different
    behaviour on read-only versus writeable arrays::

        x = xp.asarray([0, 0, 0])
        y = at(x, 0).set(2, copy=None)
        z = at(x, 1).set(3, copy=None)

    In the above example, ``x == [0, 0, 0]``, ``y == [2, 0, 0]`` and z == ``[0, 3, 0]``
    when ``x`` is read-only, whereas ``x == y == z == [2, 3, 0]`` when ``x`` is
    writeable!

    (b) The array API standard does not support integer array indices.
    The behaviour of update methods when the index is an array of integers is
    undefined and will vary between backends; this is particularly true when the
    index contains multiple occurrences of the same index, e.g.::

        >>> import numpy as np
        >>> import jax.numpy as jnp
        >>> at(np.asarray([123]), np.asarray([0, 0])).add(1)
        array([124])
        >>> at(jnp.asarray([123]), jnp.asarray([0, 0])).add(1)
        Array([125], dtype=int32)

    See Also
    --------
    jax.numpy.ndarray.at : Equivalent array method in JAX.

    Notes
    -----
    `sparse <https://sparse.pydata.org/>`_, as well as read-only arrays from libraries
    not explicitly covered by ``array-api-compat``, are not supported by update
    methods.

    Examples
    --------
    Given either of these equivalent expressions::

      x = at(x)[1].add(2, copy=None)
      x = at(x, 1).add(2, copy=None)

    If x is a JAX array, they are the same as::

      x = x.at[1].add(2)

    If x is a read-only numpy array, they are the same as::

      x = x.copy()
      x[1] += 2

    For other known backends, they are the same as::

      x[1] += 2
    """
    _x: Array
    _idx: Index
    __slots__: ClassVar[tuple[str, ...]] = ("_idx", "_x")

    def __init__(
        self, x: Array, idx: Index = _undef, /
    ) -> None:  # numpydoc ignore=GL08
        self._x = x
        self._idx = idx

    def __getitem__(self, idx: Index, /) -> "at":  # numpydoc ignore=PR01,RT01
        """
        Allow for the alternate syntax ``at(x)[start:stop:step]``.

        It looks prettier than ``at(x, slice(start, stop, step))``
        and feels more intuitive coming from the JAX documentation.
        """
        if self._idx is not _undef:
            msg = "Index has already been set"
            raise ValueError(msg)
        self._idx = idx
        return self

    def _update_common(
        self,
        at_op: str,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> tuple[Array, None] | tuple[None, Array]:  # numpydoc ignore=PR01
        """
        Perform common prepocessing to all update operations.

        Returns
        -------
        tuple
            If the operation can be resolved by ``at[]``, ``(return value, None)``
            Otherwise, ``(None, preprocessed x)``.
        """
        x, idx = self._x, self._idx

        if idx is _undef:
            msg = (
                "Index has not been set.\n"
                "Usage: either\n"
                "    at(x, idx).set(value)\n"
                "or\n"
                "    at(x)[idx].set(value)\n"
                "(same for all other methods)."
            )
            raise ValueError(msg)

        if copy not in (True, False, None):
            msg = f"copy must be True, False, or None; got {copy!r}"  # pyright: ignore[reportUnreachable]
            raise ValueError(msg)

        if copy is None:
            writeable = is_writeable_array(x)
            copy = not writeable
        elif copy:
            writeable = None
        else:
            writeable = is_writeable_array(x)

        if copy:
            if is_jax_array(x):
                # Use JAX's at[]
                func = getattr(x.at[idx], at_op)
                return func(y), None
            # Emulate at[] behaviour for non-JAX arrays
            # with a copy followed by an update
            if xp is None:
                xp = array_namespace(x)
            x = xp.asarray(x, copy=True)
            if writeable is False:
                # A copy of a read-only numpy array is writeable
                # Note: this assumes that a copy of a writeable array is writeable
                writeable = None

        if writeable is None:
            writeable = is_writeable_array(x)
        if not writeable:
            # sparse crashes here
            msg = f"Can't update read-only array {x}"
            raise ValueError(msg)

        return None, x

    def set(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] = y`` and return the update array."""
        res, x = self._update_common("set", y, copy=copy, xp=xp)
        if res is not None:
            return res
        assert x is not None
        x[self._idx] = y
        return x

    def _iop(
        self,
        at_op: Literal[
            "set", "add", "subtract", "multiply", "divide", "power", "min", "max"
        ],
        elwise_op: Callable[[Array, Array], Array],
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """
        ``x[idx] += y`` or equivalent in-place operation on a subset of x.

        which is the same as saying
            x[idx] = x[idx] + y
        Note that this is not the same as
            operator.iadd(x[idx], y)
        Consider for example when x is a numpy array and idx is a fancy index, which
        triggers a deep copy on __getitem__.
        """
        res, x = self._update_common(at_op, y, copy=copy, xp=xp)
        if res is not None:
            return res
        assert x is not None
        x[self._idx] = elwise_op(x[self._idx], y)
        return x

    def add(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] += y`` and return the updated array."""
        return self._iop("add", operator.add, y, copy=copy, xp=xp)

    def subtract(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] -= y`` and return the updated array."""
        return self._iop("subtract", operator.sub, y, copy=copy, xp=xp)

    def multiply(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] *= y`` and return the updated array."""
        return self._iop("multiply", operator.mul, y, copy=copy, xp=xp)

    def divide(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] /= y`` and return the updated array."""
        return self._iop("divide", operator.truediv, y, copy=copy, xp=xp)

    def power(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] **= y`` and return the updated array."""
        return self._iop("power", operator.pow, y, copy=copy, xp=xp)

    def min(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] = minimum(x[idx], y)`` and return the updated array."""
        if xp is None:
            xp = array_namespace(self._x)
        y = xp.asarray(y)
        return self._iop("min", xp.minimum, y, copy=copy, xp=xp)

    def max(
        self,
        y: Array,
        /,
        copy: bool | None = True,
        xp: ModuleType | None = None,
    ) -> Array:  # numpydoc ignore=PR01,RT01
        """Apply ``x[idx] = maximum(x[idx], y)`` and return the updated array."""
        if xp is None:
            xp = array_namespace(self._x)
        y = xp.asarray(y)
        return self._iop("max", xp.maximum, y, copy=copy, xp=xp)

Environment Information

Platform:              darwin; (macOS-14.6.1-arm64-arm-64bit-Mach-O)
Python version:        3.13.1 | packaged by conda-forge | (main, Dec  5 2024, 21:09:18) [Clang 18.1.8 ])
Python implementation: CPython
Sphinx version:        8.1.3
Docutils version:      0.21.2
Jinja2 version:        3.1.4
Pygments version:      2.18.0

Sphinx extensions

No response

Additional context

No response

@electric-coder
Copy link

electric-coder commented Dec 16, 2024

  1. Please trim the code to make it a Minimal Reproducible Example. The example you posted seems more like a code dump than something minimal.
  2. You don't explain what's the point you're trying to make.... Numpydoc isn't Sphinx, it's an extension to Sphinx so why shouldn't the extension be handling those changes?
  3. You don't include a screenshot of the proposed changes for ease of reading.
  4. Isn't rendered order controlled by declaration order (also considering the complex case of mixing docstring with reST declarations)? In that scenario how to you propose mixed declaration order be solved (again the form of your proposal is very shallow for not being self-explanatory of the more complex use cases).

@lucascolley
Copy link
Author

thanks for taking a look, I can try to address points (1) and (3) when I find the time.

You don't explain what's the point you're trying to make.... Numpydoc isn't Sphinx, it's an extension to Sphinx so why shouldn't the extension be handling those changes?

To quote the Napoleon docs:

Napoleon is a extension that enables Sphinx to parse both NumPy and Google style docstrings

My understanding is that Napoleon should format the docstrings as Numpydoc would. Unless it has its own separate style? If so, it would be good to make that clear in the docs.

@electric-coder

This comment was marked as outdated.

@lucascolley

This comment was marked as outdated.

@AA-Turner
Copy link
Member

Hi Lucas (@lucascolley), Happy New Year!

I agree that Sphinx should update the style to reflect what is defined by NumPy. This is what Napoleon links to internally, so I think this is the best course of action.

Happy to accept a PR, please tag me.

A

@AA-Turner AA-Turner added this to the 8.2.0 milestone Jan 5, 2025
@AA-Turner AA-Turner added good first issue easy issue that can be solved by beginners labels Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants