-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Documentation for using ConnectionPatch across Axes with constrained_… #14957
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
Documentation for using ConnectionPatch across Axes with constrained_… #14957
Conversation
…layout. When using a figure with contrained layout management, adding a ConnectionPatch between different subplot Axes breaks the layout. The artist should be removed from layout considerations before being added to the axes. This commit notes this fix in the documentation. #14907
# :class:`~matplotlib.axes.Axes` must be removed from the layout | ||
# using :meth:`~matplotlib.artist.Artist.set_in_layout` before | ||
# calling :meth:`~matplotlib.axes.Axes.add_artist`. See | ||
# :class:`~matplotlib.patches.ConnectionPatch` for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle this applies to just any artist that uses axes coordinates and is added to an axes. So similar problems are expected for annotations or annotation boxes in axes coordinates, ax{h,v}lines if they overflow the axes boundary, legends (which are actually mentionned someplace earlier in this tutorial), etc.
Maybe there is a way to formulate this more generally and name the connection patch as an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah sure. I updated the constrained_layout tutorial to apply to all artists. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contentwise I'd be fine with that. I'll leave it to a native speaker to check if that sentence makes sense grammatically. Probably, @jklymak, the original author of that tutorial would like to have a look at it anyways.
I am away from computer for a few days. If folks think this is an improvement don’t wait for me! |
Or maybe one should advocate the use of
|
I agree that using Figure.add_artist makes more sense than adding to the Axes and then excluding from the Axes layout... |
Alright, I will update the PR to suggest Figure.add_artist instead. Thanks. |
Except that |
#15020 would make it work. Once it's merged documentation can use it. |
Since #15020 was merged a few days ago, I revised the PR to recommend using Figure.add_artist(). |
Looks good to me, leaving to @ImportanceOfBeingErnest to merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meantionned a few points in above (#14957 (comment)) which would help making the option of using fig.add_artist
more discoverable for users when using layout managers.
This PR is consistent in itself and tackles some of them. So I will approve and merge.
Still further PRs, featuring the other points are still very welcome.
Thank you @canismarko and congratulations on your first Matplotlib PR 🎉 Hopefully we will hear from you again! |
Thanks, @tacaswell. I'll see if I can implement the rest of the suggestions from @ImportanceOfBeingErnest in #14957 (comment) and open a new PR when they're done. |
…layout.
When using a figure with contrained layout management, adding a
ConnectionPatch between different subplot Axes breaks the layout. The
artist should be removed from layout considerations before being added
to the axes. This commit notes this fix in the documentation.
#14907
PR Summary
PR Checklist