NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.3k

DOC: simplify API index#19727

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

timhoffm merged 9 commits intomatplotlib:masterfromjklymak:doc-api-frontpage

Mar 24, 2021

Merged

DOC: simplify API index#19727

timhoffm merged 9 commits intomatplotlib:masterfromjklymak:doc-api-frontpage

Mar 24, 2021

Conversation

Copy link

Member

jklymak commentedMar 17, 2021•
edited
Loading

PR Summary

This yields a much quicker ingress to the useful parts of the API page, and a quick example to orient the reader...

PR Checklist

Has pytest style unit tests (andpytest passes).
IsFlake 8 compliant (runflake8 on changed files to check).
New features are documented, with examples if plot related.
Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
Conforms to Matplotlib style conventions (installflake8-docstrings and runflake8 --docstring-convention=all).
New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

jklymak added the Documentation label

Mar 17, 2021

DOC: simplify API index

0e26ab8

jklymak force-pushed thedoc-api-frontpage branch from2716870 to0e26ab8Compare

March 17, 2021 21:59

Copy link

MemberAuthor

jklymak commentedMar 18, 2021•
edited
Loading

https://55150-1385122-gh.circle-artifacts.com/0/doc/build/html/index.html

jklymak added this to thev3.5.0 milestone

Mar 18, 2021

jklymak added3 commits

March 17, 2021 18:55

FIX

94ced26

More edits

e80f6b8

FIX: add API to top banner

3c9d40b

Copy link

Member

timhoffm commentedMar 18, 2021

We had API in the top menu untilhttps://matplotlib.org/3.1.0/. I think it was removed intentionally, but don't remember the discussion.

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

Suggested change

	(Note that there is also a parallel `matplotlib.pyplot` API interface
	that is considered useful for interactive work; see
	:ref:`usage patterns<usage_patterns>`, below).
	Alternatively, there is also a parallel `matplotlib.pyplot` API interface
	that is considered useful for interactive work; see
	:ref:`usage patterns<usage_patterns>`, below.

Copy link

Member

Choose a reason for hiding this comment

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

But I'm not sure we should mention the distinction here at all. Particularly also because the following example uses the object-based approach, but still needs to use pyplot for Figure and Axes creation. It's quite confusing that the non-pyplot approach still needs pyplot. So, I'd not go into the details here.

Copy link

MemberAuthor

jklymakMar 18, 2021

Choose a reason for hiding this comment

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

Sure - I struggled a bit with how to work this in, or if I should. It is such a common point of confusion that someone wrote a whole section and stuck it at the top of the API docs, so it seemed at least referring to it near the top would be useful, but I'm happy to leave it out.

doc/api/index.rst OutdatedShow resolvedHide resolved

doc/api/index.rst


		The pylab API (disapproved)
		^^^^^^^^^^^^^^^^^^^^^^^^^^^
		fig, ax = plt.subplots(figsize=(3,2), constrained_layout=True)

Copy link

Member

timhoffmMar 18, 2021

Choose a reason for hiding this comment

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

Suggested change

	fig, ax = plt.subplots(figsize=(3,2), constrained_layout=True)
	fig, ax = plt.subplots()

Keep it simple without parameters. Whileconstrained_layout is nice, IMHO this is not the place to teach it (and the benefit for this simple plot is small).

Copy link

MemberAuthor

jklymakMar 18, 2021

Choose a reason for hiding this comment

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

Yes, but then the figure is too large, in my opinion.

Copy link

Member

timhoffmMar 18, 2021

Choose a reason for hiding this comment

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

You could try with

.. plot::   :scale: 50 %

but I'm not sure if that will look good.

Copy link

MemberAuthor

jklymakMar 18, 2021

Choose a reason for hiding this comment

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

Yeah, I wish there was a two-column view.... We could just include the code and the figure as an image as well. I do think the example is useful, though, to make it concrete what we are talking about.

Copy link

MemberAuthor

jklymakMar 18, 2021

Choose a reason for hiding this comment

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

By default, the ylabel is cutoff, sosome formatting is necessary anyway.. Might as well keep what is above. I don't think we need to explain every incantation everywhere it appears.

doc/api/index.rst OutdatedShow resolvedHide resolved

jklymakand others added2 commits

March 18, 2021 13:22

Apply suggestions from code review

8029382

Co-authored-by: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>

DOC: remove sentence

961de9b

timhoffm reviewed

Mar 20, 2021

View reviewed changes

doc/api/index.rst OutdatedShow resolvedHide resolved

doc/api/index.rst Outdated

		ax.plot(x, y)
		ax.set_xlabel('t [s]')
		ax.set_ylabel('S [V]')
		fig.suptitle('Sine wave')

