Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

DOC: remove examples of subplot(123) from docs#17335

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

Closed

Conversation

jklymak
Copy link
Member

PR Summary

Changes all*subplot(123, tosubplot(1, 2, 3 in the docs and code base.

Sorry, I dislike these thrashy PRs as well, but feel we should really soft-deprecate this usage...

PR Checklist

  • Has Pytest style unit tests
  • Code isFlake 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)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@@ -92,7 +92,7 @@ Axes creation
use::

f = Figure(figsize=(5,4), dpi=100)
a = f.add_subplot(111)
a = f.add_subplot(1, 1, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly think we should not change old API changes notes -- they should stay representative of what the lib was back when that release occured.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Agreed. Well, I kept a couple of the really recent ones...

@@ -820,7 +820,7 @@ example is generated from
s = 1 + np.sin(2 * np.pi * t)

# Note that using plt.subplots below is equivalent to using
# fig = plt.figure and then ax = fig.add_subplot(111)
# fig = plt.figure and then ax = fig.add_subplot(1, 1, 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Note thatadd_subplot(111) can be spelt even more succintly asadd_subplot() now (it defaults to 111). Especially when you're clearly not going to have a single subplot, I think the argless version is actually the cleanest.

timhoffm reacted with thumbs up emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

agreed.

@timhoffm
Copy link
Member

Semi-OT: IMHOsubplot(m, n, p) is only marginally better thansubplot(mnp). Figuring out what the p-th element always confuses me and is a lot of cognitive overhead. Unless we're making a point on using pyplot (which I think hassubplot as the canonical way of adding multiple plots) we should switch tosubplots().

@jklymak
Copy link
MemberAuthor

jklymak commentedMay 5, 2020
edited
Loading

@timhoffm I agree, but thats afar bigger job.

And theplt.subplot(m, n, p) grammar isvery ingrained, and is at least well-defined.

Converselysubplot(mnp) is a really old grammar that causes genuine problems because it only applies to 9 or fewerrows and columns subplots.

@timhoffm
Copy link
Member

Anybody usingmnp orm, n, p with p > 9 must be a genius or a masochist. 😆 But I agree that we should do the change proposed here.

BTW, we could addp = i, j so thatsubplot(4, 4, (0, 3)) would be the top right edge. Would need to decide on 0-based vs. 1-based here, which is a bit unclear as the scalar p is 1-based, but I still would use numpy/matrix indexing. But that's for another time.

@matthew-brett
Copy link
Contributor

matthew-brett commentedMay 6, 2020 via email

Nice idea - `subplot(4, 4, (0, 3))`I suppose you could just forbid `subplot(44, (0, 3))`, and then 0-basedindexing would make more sense.

@anntzer
Copy link
Contributor

subplot(i, j, (k, l)) basically already exists, except that it's spelledsubplot2grid((i, j), (k, l)) (zero-based indexing). I guesssubplot2grid is so obscure though that also having the functionality insubplot may not be a bad idea? (not that I claim any understanding of what is good taste in a pyplot-centered workflow...).

@timhoffm
Copy link
Member

timhoffm commentedMay 6, 2020
edited
Loading

And when you think you know all the miraculous ways you can create an Axes, there‘s always one more API ... 😩. I‘m really not sure we should change any of these API anymore unless we have a greater plan which and how these should be used (and maybe soft-deprecate some).

This is only referring tom, n, (i, j) not to the contents of this PR.

@jklymak
Copy link
MemberAuthor

Well, we already havem, n, (start, stop), at least until it was broken ;-) Please see#17343

@efiring
Copy link
Member

I would be happy to see the 3-digit form go--but we're still stuck with it in theaxes_grid family, I see, which also has the idiosyncraticnrows_ncols kwarg.

@jklymakjklymak marked this pull request as ready for reviewMay 20, 2020 19:58
@QuLogic
Copy link
Member

I don't see any reasons not to merge this, but it's grown a bunch of conflicts now.

@timhoffm
Copy link
Member

👍 on the PR. It's proably easier to redo this from scratch using regex serach&replace than handling all the conflicts.

@jklymak
Copy link
MemberAuthor

I only relearn regexp every 6 months or so, so this may have to wait, or anyone else is welcome to take this up instead!

@timhoffm
Copy link
Member

Most cases have been addressed via the above PRs.

Still open: More complex layouts that are not full single-cell grids. These can be handled usingsubplot_mosaic, but I'd like to wait until the single-line notation#18763 is in.

jklymak reacted with thumbs up emoji

@jklymak
Copy link
MemberAuthor

I'll go ahead and close this, but feel free to restore if you need it to track the issue...

@jklymakjklymak closed thisNov 9, 2020
@jklymakjklymak deleted the doc-soft-deprecate-subplot branchNovember 9, 2020 17:58
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@anntzeranntzeranntzer left review comments

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@jklymak@timhoffm@matthew-brett@anntzer@efiring@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp