DOC: organize figure API #26629

jklymak · 2023-08-30T00:37:58Z

PR summary

This organizes the Figure API in a similar way to the Axes API. Figure is a top-level API, so deserves proper curation.

Updated a couple of figure.py docstrings to have better slugs.

Edit: built version: https://output.circle-artifacts.com/output/job/a4d69671-bb03-4c9a-a6cc-863209e4ac71/artifacts/0/doc/build/html/api/figure_api.html

PR checklist

"closes #0000" is in the body of the PR description to link the related issue
new and changed code is tested
Plotting related features are demonstrated in an example
New Features and API Changes are noted with a directive and release note
Documentation complies with general and docstring guidelines

lib/matplotlib/figure.py

story645 · 2023-08-30T00:51:02Z

doc/api/figure_api.rst

+   Figure.draw_without_rendering
+   Figure.draw_artist
+
+Other


can this go in annotate since this is also about labeling?

This is such a bizarre API I hesitate to add it at all. This should be part of the Axes API and has no business on Figure.

maybe so, but I think putting it in it;s very own section inadvertently gives it more attention

OK, done - not a huge fan, but its innocuous enough.

story645 · 2023-08-30T00:52:44Z

doc/api/figure_api.rst

+The Figure and SubFigure classes
+================================
+
+Every visualization starts with a `~.Figure` class, often instantiated via `pyplot.figure`, `.pyplot.subplots`, or `.pyplot.subplot_mosaic`.  It is possible to have `~.SubFigure`  nested inside a `~.Figure`, so many common methods are defined in `~.FigureBase`.


Suggested change

Every visualization starts with a `~.Figure` class, often instantiated via `pyplot.figure`, `.pyplot.subplots`, or `.pyplot.subplot_mosaic`. It is possible to have `~.SubFigure` nested inside a `~.Figure`, so many common methods are defined in `~.FigureBase`.

Every visualization starts with a `~.Figure` class, often instantiated via `pyplot.figure`, `.pyplot.subplots`, or `.pyplot.subplot_mosaic`. It is possible to have `~.SubFigure` objects nested inside a `~.Figure`, so many common methods are defined in `~.FigureBase`.

I think this maybe needs a sentence on what is the difference? I'm seeing the (sub) throughout here and am very confused on which when why

Changed to:

Every visualization starts with a `~.Figure` class, often instantiated via `pyplot.figure`, `.pyplot.subplots`, or `.pyplot.subplot_mosaic`. Matplotlib has the concept of a `~.SubFigure`, which is a logical figure inside a parent `~.Figure`. Most common methods are defined in `~.FigureBase`, but there are a few methods that only make sense on the parent `~.Figure`.

let me know if that is any clearer

Mostly clearer thanks, though I think instead of logical figure maybe subclass might be clearer.

This is now under Subfigure

SubFigure ========= Matplotlib has the concept of a `~.SubFigure`, which is a logical figure inside a parent `~.Figure`. It has many of the same methods as the parent. See :ref:`nested_axes_layouts`.

story645 · 2023-08-30T00:56:52Z

doc/api/figure_api.rst

+   fig = plt.figure(layout='constrained', figsize=(4, 2.5), facecolor='lightgoldenrodyellow')
+
+   # Make two subfigures, left ones more narrow than right ones:
+   sfigs = fig.subfigures(1, 2, width_ratios=[0.8, 1])
+   sfigs[0].set_facecolor('khaki')
+   sfigs[1].set_facecolor('lightsalmon')
+
+   # Add subplots to left subfigure:
+   lax = sfigs[0].subplots(2, 1)
+   sfigs[0].suptitle('Left subfigure')
+
+   # Add subplots to right subfigure:
+   rax = sfigs[1].subplots(1, 2)
+   sfigs[1].suptitle('Right subfigure')
+
+   # suptitle for the main figure:
+   fig.suptitle('Figure')
+


