Invalid class description of set/frozenset in docs

[skirpichev]

The class sphinx directive is used (see it's docs), but wrongly:

cpython/Doc/library/stdtypes.rst

Lines 4251 to 4258 in 6313cdd

	.. class:: set([iterable])
	frozenset([iterable])
	Return a new set or frozenset object whose elements are taken from
	*iterable*. The elements of a set must be :term:`hashable`. To
	represent sets of sets, the inner sets must be :class:`frozenset`
	objects. If *iterable* is not specified, a new empty set is
	returned.

It looks (despite this seems to be not documented in sphinx-doc), that the second signature takes precedence and methods below could be referenced (either internally in CPython or with the intersphinx for external projects) only like frozenset.foo, which is especially silly in cases like frozenset.add.

We could mitigate this by reordering set and frozenset in the class directive (references to set's methods are more common), but this is not a real solution.

One option to solve the problem is using two class directives (like for list/tuple). BTW, while technically the set is not a subtype of the frozenset, in fact it just has some overloaded ops and a bunch of new methods...

Another option is a new sphinx directive like currentmodule: "Directive currentclass (similar to currentmodule) would help us." Originally posted by @serhiy-storchaka in #114280 (comment)

Finally, it could be viewed as a sphinx-only issue: the class directive must support references for every declared signature (i.e. both set.add and frozenset.add).

[serhiy-storchaka]

It is not only references. It adds index entry for frozenset.add.

I think that we need to write our own Sphinx extensions. currentclass would help also in other cases, where we have the class constructor description far from methods descriptions (or do not use corresponding class directive at all). It will help also in the Tkinter docs, where I want to separate a large list of methods on several groups.

[ferdnyc]

The issue, I think, is a combination of how Sphinx interprets hierarchy in the .rst markup, and how it converts directives into document hierarchy.

.. class:: set([iterable])
           frozenset([iterable])

ends up as an HTML description list with two terms, the first of which has no description content. Editing out the purely decorative markup (SPANs and the like), it becomes:

<dl class="py class">
<dt id="set">
<span class="pre">class set([iterable])</span>
<a class="headerlink" href="https://docs.python.org/3/library/stdtypes.html#set">¶</a></dt>
<dt id="frozenset">
<span class="pre">class frozenset([iterable])</span>
<a class="headerlink" href="https://docs.python.org/3/library/stdtypes.html#frozenset">¶</a></dt>

...and then all of the content indented below the .. class:: directive is translated into the contents of the <dd>...</dd> element that follows.

Effectively, from a hierarchy standpoint, it treats the above identical to:

.. class:: set([iterable])
.. class:: frozenset([iterable])

That said, there are some limited workarounds within the existing directives.

One is to simply move most of the directive's contents out to the top level. We can reference the methods in prose with e.g. :meth:~set.add so they'll render the same, though unfortunately there doesn't seem to be an equivalent directive form. So they'd have to be visibly more fully qualified, e.g.:

Set Types --- :class:`set`, :class:`frozenset`
==============================================

.. class:: set([iterable])
           frozenset([iterable])

   Return a new set or frozenset object whose elements are taken from
   *iterable*.  The elements of a set must be :term:`hashable`.  To
   represent sets of sets, the inner sets must be :class:`frozenset`
   objects.  If *iterable* is not specified, a new empty set is
   returned.

[...]

The following table lists operations available for :class:`set` that do not
apply to immutable instances of :class:`frozenset`:

.. method:: set.add()

   Add element *elem* to the set.

Note, the non-operator versions of :meth:`~set.union`, :meth:`~set.intersection`,
[...] will accept any iterable as an argument.

which would index and cross-reference correctly, and may not be the worst thing in the world. It'd render as something like:

---

Set Types - set, frozenset

class set([iterable]) class frozenset([iterable])

Return a new set or frozenset object whose elements are taken from iterable. The elements of a set must be hashable. To represent sets of sets, the inner sets must be frozenset objects. If iterable is not specified, a new empty set is returned.

[...]

The following table lists operations available for set that do not apply to immutable instances of frozenset:

set.add(elem)

Add element elem to the set.

Note, the non-operator versions of union, intersection, [...] will accept any iterable as an argument.

---

The only real visible differences are a slight outdent for most of the content, and the addition of set. at the front of the add() method signature. But it would be indexed as set.add, be searchable as set.add. etc.

[ferdnyc]

So, guess what.

Turns out, .. currentmodule:: is somewhat misnamed, it should be called something more like .. currentcontext::, because it works just fine sorta works for classes as well.

AFAICT, in testing, these two chunks of RST code appear to have identical ALMOST identical effect, in terms of indexing/cross-referencing:

Version 1

.. class:: set

   Blah blah constructor.

   .. method:: add()

      Add element *elem* to the set.

   Blah blah :meth:`union`, :meth:`intersection` etc.

Version 2

.. class:: set

   Blah blah constructor.


.. class:: otherclass

   Other class's stuff.


.. currentmodule:: set

.. method:: add()

   Add element *elem* to the set.

Blah blah :meth:`union`, :meth:`intersection` etc.

Both versions will use #set.add as the link fragment for the add() method, the :meth:union xref will be equivalent to :meth:~set.union, etc. set does not get added to the modules index, and its entry in the overall index is still only set (built-in type).

(And unfortunately it doesn't work if .. currentmodule:: set and what follows are indented as children of the .. class:: otherclass directive — then, they become otherclass.set.foo xrefs.)

ARRRGH! And I just discovered that add() will get listed in the global index as add() (in module set), instead of add() (set method). That one thing! 😢

[skirpichev]

From the sources it seems the set type share a lot of methods with the frozenset:

cpython/Objects/setobject.c

Lines 2225 to 2269 in 7a7bce5

	PyTypeObject PyFrozenSet_Type = {
	PyVarObject_HEAD_INIT(&PyType_Type, 0)
	"frozenset", /* tp_name */
	sizeof(PySetObject), /* tp_basicsize */
	0, /* tp_itemsize */
	/* methods */
	(destructor)set_dealloc, /* tp_dealloc */
	0, /* tp_vectorcall_offset */
	0, /* tp_getattr */
	0, /* tp_setattr */
	0, /* tp_as_async */
	(reprfunc)set_repr, /* tp_repr */
	&frozenset_as_number, /* tp_as_number */
	&set_as_sequence, /* tp_as_sequence */
	0, /* tp_as_mapping */
	frozenset_hash, /* tp_hash */
	0, /* tp_call */
	0, /* tp_str */
	PyObject_GenericGetAttr, /* tp_getattro */
	0, /* tp_setattro */
	0, /* tp_as_buffer */
	Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC |
	Py_TPFLAGS_BASETYPE |
	_Py_TPFLAGS_MATCH_SELF, /* tp_flags */
	frozenset_doc, /* tp_doc */
	(traverseproc)set_traverse, /* tp_traverse */
	(inquiry)set_clear_internal, /* tp_clear */
	(richcmpfunc)set_richcompare, /* tp_richcompare */
	offsetof(PySetObject, weakreflist), /* tp_weaklistoffset */
	(getiterfunc)set_iter, /* tp_iter */
	0, /* tp_iternext */
	frozenset_methods, /* tp_methods */
	0, /* tp_members */
	0, /* tp_getset */
	0, /* tp_base */
	0, /* tp_dict */
	0, /* tp_descr_get */
	0, /* tp_descr_set */
	0, /* tp_dictoffset */
	0, /* tp_init */
	PyType_GenericAlloc, /* tp_alloc */
	frozenset_new, /* tp_new */
	PyObject_GC_Del, /* tp_free */
	.tp_vectorcall = frozenset_vectorcall,
	};

Can't we really use the frozenset as a base class for the set? In the later few methods will be overridden (tp_hash, etc) and new ops added, that mutate instances (like set.add).

If the sphinx would support class inheritance in the class directive (like the autoclass directive or the class directive of C++ domain), then the current issue will be solved. @AA-Turner does support for inheritance in the :py:class: directive make sense as a feature request for sphinx?

[ferdnyc]

Inheritance may be the right way to solve this, indeed.

However, after banging on it for the weekend, I have a working implementation of a py:currentclass directive, including unit tests. (But no documentation, yet.)

It's in my Sphinx fork, on the currentclass branch: https://github.com/ferdnyc/sphinx/tree/currentclass

In lieu of documentation, here's the docstring for the class:

class PyCurrentClass(SphinxDirective):
    """
    Like PyCurrentModule, this directive exists to adjust Sphinx's
    view of the context for the current documentation.

    Because classes are hierarchical, this directive will only affect
    the innermost local context; when exiting to an outer level of
    nesting, the previous (parent) context will be restored.

    This means that in the following example:

    .. code-block::

       .. py:class:: A

          .. py:class:: SubA

       .. py:class:: B

          .. py:class:: SubB

             .. py:currentclass:: A.SubA

             .. py:method:: m1

          .. py:method:: m2

    The documented methods will be ``A.SubA.m1()`` and ``B.m2()``
    """

[serhiy-storchaka]

Very interesting. Is it possible to implement it as an extension in Doc/tools/extensions?

How does the c:namespace directive work in a similar example with nested C structures? In any case it will likely be used mainly in a global scope.

[skirpichev]

CC @picnixz per #130822