Add fig.add_artist method by ImportanceOfBeingErnest · Pull Request #11234 · matplotlib/matplotlib · GitHub

PR Summary

As quickly discussed on gitter, an add_artist() of Figure may be useful in some cases.
This PR would add such method.

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)

Looks great asside frm the small formatting issues! Thanks!

figure.transFigure?

I think ideally this docstring would be numpydoc compliant. Most of figure.py is not, but can't hurt to make this new one compliant.

This has a lot of "only" and "most often". Note a big reason to do this is if you don't want the artist in the tighbbox of the axes. Maybe:

Usually artists are added to axes objects using :meth:matplotlib.axes.Axes.add_artist, but use this method in the rare cases that adding directly to the figure is necessary.

method

Broken sentence

Shorter: „Determines whether ...“

It doesn't seem like the failing test is due to anything commited here.

You might need to rebase....? Looks like #11204 monkeyed w/ boilerplate... ping @anntzer

Rebasing didn't change anything.

try running boilerplate.py and commit the results.

On the plus side, this means that test is working! On the down side it is a bit annoying....

I suspect what happened is that between when the tests on #11204 ran and it got merged something that changed pyplot got merged into master.

I think the 'clip' kwarg can be removed.

I don't understand this parameter for a Figure artist; I would not expect that anything could be drawn outside the Figure patch boundaries. I did a little test that seemed to confirm that even with the ps backend, so there is white space around the figure, a Figure.figimage is clipped, regardless of whether get_clip_on() returns True or False.

What if you do savefig(bbox_inches='tight')? If clipping is on, it'll clip the tight bbox (I think, or maybe it will when some of my PRs on this get merged). If clipping is off the figure will get larger...

"an" -> "a"

The ~ cuts it to just add_artist, so 'an' is correct here.

allows to add artists -> allows artists to be added

set to it previously -> previously set

This should always be passed, unless you can guarantee that the clip path was always False, or it should default to None if you really don't want to change the existing setting. That is, assuming it has an effect (re. @efiring's comment).

Actually, I think I misread this, and thought clip was being passed down. Since it isn't, this doesn't really matter.

The difference between clip=False and clip=True can be observed when saving the figure with the bbox_inches="tight" option.

Default (i.e. clip=False)

import matplotlib.pyplot as plt
plt.rcParams["figure.figsize"] = (3,2)
plt.rcParams["figure.dpi"] = 72

fig, ax = plt.subplots()
c = plt.Circle((0,0),0.2)
fig.add_artist(c)
fig.savefig("test1.png", facecolor="azure", bbox_inches="tight")

With clip=True:

If people think this should be handled differently, I'm happy to change it. Unfortunately I'm not quite sure I understood the comments above. @efiring and @QuLogic, could you explain a bit more in detail what you have in mind concerning the clip option?

My potential objections have been shown to be invalid.

This seems done, and can/should go in for 3.0. Needs a rebase and another approver...

I rebased. There was still a comment by @QuLogic, which I did not fully understand; so maybe he wants to take another look and either approve or clarify that issue?

I needed to rebase yet again; now there is an error from circle/ci saying document not included in any doctree.

Yeah, unfortunately while the release candidate for 3.0 is cut, the whats-new should be moved to the actual whats-new.rst.

Now I get Analysis Failed (could not build the merge commit), but I looking at the log, I don't see what's failing (which is probably due to me not knowing what to look for in this long log file).

You may want to (but don't have to) use #11408 for the test, which has the benefit of directly taking care of all three output formats and generating failure diffs when needed.

Ok, did that. Locally, the svg test always skips though. Not sure about the reason.

You just don't have inkscape installed. I wouldn't worry about it...

I have several inkscape versions installed, maybe that's the problem.

...sorry, shouldn't have assumed; rephrase - inkscape wasn't callable for some reason 😉

jklymak reviewed May 11, 2018

View reviewed changes

ImportanceOfBeingErnest force-pushed the fig-add-artist branch 2 times, most recently from 9969fc4 to f1046f6 Compare May 12, 2018 00:02

dstansby added this to the v3.0 milestone May 12, 2018

jklymak approved these changes May 14, 2018

View reviewed changes

timhoffm reviewed May 14, 2018

View reviewed changes

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from f1046f6 to ec6df1d Compare May 14, 2018 20:52

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from ec6df1d to 2bdffec Compare May 14, 2018 21:36

anntzer mentioned this pull request May 15, 2018

Re-run boilerplate.py. #11251

Merged

6 tasks

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from 2bdffec to 1ab9b49 Compare May 15, 2018 08:57

efiring previously requested changes May 16, 2018

View reviewed changes

QuLogic reviewed May 16, 2018

View reviewed changes

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from 1ab9b49 to 00e0ac4 Compare May 16, 2018 19:32

jklymak added the status: needs review label May 24, 2018

jklymak added status: needs rebase Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labels Jul 5, 2018

ImportanceOfBeingErnest force-pushed the fig-add-artist branch 2 times, most recently from 1158162 to 6024bf3 Compare July 5, 2018 19:08

jklymak requested a review from QuLogic July 13, 2018 22:54

ImportanceOfBeingErnest force-pushed the fig-add-artist branch 2 times, most recently from e60a0f3 to 0ade33e Compare July 13, 2018 23:23

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from 0ade33e to 5d208e9 Compare July 14, 2018 14:56

jklymak removed the status: needs rebase label Jul 16, 2018

anntzer reviewed Jul 16, 2018

View reviewed changes

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from 5d208e9 to e4e5a0b Compare July 16, 2018 21:29

Add fig.add_artist method

b6326cc

ImportanceOfBeingErnest force-pushed the fig-add-artist branch from e4e5a0b to b6326cc Compare July 16, 2018 21:41

tacaswell approved these changes Jul 19, 2018

View reviewed changes

tacaswell merged commit ac58f1f into matplotlib:master Jul 19, 2018

QuLogic removed the status: needs review label Jul 20, 2018

ImportanceOfBeingErnest deleted the fig-add-artist branch February 17, 2019 23:08

+                          transform set to it previously, its transform will be set to
+                          ``figure.transFigure``.
+                      clip : bool, optional, default ``False``
+                          An optional parameter ``clip`` may be used to determine whether the

+                          The artist to add to the figure. If the added artist has no
+                          transform set to it previously, its transform will be set to
+                          ``figure.transFigure``.
+                      clip : bool, optional, default ``False``

+                      ----------
+                      artist : `~matplotlib.artist.Artist`
+                          The artist to add to the figure. If the added artist has no
+                          transform set to it previously, its transform will be set to

+                      Add any :class:`~matplotlib.artist.Artist` to the figure.
+                      If the added artist has no transform set to it previously, its
+                      transform will be set to ``transFigure``.

+              In case the added artist has no transform set previously, it will assume
+              set it the figure transform (``fig.transFigure``).
+              This new mathod may be useful for adding artists to figures without axes or to

		@@ -0,0 +1,16 @@
		Figure has an `~.figure.Figure.add_artist` method

+              -------------------------------------------------
+              A method `~.figure.Figure.add_artist` has been added to the
+              :class:`~.figure.Figure` class, which allows to add artists directly to a

+                          artist.set_transform(self.transFigure)
+                      if clip:
+                          artist.set_clip_path(self.patch)

		assert not(fig.get_constrained_layout())


		def test_add_artist():

Add fig.add_artist method by ImportanceOfBeingErnest · Pull Request #11234 · matplotlib/matplotlib · GitHub

Repository navigation

Add fig.add_artist method - #11234

Add fig.add_artist method#11234tacaswell merged 1 commit intomatplotlib:mastermatplotlib/matplotlib:masterfrom ImportanceOfBeingErnest:fig-add-artistImportanceOfBeingErnest/matplotlib:fig-add-artistCopy head branch name to clipboard

Conversation

ImportanceOfBeingErnest commented May 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

jklymak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ImportanceOfBeingErnest commented May 14, 2018

Uh oh!

jklymak commented May 14, 2018

Uh oh!

ImportanceOfBeingErnest commented May 14, 2018

Uh oh!

tacaswell commented May 15, 2018

Uh oh!

tacaswell commented May 15, 2018

Uh oh!

efiring left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ImportanceOfBeingErnest commented May 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jklymak commented Jul 5, 2018

Uh oh!

ImportanceOfBeingErnest commented Jul 5, 2018

Uh oh!

ImportanceOfBeingErnest commented Jul 14, 2018

Uh oh!

jklymak commented Jul 14, 2018

Uh oh!

ImportanceOfBeingErnest commented Jul 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Add fig.add_artist method#11234
tacaswell merged 1 commit into
matplotlib:masterfrom
ImportanceOfBeingErnest:fig-add-artist

ImportanceOfBeingErnest commented May 11, 2018 •
edited
Loading

ImportanceOfBeingErnest commented May 16, 2018 •
edited
Loading