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

Allow sharex/y after axes creation.#15287

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

Merged
efiring merged 1 commit intomatplotlib:masterfromanntzer:lateshare
Apr 8, 2020

Conversation

anntzer
Copy link
Contributor

This intentionally does not allow unsharing or changing the shared axes,
as there are bigger questions on the API there (#9923 and issues linked there).

The API is namedAxes.sharex() to allow for a laterAxes.unsharex(),
though (set_sharex() would be fine, butset_unsharex()? or perhaps
set_sharex(None), though). Happy to hear suggestions.

An example use case is creating a grid withsubplots(), but with
custom sharing relationships between the subplots -- e.g., sharex/sharey
across all, except the first row of axes which only shares x with their
column and the first column which only shares y with their lines.

PR Summary

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

berceanu and story645 reacted with thumbs up emoji
Copy link
Member

@jklymakjklymak left a comment

Choose a reason for hiding this comment

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

Overall this seems a useful API though I suspect we haven’t thought through how it will be used. Allowing sharing to be toggled like this seems like it will be ripe for misunderstandings. Could use some examples of why you think this is useful

"""
Share the x-axis with *other*.

This is equivalent to passing ``sharex=other`` when constructing the
Copy link
Member

Choose a reason for hiding this comment

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

This docstring could be more explicit and non circular. For instance the verb to share would indicate that self shares with other, whereas the exact opposite happens. Further passing sharex=other calls this function so I don’t think it’s very helpful to say that this function is the same as that and expect the user to ferret out the behaviour

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

"self shares with other, whereas the exact opposite happens"
What do you mean by "opposite"? Isn't sharing symmetrical? (well perhaps not deep in the implementation details, but from the user PoV it is)

Copy link
Member

Choose a reason for hiding this comment

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

I meant in everyday English the transitive verb “to share” means one person shares Something with another

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Can you propose an alternate wording here?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, the main point is that sayingax.sharex is "Equivalent to passing sharex=other when constructing..." is a circular doc, because when you construct, you callax.sharex. This function ideally would define whatax.sharex does explicitly.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Right now the docs of sharex (as kwargs to the Axes constructor) are

        sharex, sharey : `~.axes.Axes`, optional            The x or y `~.matplotlib.axis` is shared with the x or            y axis in the input `~.axes.Axes`.

which is not much better :/ I agree this can be improved, but is arguably a separate issue?

Copy link
Member

Choose a reason for hiding this comment

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

The docstring defines what the function does for the user, and one of our main problems with "share" is that people don't know what it means and even in the history of the code base different definitions have popped up. Right now I'm not 100% sure what this PR changed and I think it should be defined clearly, in plain English, what calling this function will do.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

ax0 = (some axes)ax1 = fig.add_subplot(...)ax1.sharex(ax0)

is equivalent to

ax0 = (some axes)ax1 = fig.add_subplot(..., sharex=ax0)

Now it is true that what the latter does is not really well documented either, but the doc available forax1.sharex(ax0) is, well, exactly the same.

@anntzer
Copy link
ContributorAuthor

I'm explicitlynot allowing toggling, or changing who you share with, just going from "not-shared" to "shared". Indeed, allowing unsharing makes the problem much more complex.

The use case hinted above is:

  A A AB X X XB X X XB X X X

Each A plots the distribution (histogram) of variablea in conditions a1, a2, a3; each B plots the distribution of variableb in conditions b1, b2, b3, and each X plots the joint distribution (scatter plot). Here you want to sharex across rows and sharey across columns,except that the first row does not share y's and the first column does not share x's (let's say you don't want to normalize your histograms).
Right now if you want to do this you basically need to give up on subplots() and manually add axes in a loop while keeping track of who to share with what -- and that info needs to be passed immediately.
With the PR here, you can start by calling subplots(), then create the required share-linkages.

@jklymak
Copy link
Member

I understand your example, but it would be helpful if you actually provided one in the PR.

@anntzer
Copy link
ContributorAuthor

Do you mean one as comments in the code? (where?) in the commit message?

@jklymak
Copy link
Member

I meant as an example in the example folder

@anntzer
Copy link
ContributorAuthor

Sure, added a simple example.

@anntzeranntzerforce-pushed thelateshare branch 3 times, most recently from850d310 tof1ed957CompareSeptember 26, 2019 13:39
@jklymakjklymak added this to thev3.3.0 milestoneOct 1, 2019
@jklymakjklymak added the topic: geometry managerLayoutEngine, Constrained layout, Tight layout labelOct 1, 2019
@efiring
Copy link
Member

First thoughts on this:

  1. The API, with a simplesharex() method, is reasonable. I would not advocate prependingset_. More descriptive alternatives would besharex_with() andsharex_from(). The latter has the advantage of noting that info (objects to be shared) will come from the argument, not go to it.
  2. It seems like unsharing could be added by supporting theNone argument, and replacing the shared objects with deep copies of them; but I haven't looked at this in detail.
  3. Leaving aside the unsharing, which I agree does not have to be addressed here, I think the present approach would work fine when used early--immediately after Axes creation--but inevitably users will find ways to make it surprise them. For example, if two Axes are created, each is called withset_aspect(1, adjustable='datalim'), and then some time later they are shared, the problem won't be caught untildraw(), whenapply_aspect() is called.

@anntzer
Copy link
ContributorAuthor

I guess I'm +0.5 on sharex_with() and +0.25 on sharex_from() (both are better than sharex()), let's see what others think.

What about adding something like

Note that this method is only intended to be called immediately after the axes has been created; unexpected behavior may occur if the axes already has its own aspect ratio, formatters, or locators set up.

to the docstrings? (I don't really intend to support "later" calls with this PR anyways.)

@anntzeranntzerforce-pushed thelateshare branch 2 times, most recently from1abd103 tob57a4daCompareJanuary 23, 2020 23:22
@anntzeranntzer mentioned this pull requestFeb 28, 2020
13 tasks
Copy link
Member

@efiringefiring left a comment

Choose a reason for hiding this comment

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

There might be unintended consequences, but let's try it.
Approved, subject to rebase.

This intentionally does not allow unsharing or changing the shared axes,as there are bigger questions on the API there.The API is named `Axes.sharex()` to allow for a later `Axes.unsharex()`,though (`set_sharex()` would be fine, but `set_unsharex()`? or perhaps`set_sharex(None)`, though).An example use case is creating a grid with `subplots()`, but withcustom sharing relationships between the subplots -- e.g., sharex/shareyacross all, except the first row of axes which only shares x with theircolumn and the first column which only shares y with their lines.
@anntzer
Copy link
ContributorAuthor

rebased

@efiring
Copy link
Member

We seem to be getting a systematic failure--not just on this PR--in the macosx build on Travis, intest_bbox_inches_tight_raster[pdf], an image comparison failure.
In addition there are the two circleci docs failures here. Any idea what is going on?@QuLogic?

@QuLogic
Copy link
Member

macOS failure is#17065; doc failure is just spurious inability to download a Sphinx inventory.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@efiringefiringefiring approved these changes

@jklymakjklymakjklymak approved these changes

Assignees

@efiringefiring

Labels
topic: geometry managerLayoutEngine, Constrained layout, Tight layout
Projects
None yet
Milestone
v3.3.0
Development

Successfully merging this pull request may close these issues.

4 participants
@anntzer@jklymak@efiring@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp