ENH: accept autoclass member options #205

mattip · 2019-04-07T23:20:20Z

numpydoc adds member docstrings in calls to get_doc_object, in both mangle_docstringsand mangle_signature. A feature of numpydoc is that class methods get their own html page and the method in the class table is a link to that page. However, the automatic toctree generation in an autoclass directive does not add the directory generated to the toctree, resulting in references to unknown document warnings for each method. The only solution I could find was to use :exclude-members: on the autoclass directive. I modified the logic inside get_doc_object to handle both the :members: and :exclude-members: options. Tests added. Here is an example

Some text

.. autoclass:: str
    :exclude_members:

Changing case
-------------------
.. autosummary::
    :toctree: generated/
    str.upper
    str.lower

Checking content
-----------------------

.. autosummary:
    :toctree: generated/
    str.isalnum
    str.isalpha
    str.isdecimal
    str.isdigit

and so on

mattip · 2019-04-07T23:21:05Z

numpydoc/numpydoc.py

@@ -132,6 +132,7 @@ def mangle_docstrings(app, what, name, obj, options, lines):
           app.config.numpydoc_show_inherited_class_members,
           'class_members_toctree': app.config.numpydoc_class_members_toctree}

+    cfg.update(options)


Pass the options on to get_doc_object

To build the SciPy doc I had to change this to cfg.update(options or {}) because in scipyoptdoc we pass None for options

mattip · 2019-04-07T23:22:08Z

numpydoc/numpydoc.py

@@ -177,7 +178,7 @@ def mangle_signature(app, what, name, obj, options, sig, retann):

    if not hasattr(obj, '__doc__'):
        return
-    doc = get_doc_object(obj)
+    doc = get_doc_object(obj, config={'show_class_members': False})


We just want the signature at this point, not all the side effects of parsing all the members of obj

mattip · 2019-04-07T23:24:06Z

numpydoc/tests/test_docscrape.py

@@ -796,11 +796,13 @@ class BadSection(object):
        pass

    with warnings.catch_warnings(record=True) as w:
+        warnings.filterwarnings('always', '', UserWarning)


I am not sure how test runs avoid the need for a filter to work properly, this is more in line with what numpy does.

mattip · 2019-04-08T05:51:03Z

xref #184. Not strictly connected since this issue is connected with autoclass not autosummary, but the mangle_signature change may be relevant.

larsoner

I can confirm that this does not break my MNE build, which uses methods documented on the same page as the class itself, or my SciPy build (with the suggested change), which uses methods documented on their own page.

I started looking into whether or not this PR would allow me to (re-)simplify the SciPy doc build by undoing some changes I made to SciPy recently to work around the sorts of things mentioned in this PR (I think?). But if I remove the templates I had added for attribute.rst and method.rst in order to place :orphan: tags at the top, and revert the tweaks I had made to class.rst, I see the old warnings back again:

/home/larsoner/python/scipy/scipy/stats/_distn_infrastructure.py:docstring of scipy.stats.rv_histogram.var:1: WARNING: duplicate object description of scipy.stats.rv_histogram.var, other instance in /home/larsoner/python/scipy/doc/source/generated/scipy.stats.rv_histogram.rst, use :noindex: for one of them
...
/home/larsoner/python/scipy/doc/source/generated/scipy.LowLevelCallable.function.rst: WARNING: document isn't included in any toctree

Would you expect that these tweaks should no longer be necessary after this PR?

larsoner · 2019-04-08T14:03:22Z

numpydoc/numpydoc.py

@@ -132,6 +132,7 @@ def mangle_docstrings(app, what, name, obj, options, lines):
           app.config.numpydoc_show_inherited_class_members,
           'class_members_toctree': app.config.numpydoc_class_members_toctree}

+    cfg.update(options)


To build the SciPy doc I had to change this to cfg.update(options or {}) because in scipyoptdoc we pass None for options

mattip · 2019-04-08T14:37:08Z

Would you expect that these tweaks should no longer be necessary after this PR?

I think the "document isn't included in any toctree" is from numpydoc creating a document for methods but not linking it into a toctree, so the :orphan: is exactly the needed fix for this problem.

I don't see why /python/scipy/scipy/stats/_distn_infrastructure.py is linking in the docstring for rv_histogram.var.

larsoner · 2019-04-08T15:19:44Z

I don't see why /python/scipy/scipy/stats/_distn_infrastructure.py is linking in the docstring for rv_histogram.var.

This is just a peculiarity of how SciPy generates docs, as stats distribution docs are generated on the fly. When I revert my class.rst workarounds, the problem is also in more standard places like:

/home/larsoner/python/scipy/scipy/__init__.py:docstring of scipy.LowLevelCallable.count:1: WARNING: duplicate object description of scipy.LowLevelCallable.count, other instance in /home/larsoner/python/scipy/doc/source/generated/scipy.LowLevelCallable.rst, use :noindex: for one of them

But this existed before this PR, too, so I'm not too surprised it continues here.

In any case, it looks like this does not simplify anything at the SciPy end. But it looks like it isn't really designed to, doesn't break the builds, and otherwise looks reasonable to me.

numpydoc/tests/test_numpydoc.py

jnothman · 2019-04-09T10:56:54Z

numpydoc/numpydoc.py

@@ -132,6 +132,7 @@ def mangle_docstrings(app, what, name, obj, options, lines):
           app.config.numpydoc_show_inherited_class_members,
           'class_members_toctree': app.config.numpydoc_class_members_toctree}

+    cfg.update(options or {})


Is this an appropriate thing to do with already-supported options?

At some point numpydoc should re-use the sphinx versions of the options (numpdoc_show_inherited_class_members => 'inherited_members') rather than duplicate them, so I would claim it is not only appropriate but desired

jnothman · 2019-04-09T10:57:21Z

numpydoc/tests/test_numpydoc.py

+    assert 'upper' not in [x.strip() for x in lines]
+
+    lines = s.split('\n')
+    doc = mangle_docstrings(MockApp(), 'class', 'str', str, {'exclude-members': 'upper'}, lines)


is upper here intentionally a string and not a list of strings? are both meant to be supported?

Not really, but it works because 'upper' in 'upper' => True. fixing

Co-Authored-By: mattip <matti.picus@gmail.com>

larsoner · 2019-04-09T12:55:43Z

Thanks @mattip !

mattip · 2019-04-09T13:45:06Z

Thanks for the quick review

mattip commented Apr 7, 2019

View reviewed changes

ENH: accept autoclass member options

60b3fa3

mattip mentioned this pull request Apr 8, 2019

ENH: randomgen numpy/numpy#13163

Merged

larsoner reviewed Apr 8, 2019

View reviewed changes

BUG: options can be None (from review)

a9da83d

jnothman reviewed Apr 9, 2019

View reviewed changes

MAINT: fixes from review

75a6ded

Co-Authored-By: mattip <matti.picus@gmail.com>

jnothman approved these changes Apr 9, 2019

View reviewed changes

larsoner merged commit a482f66 into numpy:master Apr 9, 2019

larsoner added type: Bug fix type: Enhancement and removed type: Bug fix labels Apr 11, 2019

larsoner mentioned this pull request Apr 11, 2019

Please release a new version #203

Closed

bstaletic mentioned this pull request Jul 25, 2019

Update jedi to v0.14.1 ycm-core/ycmd#1293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: accept autoclass member options #205

ENH: accept autoclass member options #205

mattip commented Apr 7, 2019

mattip Apr 7, 2019

larsoner Apr 8, 2019

mattip Apr 8, 2019

mattip Apr 7, 2019

mattip Apr 7, 2019

mattip commented Apr 8, 2019

larsoner left a comment

larsoner Apr 8, 2019

mattip commented Apr 8, 2019

larsoner commented Apr 8, 2019

jnothman Apr 9, 2019

mattip Apr 9, 2019

jnothman Apr 9, 2019

mattip Apr 9, 2019

larsoner commented Apr 9, 2019

mattip commented Apr 9, 2019

ENH: accept autoclass member options #205

ENH: accept autoclass member options #205

Conversation

mattip commented Apr 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattip commented Apr 8, 2019

larsoner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattip commented Apr 8, 2019

larsoner commented Apr 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

larsoner commented Apr 9, 2019

mattip commented Apr 9, 2019