Probably sick of hearing this from me, but this is my usual concern that this is doing both too much and not enough - what I mean is that it feels like it's trying to show off some of the capabilities of the figure creation (like the width kwargs) but it's also not comprehensive. If there must be code (and I don't think there should be) then I think it should be at a consistent level: here is how we create figures, here is how we create subfigures - here is us showing how to use one method on both objects, and then "to learn more about this, see user guide

This code is not shown by default, though of course the user can download it if curious. The goal is to visually orient the reader as to what a Figure is versus a SubFigure.

I like visuals, but feel this is disproportional here. While SubFigure is an advanced concept that is maybe relevant for 10% of our users, the plot draws more attention to SubFigure than to Figure.

It's good that the code is not shown. I'd even claim it's not needed at all: People don't need to know how to create that figure exactly. To learn how to do SubFigures, they should go to the linked docs.
I suggest to reduce this even further to a conceptual graphic, like we're doing for the Axes creation in #26417:

While SubFigure is an advanced concept that is maybe relevant for 10% of our users, the plot draws more attention to SubFigure than to Figure.

Fair enough - the cross linking issues were such that this had to get moved down to the bottom of this page anyways.

It's good that the code is not shown. I'd even claim it's not needed at al

I'm not a fan of using graphviz or creating standalone svg's to document Matplotlib's graphics features...

This visual is now under Subfigure.

Obviously we disagree on whether such signposts are helpful. I suppose someone else will have to decide.

lib/matplotlib/figure.py

doc/api/figure_api.rst

story645 · 2023-08-30T16:53:21Z

doc/api/figure_api.rst

+   Figure.axes
+   Figure.delaxes
+
+Saving a Figure


Suggested change

Saving a Figure

Save Figure

consistent voice in titles?

Fixed all the titles - removed redundant "Figure" from them...

galleries/examples/animation/unchained.py

timhoffm

There's the downside with the Figure link targets, but I think this should not stop the PR. The usability gain is substantial.

I'll wait with merging in case somebody wants to comment on that.

timhoffm · 2023-09-01T06:54:33Z

doc/api/figure_api.rst

+The Figure class
+================
+.. autosummary::
+   :toctree: _as_gen
+   :template: autosummary_class_only.rst
+   :nosignatures:
+
+   Figure


A downside of this - and I just realized that we already have the issue for Axes - that all code links `.Figure` go to the sub-page, whereas logically it would make more sense to have them point to the index page.

I have no idea how to improve on this with existing means (well, one could introduce a custom directive :c:`Figure` , but (i) I'm reluctant to give up on the simpler formatting of `.Figure` and (ii) since that is a non-standard appoach, it's likely that new standard refrences `.Figure` will keep creeping in.) I've opened an issue to sphinx. Maybe we have to live with that for now.

I see what you are saying, but I'm not convinced it is a huge problem:

`matplotlib.figure` is the right way to get the overall index if that is what we want in the docs.

`.Figure` takes us to the instantiation docs, which I think we do not want in the overview.

At least on non-mobile, the left-hand navigation and the breadcrumb is pretty clear where the module level description is:

An alternative may be that we make entries for matplotlib.figure.Figure and matplotlib.figure.SubFigure. We don't do this with any other APIs currently, but I don't think there is any reason they API reference has to slavishly correspond to files in the library directory. I think this would mean figure_Figure_api.rst and figure_SubFigure_api.rst, but that doesn't seem the end of the world.

Finally, an even more radical suggestions would be to rearrange the source code itself; eg have a subfigure module, and split FigureBase into its own private module. That doesn't seem the end of the world - I doubt many people are doing import matplotlib.figure.SubFigure yet.

So, I think the proposed change here is better than what we currently have, but I'm open to relatively major surgery if we can get something even better.

Thinking about this more though: even if we do the more radical suggestions above, I don't see how we get the init docs onto their own page and out of the intro.

Your suggestion to sphinx seems reasonable. Not sure how we emulate that internally.

One could try to separate class and init docstring, put the class on the overview page and init in subpage where the class currently is. May be possible through appropriate configuration https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#configuration.

But that can be a follow up PR. I suggest to merge this as is for now.

I think a fair bit is possible with templates as well.

See newest version...

jklymak · 2023-09-01T21:31:49Z

So this version the init docstring shows up on the main api page, but I think .Figure resolves to this page, and having the init docstring on here is not terrible. I've added a TOC to make it easier to see that the page has more below the init docstring. I did the same thing for SubFigure.

Compare here: https://output.circle-artifacts.com/output/job/66aaf31d-2c9c-478b-80dd-d816d8157d4f/artifacts/0/doc/build/html/api/figure_api.html#
To previous version:
https://output.circle-artifacts.com/output/job/a4d69671-bb03-4c9a-a6cc-863209e4ac71/artifacts/0/doc/build/html/api/figure_api.html

timhoffm · 2023-09-03T23:24:56Z

👍 for the TOC.

So this version the init docstring shows up on the main api page, but I think .Figure resolves to this page, and having the init docstring on here is not terrible.

IMHO this doesn't really cut it:

The Figure.__init___ docstring ist mostly irrelevant. Regular users use one of our factory functions and hardly ever instantiate Figures explicitly.
Yes the .Figure link resolves to the page, but to some place in the middle of it, where you only see the (irrelevant) Figure.__init__ docstring.

I'd rather stick with the previous solution, merge, and fiddle with improvements in a subsequent PR. Note that Axes has the same issue, and it's an argument to treat them synchronously.

That said, my approval stands with either version. They are both an improvement, should be merged soon, and could need further improvement IMHO. Ping @story645 is your change request reasonably addressed?

jklymak · 2023-09-03T23:35:56Z

I'm fine putting it back the way it was. Do we need the TOC though if we don't have anything interrupting access to the table?

timhoffm · 2023-09-04T00:16:13Z

Do we need the TOC though if we don't have anything interrupting access to the table?

Either way is fine for me.

story645

My major concerns were resolved, thanks @jklymak! I don't think the TOC is necessary b/c we have it on the RHS and I agree w/ @timhoffm about the Figure API docs.

I think that one example has to get fixed, but other than that the comments are on the take or leave side.

story645 · 2023-09-04T02:35:39Z

doc/api/figure_api.rst

+   Figure.get_constrained_layout_pads
+
+
+Interacting


Suggested change

Interacting

Interactivity

slight nit that I think this tends to be common usage so folks are more likely to parse quickly

Except then the voice would be inconsistent.

hmm - Interactivity acts like a noun like layout rather than a verb like annotating, so it's still consistent?

OK, changed to "Interactive", which is what's used for all the other API pages (for better or worse). We can search and replace to "Interactivity" if that is better as a follow-up, but at least now it's consistent.

doc/api/figure_api.rst

galleries/users_explain/figure/figure_intro.rst

lib/matplotlib/figure.py

story645 · 2023-09-04T02:50:04Z

doc/api/figure_api.rst

+
+.. plot::
+
+   fig = plt.figure(layout='constrained', figsize=(4, 2.5), facecolor='lightgoldenrodyellow')


This is probably the silliest little nit but I wish this figure was a bit wider to match the text width.

The width of the figure is fixed at approximately 1/2 the maximum width of the text, so it would have to be twice as wide to match the text width. You must be looking at a thinner viewport.

jklymak · 2023-09-14T16:14:46Z

@story645 you are blocking on this.

story645 · 2023-09-14T19:42:49Z

doc/api/figure_api.rst

+   Figure.get_constrained_layout_pads
+
+
+Interacting


hmm - Interactivity acts like a noun like layout rather than a verb like annotating, so it's still consistent?

timhoffm · 2023-09-15T08:48:09Z

Since this has changes in .py files, we cannot backport to 3.8-doc.

This was accidentally lost through matplotlib#26629.

story645 requested changes Aug 30, 2023

View reviewed changes

jklymak force-pushed the doc-figure-api branch from b9d2246 to c78dc69 Compare August 30, 2023 01:19

jklymak marked this pull request as draft August 30, 2023 02:46

timhoffm reviewed Aug 30, 2023

View reviewed changes

lib/matplotlib/figure.py Outdated Show resolved Hide resolved

story645 reviewed Aug 30, 2023

View reviewed changes

story645 added the Documentation: API files in lib/ and doc/api label Aug 31, 2023

story645 reviewed Aug 31, 2023

View reviewed changes

galleries/examples/animation/unchained.py Outdated Show resolved Hide resolved

jklymak force-pushed the doc-figure-api branch from 8edabf1 to 1a4904f Compare August 31, 2023 15:12

github-actions bot added the status: needs rebase label Aug 31, 2023

jklymak force-pushed the doc-figure-api branch from 5dc7e0a to 9ff79fa Compare August 31, 2023 15:37

github-actions bot removed the status: needs rebase label Aug 31, 2023

jklymak force-pushed the doc-figure-api branch 3 times, most recently from 8d356c2 to 7b579a6 Compare August 31, 2023 18:10

jklymak marked this pull request as ready for review August 31, 2023 18:10

timhoffm approved these changes Sep 1, 2023

View reviewed changes

jklymak force-pushed the doc-figure-api branch from c232a1c to 24055e6 Compare September 1, 2023 16:16

jklymak force-pushed the doc-figure-api branch from 8356630 to 381a14e Compare September 2, 2023 15:12

story645 reviewed Sep 4, 2023

View reviewed changes

timhoffm mentioned this pull request Sep 4, 2023

[MNT]: Inconsistent API Figure.suptitle() #26691

Open

jklymak requested a review from story645 September 14, 2023 16:12

story645 approved these changes Sep 14, 2023

View reviewed changes

DOC: organize figure API [ci doc]

448e9f6

jklymak force-pushed the doc-figure-api branch from 2065f14 to 448e9f6 Compare September 14, 2023 23:21

timhoffm added this to the v3.8.1 milestone Sep 15, 2023

timhoffm merged commit c5707d9 into matplotlib:main Sep 15, 2023
29 of 30 checks passed

meeseeksmachine mentioned this pull request Sep 15, 2023

Backport PR #26629 on branch v3.8.x (DOC: organize figure API) #26783

Closed

meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Sep 15, 2023

Backport PR matplotlib#26629: DOC: organize figure API

84a6cfc

timhoffm added a commit to timhoffm/matplotlib that referenced this pull request Oct 14, 2023

Restore figaspect() API documentation

82503fb

This was accidentally lost through matplotlib#26629.

This was referenced Oct 14, 2023

Restore figaspect() API documentation #27090

Merged

DOC: Move figure member sections one level down #27110

Merged

QuLogic modified the milestones: v3.8.1, v3.9.0 Oct 31, 2023

	Every visualization starts with a `~.Figure` class, often instantiated via `pyplot.figure`, `.pyplot.subplots`, or `.pyplot.subplot_mosaic`. It is possible to have `~.SubFigure` nested inside a `~.Figure`, so many common methods are defined in `~.FigureBase`.
	Every visualization starts with a `~.Figure` class, often instantiated via `pyplot.figure`, `.pyplot.subplots`, or `.pyplot.subplot_mosaic`. It is possible to have `~.SubFigure` objects nested inside a `~.Figure`, so many common methods are defined in `~.FigureBase`.


		.. plot::

		fig = plt.figure(layout='constrained', figsize=(4, 2.5), facecolor='lightgoldenrodyellow')

DOC: organize figure API #26629

DOC: organize figure API #26629

Conversation

jklymak commented Aug 30, 2023 • edited

PR summary

PR checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak Aug 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm Sep 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak commented Sep 1, 2023 • edited

timhoffm commented Sep 3, 2023

jklymak commented Sep 3, 2023

timhoffm commented Sep 4, 2023

story645 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklymak commented Sep 14, 2023

Choose a reason for hiding this comment

timhoffm commented Sep 15, 2023

jklymak commented Aug 30, 2023 •

edited

jklymak Aug 30, 2023 •

edited

timhoffm Sep 1, 2023 •

edited

jklymak commented Sep 1, 2023 •

edited