[Doc]: drawedges attribute described twice in matplotlib.colorbar documentation

[MoIMFD]

Documentation Link

https://matplotlib.org/stable/api/colorbar_api.html#matplotlib.colorbar.Colorbar

Problem

drawedges attributed:

drawedges : bool
    Whether to draw lines at color boundaries.

is specified twice.

Suggested improvement

Remove one definition from function docstring.

[github-actions]

Good first issue - notes for new contributors

This issue is suited to new contributors because it does not require understanding of the
Matplotlib internals. To get started, please see our contributing
guide.

We do not assign issues. Check the Development section in the sidebar for linked pull
requests (PRs). If there are none, feel free to start working on it. If there is an open PR, please
collaborate on the work by reviewing it rather than duplicating it in a competing PR.

If something is unclear, please reach out on any of our communication
channels.

[tacaswell]

matplotlib/lib/matplotlib/colorbar.py

Lines 195 to 276 in 955432c

	@_docstring.interpd
	class Colorbar:
	r"""
	Draw a colorbar in an existing Axes.
	Typically, colorbars are created using `.Figure.colorbar` or
	`.pyplot.colorbar` and associated with `.ScalarMappable`\s (such as an
	`.AxesImage` generated via `~.axes.Axes.imshow`).
	In order to draw a colorbar not associated with other elements in the
	figure, e.g. when showing a colormap by itself, one can create an empty
	`.ScalarMappable`, or directly pass *cmap* and *norm* instead of *mappable*
	to `Colorbar`.
	Useful public methods are :meth:`set_label` and :meth:`add_lines`.
	Attributes
	----------
	ax : `~matplotlib.axes.Axes`
	The `~.axes.Axes` instance in which the colorbar is drawn.
	lines : list
	A list of `.LineCollection` (empty if no lines were drawn).
	dividers : `.LineCollection`
	A LineCollection (empty if *drawedges* is ``False``).
	Parameters
	----------
	ax : `~matplotlib.axes.Axes`
	The `~.axes.Axes` instance in which the colorbar is drawn.
	mappable : `.ScalarMappable`
	The mappable whose colormap and norm will be used.
	To show the under- and over- value colors, the mappable's norm should
	be specified as ::
	norm = colors.Normalize(clip=False)
	To show the colors versus index instead of on a 0-1 scale, use::
	norm=colors.NoNorm()
	cmap : `~matplotlib.colors.Colormap`, default: :rc:`image.cmap`
	The colormap to use. This parameter is ignored, unless *mappable* is
	None.
	norm : `~matplotlib.colors.Normalize`
	The normalization to use. This parameter is ignored, unless *mappable*
	is None.
	alpha : float
	The colorbar transparency between 0 (transparent) and 1 (opaque).
	orientation : None or {'vertical', 'horizontal'}
	If None, use the value determined by *location*. If both
	*orientation* and *location* are None then defaults to 'vertical'.
	ticklocation : {'auto', 'left', 'right', 'top', 'bottom'}
	The location of the colorbar ticks. The *ticklocation* must match
	*orientation*. For example, a horizontal colorbar can only have ticks
	at the top or the bottom. If 'auto', the ticks will be the same as
	*location*, so a colorbar to the left will have ticks to the left. If
	*location* is None, the ticks will be at the bottom for a horizontal
	colorbar and at the right for a vertical.
	drawedges : bool
	Whether to draw lines at color boundaries.
	%(_colormap_kw_doc)s
	location : None or {'left', 'right', 'top', 'bottom'}
	Set the *orientation* and *ticklocation* of the colorbar using a
	single argument. Colorbars on the left and right are vertical,
	colorbars at the top and bottom are horizontal. The *ticklocation* is
	the same as *location*, so if *location* is 'top', the ticks are on
	the top. *orientation* and/or *ticklocation* can be provided as well
	and overrides the value set by *location*, but there will be an error
	for incompatible combinations.
	.. versionadded:: 3.7
	"""

It looks like this is a combination of explicitly written out docstrings and @_docstring.interpd interacting badly.

The work here is:

1. verify that is actually the case
2. (minimal) remove the explicit entry and rely on the interpolation to provide it
3. (medium) add a (parameterized) test that uses numpydoc to parse the docstring and identify duplicate entries
4. (maximal) investigate if we can automatically check in @_docstring.interpd
5. (most maximal) push the check for duplicate entiries into numpydoc so they warn and then make sure our docs fail on the warning

[MoIMFD]

Dear tacaswell,

I am not familar with the matplotlib documentation and haven't contributed yet. I think the check wether a parameter is present or not is not dificult. Since the substituions are in a dictionary they can be checked for duplicates. I include a simple example below using re to extract the paramter from the docstring.

Is the approach correct or have I overlooked something?

