sphinx-doc / sphinx

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

regression in 3.1.0: docstring of ...:1:duplicate object description of ..., other instance in ..., use :noindex: for one of them #7817

Open StrikerRUS opened 4 years ago

StrikerRUS commented 4 years ago

Describe the bug Incompatibility with numpy docstring convention.

Attributes that are properties and have their own docstrings can be simply listed by name: https://numpydoc.readthedocs.io/en/latest/format.html#class-docstring

To Reproduce Toy example:

conf.py:

import os
import sys
import sphinx

CURR_PATH = os.path.abspath(os.path.dirname(__file__))
LIB_PATH = os.path.join(CURR_PATH, os.path.pardir, 'python-package')
sys.path.insert(0, LIB_PATH)

# -- General configuration ------------------------------------------------

# The master toctree document.
master_doc = 'index'

# List of patterns, relative to source directory, that match files and
# directories to ignore when looking for source files.
# This patterns also effect to html_static_path and html_extra_path
exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']

# Add any Sphinx extension module names here, as strings. They can be
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
# ones.
extensions = [
    'sphinx.ext.autodoc',
    'sphinx.ext.autosummary',
    'sphinx.ext.todo',
    'sphinx.ext.viewcode',
    'sphinx.ext.napoleon',
]

autodoc_default_flags = ['members', 'inherited-members', 'show-inheritance']
autodoc_default_options = {
    "members": True,
    "inherited-members": True,
    "show-inheritance": True,
}

# Generate autosummary pages. Output should be set with: `:toctree: pythonapi/`
autosummary_generate = ['Python-API.rst']

# Only the class' docstring is inserted.
autoclass_content = 'class'

# If true, `todo` and `todoList` produce output, else they produce nothing.
todo_include_todos = False

index.rst:

.. toctree::
   :caption: Contents:

   Python API <Python-API>

Python-API.rst:

Python API
==========

.. currentmodule:: lightgbm

Scikit-learn API
----------------

.. autosummary::
    :toctree: pythonapi/

    A

..\python-package\lightgbm\sklearn.py:

class A(object):
    """Class A."""

    def __init__(self, x=42):
        """Init description.

        Parameters
        ----------
        x : int, optional (default=42)
            X description.

        Attributes
        ----------
        n_features_
        classes_ : array of shape = [n_classes]
            The class label array (only for classification problem).
        """
        self.x = x
        self._n_features = None
        self._n_classes = None

    @property
    def n_features_(self):
        """N features description."""
        return self._n_features

    @property
    def classes_(self):
        return 42

Expected behavior No warnings.

Your project https://github.com/microsoft/LightGBM/blob/master/docs/conf.py

Environment info

tk0miya commented 4 years ago

It seems A. n_features_ is documented twice. The first time is in the docstring of A.__init__(). And the second one is the docstring of A.n_features_ property itself. I don't know why past versions do not warn this case. But it seems duplicated. So the warning is correct to me. How about removing either one?

StrikerRUS commented 4 years ago

Hi @tk0miya !

Thanks a lot for your reply!

It seems A. n_features_ is documented twice.

Yes, indeed. But it is the correct way to document properties in numpy convention. Please refer to

Attributes that are properties and have their own docstrings can be simply listed by name:

Attributes
----------
real
imag
x : float
The X coordinate
y : float
The Y coordinate

https://numpydoc.readthedocs.io/en/latest/format.html#class-docstring

tk0miya commented 4 years ago

You should not add a docstring to A.n_features_ property if you'd like to document n_features_ in the docstring of A.__init__(). They're conflicted. Is there some reason to keep it as is? If you'd not like to change it, please append :meta private: to the docstring of A.n_features_. Then the property will be suppressed on output.

StrikerRUS commented 4 years ago

@tk0miya Many thanks for the proposed workarounds! Can you please clarify what's the problem to follow numpy guide and why previous Sphinx versions had no problems with it?

tk0miya commented 4 years ago

There are no problem to follow the numpy guide. That is a misunderstanding. Only what I said is you have written a document for A.n_features_ via two ways; the docstring of A.__init__() and the docstring of A.n_features_ itself. I still don't know why old Sphinx does not raise a warning for this duplication. But I believe current behavior is correct. In other word, old sphinx has a bug about not warning for such duplication.

StrikerRUS commented 4 years ago

But I believe current behavior is correct.

I don't think so, because numpy guide allows a such type of duplication. However, I'm OK to remove documentation from __init__. So feel free to close the issue if you think that Sphinx shouldn't respect that paragraph from numpy docs I quoted early, because for me personally this won't be an issue anymore.

