Skip to content

ENH: add GL11 validation for bullet list formatting in docstrings#693

Open
Sidra-009 wants to merge 11 commits into
numpy:mainfrom
Sidra-009:gl11-bullet-list-check
Open

ENH: add GL11 validation for bullet list formatting in docstrings#693
Sidra-009 wants to merge 11 commits into
numpy:mainfrom
Sidra-009:gl11-bullet-list-check

Conversation

@Sidra-009

Copy link
Copy Markdown

this PR introduces GL11 validation to detect missing blank lines before bullet lists in docstrings

this prevents a known Sphinx Napoleon rendering issue where bullet lists are incorrectly merged into paragraphs when not separated by a blank line after a colon.

the check triggers when:

  • A line ends with ":"
  • The next line starts with a bullet (-, * or +)
  • No blank line exists between them

this improves docstring rendering consistency and documentation quality

@larsoner larsoner left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update a test that would fail on main but pass on this PR?

I think it would show that this is not working yet, as I tried it on a repo and got:

____________________________________________________________________________ test_docstring_parameters _____________________________________________________________________________
mne/tests/test_docstring_parameters.py:190: in test_docstring_parameters
    incorrect += check_parameters_match(method, cls=cls, where=name)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
mne/tests/test_docstring_parameters.py:131: in check_parameters_match
    for err in validate(name)["errors"]
               ^^^^^^^^^^^^^^
../numpydoc/numpydoc/validate.py:755: in validate
    errs.append(error("GL11"))
                ^^^^^^^^^^^^^
../numpydoc/numpydoc/validate.py:219: in error
    return code, ERROR_MSGS[code].format(**kwargs)
                 ^^^^^^^^^^^^^^^^
E   KeyError: 'GL11'

@Sidra-009

Sidra-009 commented Jun 4, 2026

Copy link
Copy Markdown
Author

Hi @larsoner,

I have addressed the requested changes:

  • i added the missing GL11 entry to ERROR_MSGS in validate.py to resolve the KeyError
  • i updated the validation logic for GL11 to correctly handle and skip empty lines before bullet lists in docstrings

could you please take another look at the PR? Thank you

@larsoner

larsoner commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

I tried it and seem to get a lot of false alarms (339 errors when I expected at most ~20). For example

larsoner:~/python/mne-python$ pytest mne/tests/test_docstring_parameters.py 
=============================================================================== test session starts ================================================================================
platform linux -- Python 3.14.4, pytest-9.0.3, pluggy-1.6.0
PySide6 6.11.1 -- Qt runtime 6.11.1 -- Qt compiled 6.11.1
MNE 1.13.0.dev54+g692eccee2 -- /home/larsoner/python/mne-python/mne
rootdir: /home/larsoner/python/mne-python
configfile: pyproject.toml
plugins: qt-4.5.0, typeguard-4.5.2, flaky-3.8.1, anyio-4.13.0, cov-7.1.0, timeout-2.4.0
collected 4 items                                                                                                                                                                  

mne/tests/test_docstring_parameters.py F...                                                                                                                                  [100%]



===================================================================================== FAILURES =====================================================================================
____________________________________________________________________________ test_docstring_parameters _____________________________________________________________________________
mne/tests/test_docstring_parameters.py:208: in test_docstring_parameters
    raise AssertionError(
E   AssertionError: 339 errors found:
E   mne : mne._fiff.meas_info.Info.anonymize : GL11 : Missing blank line before bullet list after a colon in docstring.
E   mne : mne._fiff.meas_info.Info.plot_sensors : GL11 : Missing blank line before bullet list after a colon in docstring.
...

Complains about Info.plot_sensors:

https://github.com/mne-tools/mne-python/blob/bc9419a4fca08ed943173fc08df59d1559e901e3/mne/_fiff/meas_info.py#L697-L765

Whose __doc__ is:

docstring
>>> print(mne.Info.plot_sensors.__doc__)
Plot sensor positions.

Parameters
----------
kind : str
    Whether to plot the sensors as 3d, topomap or as an interactive
    sensor selection dialog. Available options 'topomap', '3d',
    'select'. If 'select', a set of channels can be selected
    interactively by using lasso selector or clicking while holding
    control key. The selected channels are returned along with the
    figure instance. Defaults to 'topomap'.
ch_type : None | str
    The channel type to plot. Available options ``'mag'``, ``'grad'``,
    ``'eeg'``, ``'seeg'``, ``'dbs'``, ``'ecog'``, ``'all'``. If ``'all'``, all
    the available mag, grad, eeg, seeg, dbs, and ecog channels are plotted. If
    None (default), then channels are chosen in the order given above.
title : str | None
    Title for the figure. If None (default), equals to ``'Sensor
    positions (%s)' % ch_type``.
show_names : bool | array of str
    Whether to display all channel names. If an array, only the channel
    names in the array are shown. Defaults to False.
ch_groups : 'position' | array of shape (n_ch_groups, n_picks) | None
    Channel groups for coloring the sensors. If None (default), default
    coloring scheme is used. If 'position', the sensors are divided
    into 8 regions. See ``order`` kwarg of :func:`mne.viz.plot_raw`. If
    array, the channels are divided by picks given in the array.

    .. versionadded:: 0.13.0
to_sphere : bool
    Whether to project the 3d locations to a sphere. When False, the
    sensor array appears similar as to looking downwards straight above
    the subject's head. Has no effect when kind='3d'. Defaults to True.

    .. versionadded:: 0.14.0
axes : instance of Axes | instance of Axes3D | None
    Axes to draw the sensors to. If ``kind='3d'``, axes must be an
    instance of Axes3D. If None (default), a new axes will be created.

    .. versionadded:: 0.13.0
block : bool
    Whether to halt program execution until the figure is closed.
    Defaults to False.

    .. versionadded:: 0.13.0
show : bool
    Show figure if True. Defaults to True.
sphere : float | array-like of float | instance of ConductorModel | str | list of str | None
    The sphere parameters to use for the head outline.
    Can be array-like of shape (4,) to give the X/Y/Z origin and radius in meters, or a
    single float to give just the radius (origin assumed 0, 0, 0).
    Can also be an instance of a spherical :class:`~mne.bem.ConductorModel` to use the
    origin and radius from that object.
    Can also be a ``str``, in which case:

    - ``'auto'``: the sphere is fit to external digitization points first, and to
      external + EEG digitization points if the former fails.

    - ``'eeglab'``: the head circle is defined by EEG electrodes ``'Fpz'``, ``'Oz'``,
      ``'T7'``, and ``'T8'`` (if ``'Fpz'`` is not present, it will be approximated from
      the coordinates of ``'Oz'``).

      - ``'extra'``: the sphere is fit to external digitization points.

      - ``'eeg'``: the sphere is fit to EEG digitization points.

      - ``'cardinal'``: the sphere is fit to cardinal digitization points.

      - ``'hpi'``: the sphere is fit to HPI coil digitization points.

    Can also be a list of ``str``, in which case the sphere is fit to the specified
    digitization points, which can be any combination of ``'extra'``, ``'eeg'``,
    ``'cardinal'``, and ``'hpi'``, as specified above.
    ``None`` (the default) is equivalent to ``'auto'`` when enough extra digitization
    points are available, and (0, 0, 0, 0.095) otherwise.

    .. versionadded:: 0.20
    .. versionchanged:: 1.1 Added ``'eeglab'`` option.
    .. versionchanged:: 1.11 Added ``'extra'``, ``'eeg'``, ``'cardinal'``, ``'hpi'`` and
       list of ``str`` options.

verbose : bool | str | int | None
    Control verbosity of the logging output. If ``None``, use the default
    verbosity level. See the :ref:`logging documentation <tut-logging>` and
    :func:`mne.verbose` for details. Should only be passed as a keyword
    argument.

Returns
-------
fig : instance of Figure
    Figure containing the sensor topography.
selection : list
    A list of selected channels. Only returned if ``kind=='select'``.

See Also
--------
mne.viz.plot_layout

Notes
-----
This function plots the sensor locations from the info structure using
matplotlib. For drawing the sensors using PyVista see
:func:`mne.viz.plot_alignment`.

.. versionadded:: 0.12.0

Rendering also appears okay.

So I suspect the logic is wrong, which in turns suggests the set of test cases used is not big enough to differentiate correct from incorrect uses.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants