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

Fix interaction between sticky_edges and shared axes.#16450

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
jklymak merged 1 commit intomatplotlib:masterfromanntzer:sticky-shared
Feb 10, 2020

Conversation

anntzer
Copy link
Contributor

PR Summary

Closes#16448; see description there.

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

@anntzeranntzer added the Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. labelFeb 9, 2020
@anntzeranntzer added this to thev3.2.0 milestoneFeb 9, 2020
# `get_children` would raise an AttributeError.
if self._xmargin and scalex and self._autoscaleXon:
x_stickies = np.sort(np.concatenate([
artist.sticky_edges.x
Copy link
Member

Choose a reason for hiding this comment

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

Even w/ the comment, thehasattr(ax, "lines") is a bit mysterious to me. Why are you singling outlines in particular versus other children? Why doesax.get_children error out? It should just return None, shouldn't it?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

It doesn't matter which children you pick, the point is that for twinned axes this is called even before the lines attribute iscreated (i.e. the first timeself.lines = [] is called on the Axes ever), so get_children() just raises an AttributeError when it tries to returnself.lines (among other children). (Yes, Axes init is a mess.)

Copy link
Member

Choose a reason for hiding this comment

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

I believe you, but this seems to be a band-aid rather than fixing the underlying issue, which is thatget_children is not working orself.lines should be set (I guess by callingcla)?

I don't understand howself.lines is not created since__init__ callscla...

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Yes, but cla() callsself._sharex.get_xlim() even before settingself.lines, and get_xlim causes a call to unstale_viewLim and tragedy occurs.
Really self.lines should be set even earlier in__init__, and cla() should not doself.lines = [] but justfor child in self.get_children(): child.remove() (IOW it's not cla()'s job to create the attributes, just to clear them out) but I'm not going to touch that here...

Copy link
Member

Choose a reason for hiding this comment

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

Can we changeget_children to do:

if not hasattr(self, 'lines'):    # sometimes we haven't even initialized the artist lists yet:   return []    # or None?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Actually I think this is more likely to hide bugs -- normally no one should call get_children() that early in the axes init process, so I'd rather just workaround this in the sole place I know this can happen. But I can also apply your patch if you really prefer that.

Copy link
Member

Choose a reason for hiding this comment

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

Its just really mystifying why the check would be there, even with the comment.

Copy link
Member

Choose a reason for hiding this comment

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

It is a shorthand for "is the Axes actually built yet?". Agree it would be better if we could be sure that we never call this method before the Axes is fully set up, but short of re-writing the Axes init process (which is if I understand it one of our biggest performance bottle knecks) and making sure things like this that need to be computed don't get computed until as late as possible, we have to put a band-aid someplace.

I agree with@anntzer that putting this check inget_children is likely to mask other bugs in addition to solving this one.

We can't put the fix outside of this function because we may need the rest of it to run.

We could explicitly add state_done_initing, but that get awkward when you start to have sub-classes as now the order (or if!) the subclass calls super ends up really mattering.

@jklymakjklymak merged commit4312b06 intomatplotlib:masterFeb 10, 2020
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull requestFeb 10, 2020
@anntzeranntzer deleted the sticky-shared branchFebruary 10, 2020 20:34
tacaswell added a commit that referenced this pull requestFeb 10, 2020
…450-on-v3.2.xBackport PR#16450 on branch v3.2.x (Fix interaction between sticky_edges and shared axes.)
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@jklymakjklymakjklymak left review comments

@tacaswelltacaswelltacaswell approved these changes

Assignees
No one assigned
Labels
Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Milestone
v3.2.0
Development

Successfully merging this pull request may close these issues.

Bad interaction between shared axes and pcolormesh sticky edges
3 participants
@anntzer@tacaswell@jklymak

[8]ページ先頭

©2009-2025 Movatter.jp