tk0miya commented 4 years ago

The numpy guide only says the docstring of __init__() method can describe attributes (including properties) of the object. It does not mention about conflicts with the docstring of property itself. So it is better to remove the docstring of the A.n_features_ if you'd like to follow the numpy guide. Of course, it's okay to remove it from __init__().

StrikerRUS commented 4 years ago

So it is better to remove the docstring of the A.n_features_

Unfortunately, it is not an option due to D102 | Missing docstring in public method error of pydocstyle analysis tool for PEP 257.

It does not mention about conflicts with the docstring of property itself.

It should be no conflicts in case __init__ has only a name of a property and all other information is handled in property's docstring.

tk0miya commented 4 years ago

Hmm... Okay, I'll reconsider this again (for 3.2). So I keep this open for a while.

tk0miya commented 4 years ago

I reconsider this. If you'd like to write attributes to the docstring of class (or __init__() method) following spec of numpydoc, you should not create a document for attributes because of duplication. At first, I just thought not to write docstrings to each attributes (and properties) as commented above. But, I found another approach. It is updating the template of autosummary. The default class template of autosummary shows all of attributes of the class like this:

{{ fullname | escape | underline}}

.. currentmodule:: {{ module }}

.. autoclass:: {{ objname }}

   {% block methods %}
   .. automethod:: __init__

   {% if methods %}
   .. rubric:: {{ _('Methods') }}

   .. autosummary::
   {% for item in methods %}
      ~{{ name }}.{{ item }}
   {%- endfor %}
   {% endif %}
   {% endblock %}

Then the attributes are only described in the docstring of the class (or __init__() method).

Note: This approach is better for this case. But it does not work properly at this moment. Because autodoc considers properties are kind of methods now. We have a plan to change the behavior in (nearly) future. After that, it will be resolved via customized autosummary template, I believe.

I'll check my idea will really work well after the update. This is a rough implementation of my idea:

FROM python:3.8-slim

RUN apt update; apt install -y build-essential curl git make unzip vim
RUN git clone https://github.com/microsoft/LightGBM
WORKDIR /LightGBM/docs
RUN git checkout fa2de89bb6a8d9baae164e50a25ad82ff2d340a4~1
RUN pip install -r requirements_base.txt
RUN pip install Sphinx==3.1.2
ENV C_API=NO
RUN mkdir -p _templates/autosummary
RUN { \
        echo "{{ fullname | escape | underline}}"; \
        echo ""; \
        echo ".. currentmodule:: {{ module }}"; \
        echo ""; \
        echo ".. autoclass:: {{ objname }}"; \
        echo ""; \
        echo "   {% block methods %}"; \
        echo "   .. automethod:: __init__"; \
        echo ""; \
        echo "   {% if methods %}"; \
        echo "   .. rubric:: {{ _('Methods') }}"; \
        echo ""; \
        echo "   .. autosummary::"; \
        echo "   {% for item in methods %}"; \
        echo "      ~{{ name }}.{{ item }}"; \
        echo "   {%- endfor %}"; \
        echo "   {% endif %}"; \
        echo "   {% endblock %}"; \
    } > _templates/autosummary/class.rst
RUN echo "templates_path = ['_templates']" >> conf.py
RUN echo "exclude_patterns.append('_templates/**/*.rst')" >> conf.py
RUN make html
kcharlie2 commented 3 years ago

I have experienced this bug as well, but would like some clarification. Is this for attributes documented in __init__ only, or also the class definition?

In the numpy example https://numpydoc.readthedocs.io/en/latest/format.html#class-docstring, they show documenting attributes in the class definition, not __init__.

I am getting the same duplicate object description warning listing attribute names in the class's Attributes section, and documenting them in the @property, which is exactly what the example shows.

Fl4m3Ph03n1x commented 2 years ago

I am currently facing the same issue:

@dataclass(frozen=True)
class Feature:
    """
    Structured payload of a Feature...

    Attributes
    ---------
    name : str
        The name of this ``Feature``.
    enabled : bool
        Whether or not this ``Feature`` is enabled....
    """
    name: str
    enabled: bool

Generates the same duplication warning. I am also trying to follow the same numpydoc convention as @kcharlie2 .

If I change to Attributes to Parameters, the warning goes away, but I still get annoying duplication in my generated HTML for no good reason:

doc

Is there now a way to fix this and still follow the numpydoc convention?

Conchylicultor commented 1 year ago

+1, @dataclass documentation raise: WARNING: duplicate object description of MyDataclass.my_field, ..., use :noindex: for one of them, as pointed out by @Fl4m3Ph03n1x.

Is there a workaround ?