Format attributes like parameters #106
Conversation
- Provide links to autogen when possible - Use fake (commented) autosummary nodes (as already used in scipy docs)
|
Is there something special I need to do to make autogen work for attributes on the scipy docs? I'm not getting the right results on numpydoc master (i.e. autogen attributes in |
Are you getting similar results as at https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.Delaunay.html#scipy.spatial.Delaunay? Looks like the listed attributes that are defined with |
|
Had a look at building the scipy docs with/without this PR. Without: With: Conclusions:
Overall LGTM. |
|
well I've not managed to get hyperlinks locally with either branch of
numpydoc, but I'm glad you can.
I'm not sure what the story is with the italic blue formatting as opposed
to the red tt.
Thanks for checking and for the LGTM. I think this puts more control in the
docstring and that's a good thing.
…On 15 Aug 2017 7:24 pm, "Ralf Gommers" ***@***.***> wrote:
Had a look at building the scipy docs with/without this PR.
Without:
[image: image]
<https://user-images.githubusercontent.com/98330/29309846-81d5a160-81ff-11e7-86fe-3a36609da4be.png>
With:
[image: image]
<https://user-images.githubusercontent.com/98330/29309865-9353c2e6-81ff-11e7-9b0c-43fcad32fa58.png>
docs built with:
$ cd doc
$ rm -rf sphinxext/numpydoc # ensure to pick up the installed version
$ make html-scipyorg
Conclusions:
- hyperlinks are in both cases only generated for properties, which is
OK.
- without this PR the properties are listed first, irrespective of the
ordering in the Attributes table. With this PR the listing is in the order
of the table. OK with me, no preference either way
- type descriptors are now visible, without this PR they are not.
Overall LGTM.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#106 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEz6wR8w7xM31Fppz3wkhKauANMX4p3ks5sYWPCgaJpZM4Ou4WR>
.
|
|
What happens if the Attributes list is just a list of attributes,
without descriptions, but listed items are properties?
|
|
If there is a property by the listed name, it's docstring is truncated and displayed if present, taking precedence over a given description. This emulates current behaviour. |
numpydoc/docscrape_sphinx.py
Outdated
| param_obj = None | ||
| obj_doc = pydoc.getdoc(param_obj) | ||
|
|
||
| if param_obj and (obj_doc or not desc): |
There was a problem hiding this comment.
@pv, this is the logic for constructing the description when the attribute corresponds to an object. Firstly, above, we only link to the attribute's autodoc if it has either a docstring or is not described in the numpydoc (and I'd be okay with removing the or not desc, except that it is current behaviour).
If it has a docstring, we try to emulate autosummary: split the object's docstring at its first blank line. Then take a prefix up to the first . if present, or the whole first paragraph otherwise.
I should probably test this logic more thoroughly before merge.
There was a problem hiding this comment.
it turns out the "or not desc" logic here wasn't working because desc is being provided as [''] when there is no description line. Which means no one depends on this logic, which means I'm getting rid of it because it's weird (unless you can think of a use-case for linking to an autogen page where the attribute has no docstring).
And remove quirky 'or not desc' case
|
One more vote towards merging this PR here. It'd be great for lots of other projects, like mine:
|
|
I'm fine with this going in; I am a little concerned about the added complexity, and the lack of documentation to describe the changes. Should we consider starting a developer doc, so that people can at least work on the project with an understanding for the historical motivations behind choices? Otherwise, just much more detailed docstrings for newly added features? |
|
I agree it adds a lot of complexity. I'm happy to factor bits out to improve readability. I disagree that this is about historical motivations. It is about wanting something that functions like autosummary and also like parameter listing, where no corresponding python object can be retrieved. It also fixes issues about the inconsistent display of types and lack of support for block markup in descriptions of attributes. I agree that numpydoc should have better documentation:
|
|
To clarify: I did not suggest that this was about a historical motivation. Merely that, developers coming in from the future (!) will have to look at these (historical) decisions we are making now, and that it would help if we noted down their motivation (as we make them). |
|
How does this look to you, @stefanv? |
|
That's much better; @pv? |
|
The screenshot for scipy docs above looks fine to me, so no objections. |
|
thanks for merge!
…On 10 Sep 2017 6:10 am, "Stefan van der Walt" ***@***.***> wrote:
Merged #106 <#106>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#106 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEz66VlRbSrOwL4of1uwwfHP9efe5Nhks5sgvCpgaJpZM4Ou4WR>
.
|
|
Awesome, thanks @jnothman ! |
|
Rather late to the party, but on the pandas side we are also looking into moving to use upstream numpydoc instead our vendored version + hacks. The main reason for that is because we don't list attributes explicitly in our docstrings, and thus solely rely on the inferred docstrings. That also means that we never have type descriptions for attributes, hence making the parameter listing much more spacious (for nothing) compared to the autosummary methods listing. Would it be possible to have this as an option? Altough I suppose that would even further complicate the code .. |
|
to be sure, you don't care whether we're using autosummary or not, you care
that it looks different? And if we changed the method summary to look the
same would that be better or worse? And if we made a way for you to use CSS
to style these as tables, would that be better?
IMO, attributes are more like return values from the perspectives of
documentation than they are like methods, in that they have types, etc...
Except that you may also want to have sections like examples and see also,
which is why you want to link to their own separate documentation, I
presume.
…On 10 Nov 2017 3:59 am, "Joris Van den Bossche" ***@***.***> wrote:
Rather late to the party, but on the pandas side we are also looking into
moving to use upstream numpydoc instead our vendored version + hacks.
However, I think we (currently) would prefer how it was before: an
autosummary table for the attributes, to have it consistently with the
methods (see eg http://pandas.pydata.org/pandas-docs/stable/generated/
pandas.DataFrame.html).
The main reason for that is because we don't list attributes explicitly in
our docstrings, and thus solely rely on the inferred docstrings. That also
means that we never have type descriptions for attributes, hence making the
parameter listing much more spacious (for nothing) compared to the
autosummary methods listing.
Would it be possible to have this as an option? Altough I suppose that
would even further complicate the code ..
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#106 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEz6wZhcstcEH-toR9MuiL53gLFMOhnks5s0y9ggaJpZM4Ou4WR>
.
|
Not only that. I also think the methods listing looks better for our use case (but, that can also just be the conservative bit in me regarding changing how things look ..). I think this stems a bit from a different usage of the class docstring pages. In scikit-learn they contain all the information, for both the attributes and the methods all documentation is listed on that page. In the pandas docs, we have separate pages for attributes/methods, and on the class docstring page we only want to have a summary table for them and link to the actual pages.
Not fully sure. Currently, the autosummary table will automatically only give the first line of the docstring (and this is an aspect we want to keep I think). If that can be achieved with css to style them as summary tables, then yes, that would be OK.
Indeed, although I will not say this is the case currently for many docstrings. But ideally, yes. ( Just out of curiosity: how would a method look like in such a parameter-style listing? What would be put in the type description? The signature? Or return type? |
|
currently the description will be extracted from the attribute docstring if
present, and abbreviated as in autosummary.
I'm not sure about where function signatures fit into a similar method
listing, and it should be possible to do a lot with CSS if we can mark the
sections, although I'm not sure whether the colon and type description will
be easy to mark and style separately
|
|
To come back to the original question: would you be open to a config option to choose the type of listing ? |
|
I don't think adding the option is too hard (code-wise, but of course it adds to the user complexity). See proof of concept: jorisvandenbossche@cac8fe2 (certainly missing docs and tests) |
|
could also format both and allow a custom template to decide between. but
fwiw it's not hard to change a dl to look like a table (with fixed width
columns) in css
|
Not hard for somebody who knows css maybe :-) If that is the preferred route, I would appreciate some input on that front |
|
I'm looking now Joris. Will post in a bit.
…On Tue, Nov 14, 2017 at 5:07 AM, Joris Van den Bossche < ***@***.***> wrote:
but fwiw it's not hard to change a dl to look like a table (with fixed
width columns) in css
Not hard for somebody who knows css maybe :-) If that is the preferred
route, I would appreciate some input on that front
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#106 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABQHInR14Iq-8gAsDfwHi20imVBcnna-ks5s2XRhgaJpZM4Ou4WR>
.
|
|
Well, I don't see an easy way to select just the And for attribute the only difference is the value in the |
|
well I've been working on a fix to that. but assuming that's not an issue,
displaying as inline-block should put the dd next to the dt.
|
|
The problem with a css solution is that, to mimic the member listing, we also want the attribute names to be links to the separate docstring pages (and the docstring pages should be generated). |
|
@jorisvandenbossche they're links:
A simple |
|
Ah sorry, my bad. In my test it were not links because the attribute pages didn't exist (that's a problem with numpydoc no longer generating them, but which is something we can solve in pandas by listing them all manually (as you are doing)) Anyhow, I think my proposed fix (jorisvandenbossche@cac8fe2) is rather simple if it seems difficult to get a nice way to do it with css |
|
I don't know if/when numpydoc stopped generating them, but I don't think
it's due to a recent change.
|
|
You're right. I was testing both numpydoc master and sphinx 1.6 at the same time (our pandas docs are currently pinned at older numpydoc and sphinx 1.5), and apparently the missing attributes were caused by the sphinx upgrade (sphinx-doc/sphinx#3235) |
|
I would like to come back to this, as this is one of the blockers for pandas to use upstream numpydoc. As said above, I think adding an option to control this, is rather easy (eg jorisvandenbossche@cac8fe2). I could do a PR for this, if that would be accepted. |
|
@jorisvandenbossche I can take a look as well if you have difficulty with the HTML / CSS. I don't recall how much experience you have with those. |
|
What's the deal with the screenshot you show above? The definition list items are all over the place. I can accept a switch, but given how brittle rest can be, it becomes hard to test with more switches. |
Not sure. My comment seems to suggest I was manually adding |
A little bit, but nothing more than googling and trying until something 'looks' ok on my computer. While adding an option is a few lines understandable python code :-) And apart from that, I still don't really see how it would be possible to get it fully looking the same as the Methods autosummary table listing. Of course if that is needed is debatable, but IMO giving it a different style than the methods overview will only give 'visual noise'.
It's true that the definition list is used elsewhere, but the autosummary table is also not specific to this (and in our docs also all over the place). I mean, it is not introducing new formatting code, only a switch between two already existing formatting options. |
Resolves #102.
Resolves #104.
Resolves scikit-learn/scikit-learn#9495
I'm a bit confused about how/whether the autosummaries inside numpydoc-munged docstrings work. I'm not quite sure whether this patch is working on scipy docs and would appreciate someone checking. It's good for scikit-learn.