Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Need Attributes displayable like param list #102

Closed
jnothman opened this issue Jul 19, 2017 · 17 comments
Closed

Need Attributes displayable like param list #102

jnothman opened this issue Jul 19, 2017 · 17 comments

Comments

@jnothman
Copy link
Member

I've not investigated when/whether there was a change in numpydoc, but scikit-learn has long relied on Attributes (as per docstring, rather than than descriptors on the class) being listed like Parameters. See old docs at http://scikit-learn.org/0.18/modules/generated/sklearn.linear_model.LogisticRegression.html for instance.

We've merged a recent numpydoc into scikit-learn master, which uses _str_member_list to render Attributes, rather than _str_param_list. See new docs at http://scikit-learn.org/dev/modules/generated/sklearn.linear_model.LogisticRegression.html. The listing looks more like methods, and hence like what you get out of autosummary for descriptors, but it altogether (and particularly the type spec) looks wrong. I'm not sure how those type specs work well for any users of Attributes.

To fix scikit-learn's needs, I propose that we extend the Jinja templating of numpy_docstring.rst such that instead of just rendering strings with e.g. {{ attributes }}, we have Jinja statements like:

{% members_list Attributes %}

or perhaps Jinja filters like:

{% raw_attributes | members_list %}

but unless we want to compose filters, I'm not sure the benefit of this.

Would this be acceptable?

@stefanv
Copy link
Contributor

stefanv commented Jul 20, 2017

I don't know why this change was introduced, but I agree that it doesn't convey the information we intended in the rendered documentation.

@jnothman
Copy link
Member Author

jnothman commented Jul 20, 2017 via email

@stefanv
Copy link
Contributor

stefanv commented Jul 20, 2017

I would; thoughts @pv?

@pv
Copy link
Member

pv commented Jul 20, 2017 via email

@jnothman
Copy link
Member Author

jnothman commented Jul 20, 2017 via email

@stefanv
Copy link
Contributor

stefanv commented Jul 20, 2017 via email

@jnothman
Copy link
Member Author

jnothman commented Jul 20, 2017 via email

@amueller
Copy link
Contributor

@pv sorry, I didn't understand the explanation.

@pv
Copy link
Member

pv commented Jul 21, 2017

@amueller: for an example, look here https://scipy.github.io/devdocs/generated/scipy.spatial.Delaunay.html --- some of the attributes are properties and have docstrings of their own.

@pv
Copy link
Member

pv commented Jul 21, 2017

@stefanv: no, not necessarily --- in some cases it's just a list of the attributes without any explanations, similar to the Methods section.

I'm not saying that it cannot be changed, but that how these sections have been used can vary.

@stefanv
Copy link
Contributor

stefanv commented Jul 21, 2017 via email

@pv
Copy link
Member

pv commented Jul 21, 2017

Well, I'm not going to argue against that :)

@jnothman
Copy link
Member Author

jnothman commented Jul 22, 2017 via email

@jnothman
Copy link
Member Author

I think the ideal functionality, ensuring backwards compatibility, would:

  • list autosummary and docstring-listed members together in the one listing
  • adopt the ordering provided in the docstring
  • summarise the description provided in the dosctring of the member where it can be resolved (as autosummary does), otherwise the description provided in the parent docstring
  • still ensure that sphinx-autogen works for autosummary entries
  • display with a definition list in order to (a) look like parameter listings; and (b) support block markup in the description

This is like extending autosummary to support:

  • specified descriptions where the member cannot be accessed
  • outputting a definition list rather than a table

The latter point could be achieved by patching upstream in sphinx perhaps to have a :layout: definition-list switch in autosummary. The former is harder to achieve because AutoSummary expects the entire content of the directive to be whitespace-separated names to be summarised. So again, this could potentially be achieved upstream with a :provided-descriptions: flag which causes it to parse the content differently. Not sure sphinx would ever accept these kinds of changes, but AutoSummary is pretty rigid as it stands.

Also, extending/reimplementing autosummary will break sphinx-autogen.

The best I can come up with in terms of keeping all of this functionality (without touching sphinx upstream) is:

  • copy and modify the AutoSummary directive as NumpyDocAutoSummaryHack to support the definition list output and the provided-descriptions input, while still taking advantage of AutoSummary features such as docstring and signature mangling.
  • translate docstrings to something hacky like the following so that they get picked up as an invocation of our hack directive, while also matching autogen's regex for autosummary:
.. numpydoc-autosummary-hack::
    .. autosummary::
        :toctree:

        these
        members
        can
        be
        autosummarised

    these
    members
    can
    be
    autosummarised
    others : array-like
        can not be autosummarised

I assume this is unmaintainable madness, but I'm not sure where to find the right compromise.

From scikit-learn's perspective, changing the member list for attributes back to a parameter listing would suffice most/all of the time.

@amueller
Copy link
Contributor

So how about a switch to on how to parse? I don't like to get involved with autosummary tbh.

@jnothman
Copy link
Member Author

jnothman commented Aug 1, 2017 via email

@amueller
Copy link
Contributor

amueller commented Aug 1, 2017

sorry, yes, how to display, I guess. And we haven't needed autosummaries yet... so global config would be the easiest work-around.

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

No branches or pull requests

4 participants