Copy link

Member

timhoffmMar 20, 2021

Choose a reason for hiding this comment

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

Suggested change

	fig.suptitle('Sine wave')
	ax.set_title('Sine wave')
	fig.patch.set_color('lightsteelblue')

One would not usefig.suptitle with a single Axes. This looks a bit odd because the title is centered on the figure not the Axes. There are actually not too many reasonable uses for figure methods in this simple example. Coloring the background may be one of the better ones.

Copy link

MemberAuthor

jklymakMar 20, 2021

Choose a reason for hiding this comment

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

Are we trying to soft deprecatefig.set_facecolor? I understand not wanting duplicate ways of doing the same thing, but asking folks to modify a property like this is a discoverability issue.

doc/api/index.rst


		Modules
		-------

		Matplotlib consists of the following submodules:
		Alphabetical list of submodules:

		.. toctree::
		:maxdepth: 1

Copy link

Member

timhoffmMar 20, 2021

Choose a reason for hiding this comment

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

Suggested change

	:maxdepth: 1
	:maxdepth: 1
	:multicol-toc:

Maybe.

Copy link

MemberAuthor

jklymakMar 20, 2021

Choose a reason for hiding this comment

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

Is this unreleased? I get/Users/jklymak/matplotlib/doc/api/index.rst:45: WARNING: Error in "toctree" directive: unknown option: "multicol-toc". on sphinx 3.5.3

Copy link

Member

timhoffmMar 21, 2021

Choose a reason for hiding this comment

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

Sorry, this should have been:class: multicol-toc.

multicol-toc is just a CSS class we have added to ourmpl.css.

Copy link

MemberAuthor

jklymakMar 21, 2021•
edited
Loading

Choose a reason for hiding this comment

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

oops searched the internet - didn't think to search our source code...

Copy link

Member

timhoffmMar 21, 2021

Choose a reason for hiding this comment

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

Sorry this does not seem to work... toctree: does not support a:class: attribute.

I've otherwise only used it for.. contents: so far. The above screenshot was a hacked CSS in the browser, because I didn't want to wait for a docs build.

Seems we have to live with the long list for now.

Copy link

MemberAuthor

jklymakMar 22, 2021

Choose a reason for hiding this comment

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

Ha! I thought I was going crazy... Certainly adding a class directive to toctree seems like a good idea, but seems like an upstream issue.

Fix plot

71fa500

jklymak force-pushed thedoc-api-frontpage branch from308bc7b to71fa500Compare

March 20, 2021 17:34

jklymak added2 commits

March 21, 2021 15:10

FIX: style

e46ebc1

FIX

783f606

tacaswell approved these changes

Mar 24, 2021

View reviewed changes

Copy link

Member

tacaswell commentedMar 24, 2021

I'm good with this as it is now, but did not catch up on the full discussion so am holding off on merging.

jklymak requested a review fromtimhoffm

March 24, 2021 20:32

timhoffm approved these changes

Mar 24, 2021

View reviewed changes

Copy link

Member

timhoffm left a comment

Choose a reason for hiding this comment

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

Certainly an improvement. Thanks@jklymak!

timhoffm merged commit1937d0b intomatplotlib:master

Mar 24, 2021

jklymak deleted the doc-api-frontpage branch

March 24, 2021 23:21

Copy link

MemberAuthor

jklymak commentedMar 24, 2021

Thanks both!

jklymak mentioned this pull request

Aug 29, 2023

DOC: re-add api example#26624

Closed

5 tasks

Labels

Documentation

3 participants

		(Note that there is also a parallel `matplotlib.pyplot` API interface
		that is considered useful for interactive work; see
		:ref:`usage patterns <usage_patterns>`, below).

Movatterモバイル変換

Uh oh!

DOC: simplify API index#19727

DOC: simplify API index#19727

Uh oh!

Conversation

jklymak commentedMar 17, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

jklymak commentedMar 18, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

timhoffm commentedMar 18, 2021

Uh oh!

jklymak commentedMar 18, 2021

Uh oh!

Uh oh!

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!

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!

Uh oh!

Uh oh!

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!

jklymakMar 21, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

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!

tacaswell commentedMar 24, 2021

Uh oh!

timhoffm left a comment

Choose a reason for hiding this comment

Uh oh!

jklymak commentedMar 24, 2021

Uh oh!

Uh oh!

jklymak commentedMar 17, 2021•
edited
Loading

jklymak commentedMar 18, 2021•
edited
Loading

jklymakMar 21, 2021•
edited
Loading