FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

self in key_press_handler() should be canvas.manager by mspacek · Pull Request #919 · matplotlib/matplotlib · GitHub

Repository navigation

self in key_press_handler() should be canvas.manager - #919

Merged
pelson merged 1 commit into
matplotlib:masterfrom
mspacek:issue_918
Jun 5, 2012
Merged

pelson merged 1 commit into
matplotlib:masterfrom
mspacek:issue_918

Conversation

mspacek commented Jun 4, 2012

Copy link
Copy Markdown
Contributor

Fixes #918

Copy link
Copy Markdown
Member

Looks like it. I presume this fixes it for you?

efiring commented Jun 4, 2012

Copy link
Copy Markdown
Member

My only reservation is that I am fairly sure this is also fixed in another pull request, but I haven't located it yet.

efiring commented Jun 4, 2012

Copy link
Copy Markdown
Member

Found it: #851 has a different fix for this bug, so this requires a more careful look, as well as a decision on the status of #851.

mspacek commented Jun 4, 2012

Copy link
Copy Markdown
Contributor Author

Yes, my fix works for me. Ah, #851 uses fig_manager = Gcf.get_active() at line 2267 in backend_bases.py to get the manager, instead of canvas.manager. I have no idea which is better.

Copy link
Copy Markdown
Member

My vote is for your method, since "canvas" is what gets passed in, while "Gcf.get_active()" seems like a kludge that happens to work for most situations.

pelson commented Jun 5, 2012

Copy link
Copy Markdown
Member

I agree that this method is preferable (and the correct one).

@other devs: In order to merge, can I just press the "Merge pull request" button or should I merge by command line pull & push?

efiring commented Jun 5, 2012

Copy link
Copy Markdown
Member

On 06/04/2012 09:35 PM, Phil Elson wrote:

I agree that this method is preferable (and the correct one).

@other devs: In order to merge, can I just press the "Merge pull request" button or should I merge by command line pull& push?

If the button is green, push it!

Copy link
Copy Markdown
Member

Quick caveat... always make sure that the PR is to the correct branch first. Also, if you had to do testing of the PR on your system, you might as well do the merging there anyway. Then, if the merge is to the maintenance branch, you will then have to manually perform a merge down to the master branch.

pelson pushed a commit that referenced this pull request Jun 5, 2012
self in key_press_handler() should be canvas.manager
pelson merged commit 513ac45 into matplotlib:master Jun 5, 2012

pelson commented Jun 5, 2012

Copy link
Copy Markdown
Member

Thanks for fixing this @mspacek, I've merged it onto master.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

key_press_handler(): NameError: global name 'self' is not defined

4 participants


Back | FazBrowse Home | New Git URL