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

Docstring touchups #1866

Merged
merged 8 commits into from
Nov 4, 2019
Merged

Conversation

joelostblom
Copy link
Contributor

I noticed these minor issues while working on the other PRs.

@@ -444,7 +446,7 @@


def make_docstring(fn):
tw = TextWrapper(width=79, initial_indent=" ", subsequent_indent=" ")
tw = TextWrapper(width=77, initial_indent=" ", subsequent_indent=" ")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand this change...

Copy link
Contributor Author

@joelostblom joelostblom Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's to prevent spillover in the help message that pops up when clicking shift + tab in Jupyter notebooks. I thought the width for that was 79 when I reformatted the docstrings previously, but noticed today that it actually is 77.

Before:
image

After:
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK thank you for the explanation! It's weird however because I don't have the problem
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which version of jupyter are you using? Mine is

$ jupyter --version
jupyter core     : 4.5.0
jupyter-notebook : 6.0.1
qtconsole        : 4.5.5
ipython          : 7.8.0
ipykernel        : 5.1.2
jupyter client   : 5.3.1
jupyter lab      : 1.1.4
nbconvert        : 5.6.0
ipywidgets       : 7.5.1
nbformat         : 4.4.0
traitlets        : 4.3.2

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I'm using a dark theme for jupyter which might tune the configuration. I also tested on binder and there is even more spillover I think, it looks like it does not use wrapping at all?
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue seems more complicated than I orginally thought, would you mind reverting here to 79 and opening an issue so that we can take the time to understand what's going on, and merge the rest of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is the custom dark theme that changes the width of the help box in the first screen shot (it looks like you have even more room on the right). I tried the default Jupyter Light and Dark theme and they both perform the wrapping as I indicated above:

image

On your binder screenshot, it appears as if you are running without PR #1835. I'm on jupyter 4.4.0 and jupyterlab 1.0.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you prefer if I make a new commit that to revert the change, or can I drop the old one in a rebase and then force push?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for helping to document this. OK, so no need to revert the change, I'll merge as it is.

@@ -461,6 +461,11 @@ def box(
"""
In a box plot, rows of `data_frame` are grouped together into a
box-and-whisker mark to visualize their distribution.
Each box spans from quartile 1 (Q1) to quartile 3 (Q3). The
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please add a blank line before this sentence? The convention is to have a 1-line sentence to start the docstring, then if needed explanations in another paragraph.

"If set, a vertical subplot is drawn to the right of the main plot, visualizing the y-distribution.",
],
trendline=[
"str",
"One of `'rug'`, `'box'`, `'violin'`, `'histogram'`.",
"One of `'ols'` or `'lowess'`.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops good catch!

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.

2 participants