Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qt closeevent fixes for v1.2.x #1705

Merged
merged 10 commits into from Jan 28, 2013
Merged

Conversation

tacaswell
Copy link
Member

This rolls up PR #1678 and PR #1683 cherry-picked on to v1.2.x.

For future reference, is there a better way to do something like this than cherry-picking each commit by hand? (my naive attempt to re-base blew up as it tried to move all of master over).

dmcdougall and others added 10 commits January 25, 2013 22:04
added QtGui.QMainWindow.closeEvent() to make sure the close event
Gcf when it is closed.

In PR matplotlib#1498 the attribute WA_DeleteOnClose was no longer set on the
QtMainWindow object in QtFigureManager.  The signal connection that
was being used to remove the figure from Gcf when the window was closed
was tied to the `destroyed()` signal of QtMainWindow, which is no
longer being destroyed.  Thus, gca and gcf would return references
to no-longer visible figures/axes.  _widgetclosed is now called when
MainWindow emits 'closing()'.
as this should hit either PySide or PyQt4, depending on which is
installed.  (I have faith that the setup code that will make sure that
if only one of them is installed, it defaults to using that one)
@mdboom
Copy link
Member

mdboom commented Jan 26, 2013

You can cherry-pick the merge commit (the one that's displayed in the PR). Generally, we just do this manually rather than through a PR, but it doesn't really matter.

@dmcdougall
Copy link
Member

@tacaswell I think it's possible to cherry-pick a range of commits, but I haven't had success with it.

Also, when I said 'rebase' I didn't actually mean git rebase, sorry for the confusion. It's not possible to git rebase --onto from a development branch to a release branch without including development history which may contain api changes or new features. You can, however, git rebase --onto from a release branch onto the development branch. This is akin to a merge, but will play history on top of the tip of the development branch.

To be honest, I think I'm just used to thinking to myself, "Are my changes going to fix a bug or enhance the codebase?" before I branch. I realise it may take a while to force yourself to do that, though. I also realise you may enhance code while fixing a bug. If in doubt, branch off of the release branch because then you can rebase it onto master (or merge it in) if you end up making a bigger change than you had initially envisaged.

As usual, thanks for the hard work an perseverance.

@tacaswell
Copy link
Member Author

OK, I will be more careful in the future. Sorry for learning via destruction.

If you do these moves by hand, PR #1678 should also be marked as v1.2.x milestone.

@dmcdougall
Copy link
Member

OK, I will be more careful in the future. Sorry for learning via destruction.

But it's the best way! Joking aside, it's not a problem.

I'll leave the merging decision to @mdboom, but this gets my +1.

@mdboom
Copy link
Member

mdboom commented Jan 28, 2013

Ok. I'll merge this and then merge it onto master (and just close #1683). That follows our usual procedure. Thanks for getting this all into shape!

mdboom added a commit that referenced this pull request Jan 28, 2013
@mdboom mdboom merged commit 7147317 into matplotlib:v1.2.x Jan 28, 2013
@tacaswell tacaswell deleted the qt_closeevent_v1.2 branch January 28, 2013 16:51
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.

None yet

3 participants