Allow multiple `Warning(s)` sections

[lucascolley]

Using the specified Warnings section I can get something that looks like this:

It would be nice to be able to split this into two separate admonitions. It feels natural to me to have two separate headers, both titled Warning.

[lucascolley]

It seems like sphinx.ext.napoleon supports this: https://www.sphinx-doc.org/en/master/usage/extensions/napoleon.html#docstring-sections

[lucascolley]

napoleon parses warning blocks as admonitions (at least for GoogleDocstring): https://github.com/sphinx-doc/sphinx/blob/c4929d026c8d22ba229b39cfc2250a9eb1476282/sphinx/ext/napoleon/docstring.py#L400

[rossbar]

IMO the Warnings section shouldn't exist... the sphinx built-in .. warning:: directive is preferable. Since it does exist it may be straightforward to just parse the Warnings heading(s) and replace it with .. warning::.

[stefanv]

It does feel like Warnings should have been the spelling for Warns. But, given that we didn't do that, I agree with @rossbar's assessment.

[larsoner]

To me this seems a bit off from the stated purpose of the section, which to me is to enumerate and describe the Warning subclasses that can be warnings.warn'ed by the called function. From the doc:

An optional section detailing which warnings get raised and under what conditions, formatted similarly to Raises.

So the idea is that the section should be similar to the Raises one, e.g., something like:

Warnings
--------
RuntimeWarning
    Emitted if the denominator is zero. In this case, NaN will be returned.
UserWarning
    Emitted when blah blah blah.

With this in mind, there is some overlap potentially with when people would want to use the .. warning:: admonition sometimes, but to me the goals are not sufficiently distinct that I think we should keep the Warnings section.

More specific to the OP's use case, I think they should use .. warning:: for that whole box somewhere in the Notes subsection, since it has to do with a potential usage "gotcha" rather than with what warnings can be emitted by calling the function. So for that use case, there would no Warnings subheading use at all.

If this makes sense, we should probably add some text to clarify the intended use of the Warnings section.

[larsoner]

Ooof wait I was reading the description of the Warns section! Agreed that Warnings probably does not make sense 🤦

[lucascolley]

Yes, this is about Warnings, not Warns 👍

IMO the Warnings section shouldn't exist... the sphinx built-in .. warning:: directive is preferable.

they should use .. warning:: for that whole box somewhere in the Notes subsection, since it has to do with a potential usage "gotcha" rather than with what warnings can be emitted by calling the function. So for that use case, there would no Warnings subheading use at all.

I think I tried this initially, and IIRC it worked under the Notes header, but not under other headers. The problem with that is that one might want the warnings to show higher up the page than Notes, e.g. right below Parameters (or at least, above Notes and See Also, as per the current spec).

I might be misremembering, though—feel free to play around with the source of https://data-apis.org/array-api-extra/generated/array_api_extra.at.html#array_api_extra.at and pixi run docs if there is an easier solution 👍

[rossbar]

I think I tried this initially, and IIRC it worked under the Notes header, but not under other headers. The problem with that is that one might want the warnings to show higher up the page than Notes, e.g. right below Parameters (or at least, above Notes and See Also, as per the current spec).

This is another reason to prefer the .. warning:: directive. I haven't tested, but in principle this should show up at the exact location in the docstring where you place it. In other words if you have something like:

def foo(x, y):
    """foo's basic description.

    Foo's extended description

    .. warning:: ``foo`` is way too common a name.

    Parameters
    ----------
    x : scalar
        A value
        .. warning:: ``x`` is also too generic
    y : scalar
    
    Notes
    -----
    This function is an example to show multiple warnings in
    docstrings.
    
    .. warning:: here's another

Then you should get three separate warning admonitions rendered in correct locations (the extended summary, the parameter description, and the notes section, respectively)

If you instead rely on a Warnings section, numpydoc will grab that and move it to the "Warnings" location in the docstring (i.e. after Warns but before See Also).

Without having looked at it too closely, I suspect it would be possible to break this pattern to allow multiple Warnings sections and preserve their location in the docstring, but I'd much prefer to "deprecate by documentation" instead and just recommend folks use the .. warning:: directive instead of the Warnings headaing.

[lucascolley]

I tried:

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

    .. warning::

        When you omit the ``copy`` parameter, you should never reuse the parameter
        array later on; ideally, you should reassign it immediately::

            >>> import array_api_extra as xpx
            >>> x = xpx.at(x, 0).set(2)

        The above best practice pattern ensures that the behaviour won't change depending
        on whether ``x`` is writeable or not, as the original ``x`` object is dereferenced
        as soon as ``xpx.at`` returns; this way there is no risk to accidentally update it
        twice.

But that doesn't work:

[lucascolley]

it may be important to mention that I am using "sphinx.ext.napoleon" rather than numpydoc for the docstring processing. Full list of extensions:

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

[rossbar]

The .. warnings:: directive is a sphinx built-in, and should "just work" wherever it's placed. I've opened a dummy PR for illustration purposes where I've added .. warning:: 's at various places (diff). AFICT they render as expected.

I'm not sure what's happening in your case - my guesses would be a) a subtle indentation bug (though I don't see any problems) followed by b) different behavior in the napoleon parser, whether intentional or not.