Skip to content

Improve Figure docstrings#10595

Merged
anntzer merged 2 commits intomatplotlib:masterfrom
timhoffm:doc-figure
Mar 3, 2018
Merged

Improve Figure docstrings#10595
anntzer merged 2 commits intomatplotlib:masterfrom
timhoffm:doc-figure

Conversation

@timhoffm
Copy link
Copy Markdown
Member

PR Summary

The title says all.

Copy link
Copy Markdown
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

This seems fine, except for the one typo (IMO) at the beginning.


cax : :class:`~matplotlib.axes.Axes` object, optional
Axis into which the colorbar will be drawn
Axis into which the colorbar will be drawn.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be "Axes"

@timhoffm
Copy link
Copy Markdown
Member Author

timhoffm commented Feb 25, 2018

Strange. Just updated a typo in a docstring and suddenly an image comparison test fails. Did anything else in the build environment change?
Note that the rms 0.011 is only slightly larger than the given tolerance 0.01 for the test.

@jklymak
Copy link
Copy Markdown
Member

jklymak commented Feb 25, 2018

Yeah, I've seen the same thing. Restarting the job often gets it to pass. Not sure what causes the instability.

@timhoffm
Copy link
Copy Markdown
Member Author

Is there a way to restart just a specific CI service or job? Up to now, I've usually rebased in such a case, which then triggers the CI as a side effect.

@jklymak
Copy link
Copy Markdown
Member

jklymak commented Feb 25, 2018

For Travis, you can just hit restart.

For appveyor I think you need to log in and can restart. Its a bit of a pain.

@QuLogic
Copy link
Copy Markdown
Member

QuLogic commented Feb 25, 2018

The failures are consistent across tests, but I'm not sure why they suddenly started appearing.

@timhoffm
Copy link
Copy Markdown
Member Author

From the changed rms I would assume that some commit slightly changed the output, just about enough to make this test fail.

@timhoffm
Copy link
Copy Markdown
Member Author

Rebased.

axes = property(fget=_get_axes,
doc="List of axes in the Figure. You can access and "
"modify the axes in the Figure through this list. "
"Do not modify the list itself. Instead, use "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the second and third sentence are contradicting each other.

Copy link
Copy Markdown
Member Author

@timhoffm timhoffm Mar 1, 2018

Choose a reason for hiding this comment

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

No. Assume fig.axes being [ax1, ax2]. You can access and modify ax1, e.g. fix.axes[0].set_xlim(0, 1). But you cannot add or remove an axes from the figure by fig.axes.append(ax3) or similar.

Suggestions how to describe this more clearly are welcome.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just remove the "and modify" in the first sentence? it's not as if python had a notion of const reference anyways...


def get_frameon(self):
"""Get the boolean indicating frameon."""
"""Return a bool indicating whether the figure frame will be dawn."""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tend to write these just as "return whether ..." which seems less verbose and just as clear.
Also typo: dawn.

@anntzer anntzer merged commit 609e9bf into matplotlib:master Mar 3, 2018
@anntzer
Copy link
Copy Markdown
Contributor

anntzer commented Mar 3, 2018

thanks

@timhoffm timhoffm deleted the doc-figure branch March 3, 2018 23:37
@tacaswell tacaswell added this to the v2.2.3 milestone Aug 5, 2018
tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request Aug 5, 2018
Improve Figure docstrings
Conflicts:
	lib/matplotlib/figure.py
          - kept master version of docstring in 2 cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants