Pyro: Use PyTorch/Google style in Distributions docstrings

Created on 5 Oct 2017  路  13Comments  路  Source: pyro-ppl/pyro

To ease future migration of pyro.distributions to pytorch, we should adopt their docstring style:

PyTorch uses Google style for formatting docstrings. Length of line inside docstrings block must be limited to 80 characters to fit into Jupyter documentation popups.

In particular, this implies

  • [ ] 80 character line length in docstrings. (This is important for Jupyter usability!)
  • [ ] Single line description, followed by paragraphs of longer text, followed by Args, Returns
  • [ ] Args:, Returns:, and Raises: instead of :param:, :return:, and :raises:
documentation low priority usability

All 13 comments

Seems like it would be nice, but unlike code standardization with yapf/flake8, this would require manually rewriting every single docstring in the entire codebase for compatibility. Is this really necessary as long as Sphinx and our linter are happy?

Let's steer in that general direction. First learn what PyTorch/Google style is. Then start writing any new docstring in that style.

The 80char line limit is really important for Jupyter readability. That should be done before release.

I volunteer to reformat all Distribution docstrings. I'd really like to get them adopted upstream.

-Style Tzar

Okay, your right, I guess we can continue using Sphinx until we actually do the migration.

(But the 80char docstring thing does actually matter.)

i used sphinx style because thats what sphinx recommends (PT also uses sphinx framework for docs), but for Google style, i assume we'd need something like napolean to get it to render correctly?

Yikes, I thought Sphinx rendered Google-style correctly by default, I hope we can just copy pytorch's way. Edward copies Tensorflow's crazy docstring -> markdown script.

It took me many hours to understand TensorFlow's parser and hack at it to get it to work with Edward's. If you all agree with using Pytorch's docstring style (I do), I'm happy to submit a PR to build the parser and return Markdown documents for the user-facing Pyro docs.

@dustinvtran sorry, missed this comment, that would be awesome!

@dustinvtran thanks for the offer! well before taking on the considerable effort, i think the sphinx extension i linked to above should be able to render Google format docstrings? it wont take care of changing the _existing_ documentation in sphinx format, but afaik nothing will do that...

@jpchen You're right. It looks like PyTorch even uses Napoleon. The custom parser is more for edge cases that differ just slightly from the official Google-style docstrings in TF. Pytorch doesn't seem to need it.

@fritzo is this now moot given that we already went with a particular docstring format?

@martinjankowiak @fritzo is this change being made upstream?

I suppose this was to ease migration of the distribution library to PyTorch. Since PyTorch already follows this format, and we are rewriting all the distributions directly there, I think we can close this task.

@fritzo - Feel free to reopen, if you think there is something we need to track within Pyro.

This is fine to close. We're basically moving the math from pyro.distributions to torch.distributions and adopting the torch docstring conventions there.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

neerajprad picture neerajprad  路  4Comments

martinjankowiak picture martinjankowiak  路  3Comments

robsalomone picture robsalomone  路  4Comments

eb8680 picture eb8680  路  4Comments

fritzo picture fritzo  路  4Comments