Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Format docstrings to facilitate reading #154

Closed
joelostblom opened this issue Oct 19, 2019 · 5 comments · Fixed by plotly/plotly.py#1835
Closed

Format docstrings to facilitate reading #154

joelostblom opened this issue Oct 19, 2019 · 5 comments · Fixed by plotly/plotly.py#1835

Comments

@joelostblom
Copy link

joelostblom commented Oct 19, 2019

Reformatting the docstrings could make them easier to read. This is how the lineplot docstring renders in jupyterlab:

image

It is very difficult to read the "Arguments" section, mainly due to that the argument is indented rather than its description and that newlines are inserted in the middle of words so that they are split over two lines.

The image below shows what is the standard for many packages. Here, it is easy to skim the "Parameters" section to find the parameter name of interested, and the chunk of text describing that parameter is clearly indented. Words are never split over multiple lines.

image

I realize that the docstrings in plotly are not hardcoded for each function, like in other packages, so I don't know if a formatting change is possible, but it would greatly improve the reading experience if it was possible to change the docstring indentation.

@joelostblom joelostblom changed the title Indent docstrings to facilitate reading Format docstrings to facilitate reading Oct 19, 2019
@nicolaskruchten
Copy link
Contributor

Thanks for the feedback! I very much agree :)

Actually the docstrings for PX are auto-generated, so a fix might be pretty easy if you want to give it a shot... the relevant code is here: https://github.com/plotly/plotly.py/blob/master/packages/python/plotly/plotly/express/_doc.py#L376

@joelostblom
Copy link
Author

Thanks for pointing me to the right direction! As you said the reformatting is pretty straightforward. Here are three suggestions I came up with, let me know what you prefer and I can create a PR.

  1. Split the parameter name onto its own line. Although this is consistent with how the graph objects' docstrings are, there is an important difference: graph object don't have the parenthesis indicating type and default values, which does not look that neat having mixed into the text.

    image

  2. Add the parenthesized sentence on the same line as the parameter name. Since the parentheses are so long, this approaches loses the clear segmentation between parameters.

    image

  3. Only add the parameter type to the same line as the parameter name. The retains the clear parameter sections and formatting the parenthesis into a sentence makes it look tidier. This is consistent with the plotly.subplots.make_subplots docstring (and many other packages so readers would be familiar with it). This relies on docstrings being written the same way (which seems like a reasonable criteria) because I split on the : in the parenthesis (I still have to go through and see if there are any notable exceptions).

    image

In both 2 and 3, I would remove the space after the parameter name, in front of the :. Spoiler, my favorite below:

My choice would be number...
3

@nicolaskruchten
Copy link
Contributor

Thanks for the analysis! Option 3 is probably the best, I agree, although some rewriting will be necessary for the ones that accept multiple types (like x in your screenshots) to make it clear which part of the description applies to which type (i.e. if it's a string or int then it's a name or index into data_frame, otherwise it's used as-is). Also note that the current set of docstrings isn't all that consistently written (sorry!) so something more complicated might be needed than a split on colon. You may want to make the lists in the big dict be a bit more structured i.e. the first element is the one that's on the same line, and the rest aren't.

Thanks for being willing to contribute, it's much-appreciated :)

@joelostblom
Copy link
Author

joelostblom commented Oct 19, 2019 via email

@nicolaskruchten
Copy link
Contributor

I’m not that attached to the parentheses so it makes sense to get rid of them to me. It’s important to keep some clear indication of the default value, but that can go into the main body block.

Thanks again!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants