8000 Improve indentation of Line2D properties in docstrings. by bingyao · Pull Request #14739 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]
Skip to content

Improve indentation of Line2D properties in docstrings. #14739

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 30, 2019
Merged

Improve indentation of Line2D properties in docstrings. #14739

merged 1 commit into from
Jul 30, 2019
+79 −92

Conversation

bingyao
Copy link
Contributor
@bingyao bingyao commented Jul 10, 2019 •
edited
Loading

PR Summary

This PR changed indentation of the Line2D properties in a few methods' docstrings.

When typing help(ax.grid) or plt.grid? in IPython/Jupyter:

Before the fix:
image

After the fix:
image

The line with only Properties: is not added by me, I guess it's something new feature not released yet.

@bingyao
Copy link
Contributor Author
bingyao commented Jul 10, 2019

Also, I notice that in the docstrings there are Valid kwargs are and Valid *kwargs* are, should we change make kwargs italic in these cases to keep consistency? or the other way around?

timhoffm
timhoffm approved these changes Jul 10, 2019
View reviewed changes
Copy link
Member
@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

I thought that there was a problem with the text injection when the % is not no the base column of the docstring, I cannot find the issue though and it apparently works know:
https://22900-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/api/_as_gen/matplotlib.axes.Axes.grid.html?highlight=grid#matplotlib.axes.Axes.grid

Maybe numpydoc got better in this since the last time I checked.

Anyway, it works.

@bingyao bingyao force-pushed the improve_indentation_of_Line2D_properties branch from e15005a to 6d93302 Compare July 11, 2019 03:44
@bingyao
Copy link
Contributor Author
bingyao commented Jul 11, 2019

@timhoffm Thanks for the reviewing! I added two more commits:

  • Commit 27da050:
    Change those:

    :class:`~matplotlib.lines.Line2D`, :class:`matplotlib.lines.Line2D`, `~.Line2D`

    to `.Line2D`

  • Commit 87ff17c: kwargs are, Valid kwargs are -> Valid keyword arguments are:

@jklymak
Copy link
Member
jklymak commented Jul 17, 2019

Sorry @bingyao this now needs a rebase.... Please squash your commits as well.

@bingyao bingyao force-pushed the improve_indentation_of_Line2D_properties branch 2 times, most recently from f852193 to a243188 Compare July 18, 2019 16:10
@bingyao
Copy link
Contributor Author
bingyao commented Jul 18, 2019

@jklymak Hi, I just finished rebasing and squashing.

@jklymak jklymak self-assigned this Jul 18, 2019
timhoffm
timhoffm approved these changes Jul 18, 2019
View reviewed changes
Copy link
Member
@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

We're moving now so fast with merging the piled up PRs that this staled in <= 5h. :frown:

@bingyao sorry, but could you please rebase once more?

Anybody can (and should) merge after the rebase.

@jklymak
Copy link
Member
jklymak commented Jul 18, 2019

I resolved it. The online resolver is actually pretty good.

BTW, it was "string"->"str" that caused the conflicts here.

@jklymak
Copy link
Member
jklymak commented Jul 18, 2019

Sorry for the mis-commit there...

@bingyao bingyao force-pushed the improve_indentation_of_Line2D_properties branch 2 times, most recently from 3115a43 to 7e212f9 Compare July 19, 2019 03:38
@bingyao
Copy link
Contributor Author
bingyao commented Jul 19, 2019 •
edited
Loading

@timhoffm @jklymak It's fine. I totally understand the situation. 😄Just did another rebasing and squashing.

@bingyao
Copy link
Contributor Author
bingyao commented Jul 19, 2019 •
edited
Loading

I think the testing failure of lib/matplotlib/tests/test_axes.py::test_bar_pandas is due to the setting in:
lib/matplotlib/testing/conftest.py:

try:
    from pandas.plotting import (
        deregister_matplotlib_converters as deregister)
    deregister()
except ImportError:
    pass

Although I am not very clear why deregister_matplotlib_converters() is needed in here as an overall setting.

QuLogic
QuLogic reviewed Jul 19, 2019
View reviewed changes
8000
@jklymak
Copy link
Member
jklymak commented Jul 19, 2019

We deregister the pandas converters because we want pandas to test their own converter and we will test ours. It seems the new version of pandas has broken out tests so suggest we pin away from it until we sort the problem out

@bingyao
Copy link
Contributor Author
bingyao commented Jul 19, 2019 •
edited
Loading

@jklymak Thanks! And yes, it's the pandas 0.25.0 that causes the failure, 0.24.2 was fine.

Can we change the test_bar_pandas(pd) to following to avoid this failure?

def test_bar_pandas(pd):
    # Smoke test for pandas
    pd.plotting.register_matplotlib_converters()
    fig, ax = plt.subplots()

    df = pd.DataFrame(
        {'year': [2018, 2018, 2018],
         'month': [1, 1, 1],
         'day': [1, 2, 3],
         'value': [1, 2, 3]})
    df['date'] = pd.to_datetime(df[['year', 'month', 'day']])

    monthly = df[['date', 'value']].groupby(['date']).sum()
    dates = monthly.index
    forecast = monthly['value']
    baseline = monthly['value']
    ax.bar(dates, forecast, width=10, align='center')
    ax.plot(dates, baseline, color='orange', lw=4)
    pd.plotting.deregister_matplotlib_converters()

i.e. register at beginning and deregister at the end.

@jklymak jklymak mentioned this pull request Jul 19, 2019
Closed
timhoffm
timhoffm reviewed Jul 20, 2019
View reviewed changes
@bingyao bingyao force-pushed the improve_indentation_of_Line2D_properties branch from 7e212f9 to 500153e Compare July 27, 2019 12:10< 8000 /a>
@bingyao
Copy link
Contributor Author
bingyao commented Jul 27, 2019

Sorry for the delay. And thanks @timhoffm @QuLogic for the references and reminding!

@bingyao bingyao force-pushed the improve_indentation_of_Line2D_properties branch from 500153e to 0c4714b Compare July 29, 2019 02:01
@bingyao
Copy link
Contributor Author
bingyao commented Jul 30, 2019

@jklymak Hi, I think the Needs rebase label can be removed at this time, right? 😄

@anntzer anntzer merged commit 7568084 into matplotlib:master Jul 30, 2019
@QuLogic QuLogic added this to the v3.2.0 milestone Jul 30, 2019
@bingyao bingyao deleted the improve_indentation_of_Line2D_properties branch July 31, 2019 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic left review comments

@anntzer anntzer anntzer approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees

@jklymak jklymak

Labels
Documentation
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.
6 participants
@bingyao @jklymak @QuLogic @anntzer @timhoffm @dstansby
0