Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Go back to checking figures for their manager in destroy.#18184
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| num = next((manager.num for manager in cls.figs.values() | ||
| if manager.canvas.figure == fig), None) | ||
| if num is not None: | ||
| cls.destroy(num) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can we add a comment, why we have to destroy by number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It actually works either way (manager ormanager.num); it's just that the code indestroy is simpler fornum, i.e., we know thatcls.figs.get(manager.num) is manager because we just pulledmanager fromcls.figs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Travis OSX fails. I'm unclear if the error is related to the PR.
Yea, that's weird, a restart was not effective; I'll try it out on the mac. |
It worked on the Mac Mini, so I restarted Travis again. |
4d8e0d1 tof65eb17CompareQuLogic commentedAug 6, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I tried increasing the timeout, which didn't work, but it's appearing on other tests now, so I've reverted that as the problem doesn't seem strictly related to this PR. |
As discussed on the call to get 3.3.1 out the door we are going to disable this test on travis (it should be noted that we do run this test on azure) so the current working theory is that this is a travis-specific issues with the GUI and is not worth the time right now to track down the root problem (but we should open a new issue and label it "good first issue / medium" localized, but involves defeating CI ;) ). |
This is a partial revert of a small part ofmatplotlib#13581.Fixesmatplotlib#18163.
We will follow up on this later,matplotlib#18213.
f65eb17 todb6fb4dCompare…anager in destroy.
…184-on-v3.3.xBackport PR#18184 on branch v3.3.x (Go back to checking figures for their manager in destroy.)
PR Summary
This is a partial revert of a small part of#13581.
Fixes#18163.
PR Checklist