class Colorbar:
    r"""
    Draw a colorbar in an existing Axes.

    Typically, colorbars are created using `.Figure.colorbar` or
    `.pyplot.colorbar` and associated with `.ScalarMappable`\s (such as an
    `.AxesImage` generated via `~.axes.Axes.imshow`).

    In order to draw a colorbar not associated with other elements in the
    figure, e.g. when showing a colormap by itself, one can create an empty
    `.ScalarMappable`, or directly pass *cmap* and *norm* instead of *mappable*
    to `Colorbar`.

    Useful public methods are :meth:`set_label` and :meth:`add_lines`.

    Attributes
    ----------
    ax : `~matplotlib.axes.Axes`
        The `~.axes.Axes` instance in which the colorbar is drawn.
    lines : list
        A list of `.LineCollection` (empty if no lines were drawn).
    dividers : `.LineCollection`
        A LineCollection (empty if *drawedges* is ``False``).

    Parameters
    ----------
    ax : `~matplotlib.axes.Axes`
        The `~.axes.Axes` instance in which the colorbar is drawn.

    mappable : `.ScalarMappable`
        The mappable whose colormap and norm will be used.

        To show the under- and over- value colors, the mappable's norm should
        be specified as ::

            norm = colors.Normalize(clip=False)

        To show the colors versus index instead of on a 0-1 scale, use::

            norm=colors.NoNorm()

    cmap : `~matplotlib.colors.Colormap`, default: :rc:`image.cmap`
        The colormap to use.  This parameter is ignored, unless *mappable* is
        None.

    norm : `~matplotlib.colors.Normalize`
        The normalization to use.  This parameter is ignored, unless *mappable*
        is None.

    alpha : float
        The colorbar transparency between 0 (transparent) and 1 (opaque).

    orientation : None or {'vertical', 'horizontal'}
        If None, use the value determined by *location*. If both
        *orientation* and *location* are None then defaults to 'vertical'.

    ticklocation : {'auto', 'left', 'right', 'top', 'bottom'}
        The location of the colorbar ticks. The *ticklocation* must match
        *orientation*. For example, a horizontal colorbar can only have ticks
        at the top or the bottom. If 'auto', the ticks will be the same as
        *location*, so a colorbar to the left will have ticks to the left. If
        *location* is None, the ticks will be at the bottom for a horizontal
        colorbar and at the right for a vertical.

    drawedges : bool
        Whether to draw lines at color boundaries.


    %(_colormap_kw_doc)s

    location : None or {'left', 'right', 'top', 'bottom'}
        Set the *orientation* and *ticklocation* of the colorbar using a
        single argument. Colorbars on the left and right are vertical,
        colorbars at the top and bottom are horizontal. The *ticklocation* is
        the same as *location*, so if *location* is 'top', the ticks are on
        the top. *orientation* and/or *ticklocation* can be provided as well
        and overrides the value set by *location*, but there will be an error
        for incompatible combinations.

        .. versionadded:: 3.7
    """

    def __init__(self):
        pass

### Demo Code ###

import re

substitutions = {
    "ax": "Some documentation for axis",   # <-- duplicated parameter entry
    "some_new_argument": "A little more documentation",  # <- new parameter entry
}

docstring_parameter = re.findall(
    pattern=r"    (\w+) : ",  # regex to match parameter according to docstring style
    string=Colorbar.__doc__.split("Parameter")[
        1
    ],  # only consider parameter section of docstring
)

assert all(
    [substitution not in docstring_parameter for substitution in substitutions]
), "Something is off"  # check if there are duplicats between docstring parameter and substitutions

[tacaswell]

It seems better to me to use numpydoc rather than trying to do it with regex + splits. For example if in the earlier section the sentence "Parameters are set..." appeared, the split would catch it.

On a bit more consideration I think (4) in my last post is a bad idea...we do not want to pay for this checking at every import.

[MoIMFD]

I had a look at numpydoc and thought about adding a check after the parsing process in NumpyDocString __init__ method.

https://github.com/numpy/numpydoc/blob/acad4ece8805328d8cf34279d2fe923205a1f79c/numpydoc/docscrape.py#L111-L569

A possible check is shown here. Unfortunately I haven't figured out yet how to access the function/class name and location from within NumpyDocString to indicate where the duplicate is found.

from collections import Counter
import warnings

class NumpyDocString(Mapping):
    """Parses a numpydoc string to an abstract representation

    Instances define a mapping from section title to structured data.

    """

    sections = {
        "Signature": "",
        "Summary": [""],
        "Extended Summary": [],
        "Parameters": [],
        "Returns": [],
        "Yields": [],
        "Receives": [],
        "Raises": [],
        "Warns": [],
        "Other Parameters": [],
        "Attributes": [],
        "Methods": [],
        "See Also": [],
        "Notes": [],
        "Warnings": [],
        "References": "",
        "Examples": "",
        "index": {},
    }

    def __init__(self, docstring, config=None):
        orig_docstring = docstring
        docstring = textwrap.dedent(docstring).split("\n")

        self._doc = Reader(docstring)
        self._parsed_data = copy.deepcopy(self.sections)

        try:
            self._parse()
        except ParseError as e:
            e.docstring = orig_docstring
            raise
        
        #### Added Code ####
        for section, content in self._parsed_data.items():
            parameters = [item.name for item in content if isinstance(item, Parameter)]
            param_counts = Counter(parameters)
            for parameter, count in param_counts.items():
                if count > 1:
                    warnings.warn(f"Parameter '{parameter}' specified multiple times in section '{section}'")

Sample Output:

numpydoc/numpydoc/docscrape.py:162: UserWarning: Parameter 'orientation' specified multiple times in section 'Parameters'
numpydoc/numpydoc/docscrape.py:162: UserWarning: Parameter 'drawedges' specified multiple times in section 'Parameters'
numpydoc/numpydoc/docscrape.py:162: UserWarning: Parameter 'location' specified multiple times in section 'Parameters'

What is your opinion on this approach?

[malhar2460]

hi @tacswell ,In my pull request i failed some tests again, can you help me with what went wrong, last time it failed because i had an empty folder named matplotlib and it considered it as sub module or something