Skip to content

Convert VO documentation into the new organization - #219

Merged
eteq merged 4 commits into
astropy:masterfrom
mdboom:vo/docs-update
Jun 8, 2012
Merged

eteq merged 4 commits into
astropy:masterfrom
mdboom:vo/docs-update

Conversation

@mdboom

@mdboom mdboom commented May 1, 2012

Copy link
Copy Markdown
Contributor

This is a first pass at converting the io.vo 's docs to use the new subpackage template as proposed in #218.

There are a few issues I ran into, and maybe @eteq or others know the answers. Let me know if it makes sense to create new issues for them.

  1. The io.vo docs contain many links to the IOVA VOTable spec so that the reader can get more detail from the horse's mouth if desired. Previously, this was handled by putting a number of references in docs/vo/references.txt and including that from the top of every API doc. Now that most of the docstrings are on their own auto-generated pages, those links are no longer defined. It seems to me like we need a way to, from the automodapi directive, define some preamble to go at the top of every generated file. Or maybe there's another solution, but I'd like to avoid sprinkling these URLs all through the docstrings.
  2. The convention that the summary table contains the first line of the docstring is often too limiting, particularly when you want to say:
Given an `~astropy.io.vo.tree.VOTable` instance, return the first `~astropy.io.vo.tree.Table` instance.

At the moment, I've just used lines much longer than 80 characters, but I'm wondering if the summarizer can't be convinced to use the first paragraph rather than the first line.

  1. Sometimes I find myself wanting to document classes that are not made public, but are returned by the public API. For example, I have a function get_converter that returns one of many Converter subclasses. I don't want to expose these subclasses through __all__, since they shouldn't be instantiated directly, but I do want to document their methods. Any pointers?
  2. It doesn't seem that class methods get documented. If you look at a Class page, for example astropy.config.ConfigurationItem, the summary for its save method is included in the table, but there is no link to the complete documentation for save.

@mdboom

mdboom commented May 4, 2012

Copy link
Copy Markdown
Contributor Author

Just noticed that my point 4) above is a duplicate of #217.

@astrofrog

Copy link
Copy Markdown
Member

Yes, I noticed the lack of docstrings on methods too when compiling the docs for the logger. @eteq - do you have any thoughts on the other points?

@eteq

eteq commented May 8, 2012

Copy link
Copy Markdown
Member

@mdboom - I'll address 1-3, as there is indeed already an issue for 4. Hopefully once we get this ironed out, the docs can do anything we want! (I think io.vo is probably the most complex docs we have right now... although perhaps @iguananaut can correct me if its io.fits :)

  1. Just so we're clear, you want a preamble in the generated documentation, not in the references/API section that automodapi creates, is that correct? If so, feel free to create an issue on that one (and assign it to me). There are a few ways this could be implemented, although I'll need to look up a few things to decide which is cleanest and/or easiest.
  2. Unfortunately, this one is a bit harder - the table is generated by the autosummary extension, which I'm just using. The reason for the one-sentence limit is that the numpy docstring format clearly states that the docstring should start with a one-sentence summary of whatever it is. You're supposed to follow the link to the sub-page if you want to know more. Or are you saying that it's only the first line and thus cutting off the first sentence?
  3. Hmm... so you want to document a few specific classes/functions that aren't in that particular package, is that correct? I could add an option that just accepts the full path (or perhaps relative to the package that's documented?) to specific attributes, and add that into the list of items that are to be documented... That should be possible, although I'll have to look into it a bit. And do you want these documented in a separate list from the "regular" public API, or in the same function and class lists?

@mdboom

mdboom commented May 8, 2012

Copy link
Copy Markdown
Contributor Author

