Skip to content

Update diagram in subplots_adjust documentation to clarify parameters #30029

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

Merged
timhoffm merged 3 commits into from
May 9, 2025
Merged

Update diagram in subplots_adjust documentation to clarify parameters #30029

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac52254
Update diagram in subplots_adjust documentation to clarify parameter …
May 9, 2025
dd7f51b
Corrected subplots_adjust diagram
May 9, 2025
785140b
Addressed diagram feedback and simplified annotation calls
May 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions doc/_embedded_plots/figure_subplots_adjust.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,30 @@


def arrow(p1, p2, **props):
axs[0, 0].annotate(
"", p1, p2, xycoords='figure fraction',
overlay.annotate(
"", p1, p2, xycoords='figure fraction',
arrowprops=dict(arrowstyle="<->", shrinkA=0, shrinkB=0, **props))


fig, axs = plt.subplots(2, 2, figsize=(6.5, 4))
fig.set_facecolor('lightblue')
fig.subplots_adjust(0.1, 0.1, 0.9, 0.9, 0.4, 0.4)

overlay = fig.add_axes([0, 0, 1, 1], zorder=100)
overlay.axis("off")

for ax in axs.flat:
ax.set(xticks=[], yticks=[])

arrow((0, 0.75), (0.1, 0.75)) # left
arrow((0.435, 0.75), (0.565, 0.75)) # wspace
arrow((0.9, 0.75), (1, 0.75)) # right
arrow((0.435, 0.25), (0.565, 0.25)) # wspace
arrow((0, 0.8), (0.9, 0.8)) # right

@story645 story645 May 9, 2025 •
edited
Loading

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.

looking at the function and function calls, I think the arrow function adds in an unncessary layer of indirection given that the only argument actually passed in is the point. I think here it would be a lot clearer to just do:

arrowprops=dict(arrowstyle="<->", shrinkA=0, shrinkB=0)
xycoords = 'figure fraction'

overlay.annotate("", (0, 0.75), (0.1, 0.75)), xycoords=xycoords, arrowprops=arrowprops)
overlay.annotate("",(0.435, 0.25), (0.565, 0.25)  , xycoords=xycoords, arrowprops=arrowprops)
overlay.annotate("", (0, 0.8), (0.9, 0.8), xycoords=xycoords, arrowprops=arrowprops)

@Bindi003 Bindi003 May 9, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @story645 , thanks for the suggestion! I've updated the code to use the direct overlay.annotate() calls as you recommended :)

@timhoffm timhoffm May 9, 2025

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.

@story645 I disagree. I intentionally introduced the arrow function as a semantic wrapper. I still find annotate("", xy1, xy2) a really awkward API. This is basically the discussion of #29826 (which is only deferred because we need to agree what the underlying artist should be), and the need for a reasonable high-level API still persists.

I'm not quite happy with reverting the wrapper, but since this is only an internal helper script and does not show up prominently in the docs, it's not worth fighting over.

@story645 story645 May 9, 2025 •
edited
Loading

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.

Sorry, but I also think doc examples are not the place to introduce new API. In this example, the extra arrow function to me felt very distracting once I figured out it wasn't doing anything extra. I would say here if anything then ConnectionPatch should just be used explicitly or the arrow function here should be wrapping ConnectionPatch.

@timhoffm timhoffm May 9, 2025

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.

This is not intoducing new API and the code is not user-facing or teaching by intent. In my perspective, it's just a helper function making the example more readable for editors of that code. Obviously, YMMV. Let's not get into the discussion right now.

@story645 story645 May 9, 2025

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.

Sorry, is kinda how I read `intentionally introduced the arrow function as a semantic wrapper. I still find annotate("", xy1, xy2) a really awkward API."

fig.text(0.05, 0.7, "left", ha="center")
fig.text(0.5, 0.7, "wspace", ha="center")
fig.text(0.95, 0.7, "right", ha="center")
fig.text(0.5, 0.3, "wspace", ha="center")
fig.text(0.05, 0.83, "right", ha="center")

arrow((0.25, 0), (0.25, 0.1)) # bottom
arrow((0.75, 0), (0.75, 0.1)) # bottom
arrow((0.25, 0.435), (0.25, 0.565)) # hspace
arrow((0.25, 0.9), (0.25, 1)) # top
fig.text(0.28, 0.05, "bottom", va="center")
arrow((0.80, 0), (0.8, 0.9)) # top
fig.text(0.65, 0.05, "bottom", va="center")
fig.text(0.28, 0.5, "hspace", va="center")
fig.text(0.28, 0.95, "top", va="center")
fig.text(0.82, 0.05, "top", va="center")