@eteq: Thanks for looking at this.

  1. Yes -- there needs to be some way to inject the named hyperlinks into each of the generated pages for each of the docstrings. I'll create a new issue for this to elaborate.
  2. It appears to only take the first line of the docstring. Since some of the class references can be quite long in RST (though quite short when rendered on screen in the browser), it's not always possible to fit a short summary in 80 characters.
  3. I want to document some classes that aren't listed in the module's __all__ but are returned from functions in that module. It's helpful to know how the class works, but instantiating the class is a private implementation detail. Having a way of listing additional module members might do the trick. They wouldn't have to be displayed in a separate list. This is a pretty low-priority concern relative to the others. In the end, I've just added the class to __all__ as a workaround for now.

@eteq

eteq commented May 8, 2012

Copy link
Copy Markdown
Member

(2) - hmm, odd... I'll take a look and try to fix this - I've created #231 to address this.
(3) - ok, feel free to create an issue on this either, but I'll treat it as lower priority (maybe label it a "feature request").

@astrofrog

Copy link
Copy Markdown
Member

@mdboom - regarding the first point about the links, if we merge in #217 then this should be easy to do by adding a preamble to astropy/sphinx/ext/templates/autosummary/base.rst.

EDIT: fixed issue number

@mdboom

mdboom commented May 23, 2012

Copy link
Copy Markdown
Contributor Author

Great, thanks!

@astrofrog

Copy link
Copy Markdown
Member

@mdboom - I figured out how to add reference files at the top of the generated pages, and have included it in #217.

@mdboom

mdboom commented May 23, 2012

Copy link
Copy Markdown
Contributor Author

I have merged #217 here and all seems to be well. I have to move docs/vo to docs/io/vo so that references.txt would be picked up. We should probably do the same for fits.

@astrofrog

Copy link
Copy Markdown
Member

Ok, great!

@eteq

eteq commented Jun 7, 2012

Copy link
Copy Markdown
Member

FYI, assuming #218 is accepted with the changes to automodapi, you'll want to add

Reference/API
-------------

above the automodapi directive

@eteq

eteq commented Jun 8, 2012

Copy link
Copy Markdown
Member

@mdboom - I think everything here has been addressed aside from your item 3, which I've added #265 to address in the future.

A couple other things about this:

  • I tried to merge this in with master as a test - it seemed pretty straightforward, but I'm getting a warning now when I build the docs - can you rebase against master in case I just screwed something up in the merging?
  • Also, it appears you moved the index page to vo.rst, but still have api_exceptions.rst. I would say it's fine (in fact, perhaps preferred) to have the main page inside the directory that other files associated with that page live in. That is, I'm suggesting you move vo.rst back to io/vo/index.rst or possibly io/vo/vo.rst (whichever you prefer).

@mdboom

mdboom commented Jun 8, 2012

Copy link
Copy Markdown
Contributor Author

I'm fine to move vo.rst to io/vo/index.rst (in fact, I think I prefer it, because it makes the resulting HTML URL more obvious), but the template should also be updated to reflect that. The fact that the template is in the top-level doc directory suggested to me to do it this way.

@mdboom

mdboom commented Jun 8, 2012

Copy link
Copy Markdown
Contributor Author

I think this is ready to merge now.

@eteq

eteq commented Jun 8, 2012

Copy link
Copy Markdown
Member

@mdboom - see eteq/astropy@bbd8dd8 - does the changes in the "using" section make things more clear? If so, I'll put it directly in master.

The docs look great, now, though - merging!

eteq added a commit that referenced this pull request Jun 8, 2012
Convert VO documentation into the new organization
@eteq
eteq merged commit 818d816 into astropy:master Jun 8, 2012
@mdboom

mdboom commented Jun 8, 2012

Copy link
Copy Markdown
Contributor Author

Yes -- the clarification in the template helps. Thanks!

keflavich pushed a commit to keflavich/astropy that referenced this pull request Oct 9, 2013
Convert VO documentation into the new organization
@mdboom
mdboom deleted the vo/docs-update branch May 21, 2014 23:59
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.

3 participants