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

added QtGui.QMainWindow.closeEvent() to make sure the close event #1678

Merged
merged 2 commits into from Jan 18, 2013

Conversation

tacaswell
Copy link
Member

cascades closeEvents up properly.

issue #1676

I am not sure if there is a testable issue that shows up with out this patch, but this my understanding of good practice when over-riding functions in sub-classed QT classes.

I will look for any issues that show up without this.

@dmcdougall
Copy link
Member

Hmmm. On master my test_bbox_inches tests pass, but with your patch one of them fails. This is locally. Strange.

Edit: And it's the same diff as we remarked here

@@ -370,6 +370,7 @@ def idle_draw(*args):
class MainWindow(QtGui.QMainWindow):
def closeEvent(self, event):
self.emit(QtCore.SIGNAL('closing()'))
QtGui.QMainWindow.closeEvent(self,event)
Copy link
Member

Choose a reason for hiding this comment

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

For PEP8 compliancy, can you add a space after the comma?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry about that. That habit has not fully taken yet.

@tacaswell
Copy link
Member Author

re-based to master (to avoid

On master, I get failures on test_arraw_ptachs.test_fancyarrow.test (1 error, 2 failures) which looks like issues with size of the output images.

I get the exact same failures with this patch.

ERROR: matplotlib.tests.test_arrow_patches.test_fancyarrow.test

Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/tcaswell/local_installs/lib/python2.7/site-packages/matplotlib/testing/decorators.py", line 39, in failer
    result = f(*args, **kwargs)
  File "/home/tcaswell/local_installs/lib/python2.7/site-packages/matplotlib/testing/decorators.py", line 148, in do_test
    self._tol, in_decorator=True)
  File "/home/tcaswell/local_installs/lib/python2.7/site-packages/matplotlib/testing/compare.py", line 340, in compare_images
    np.asarray(expectedImage, dtype=np.int))
ValueError: operands could not be broadcast together with shapes (450,800,4) (600,800,4) 

@dmcdougall
Copy link
Member

@tacaswell Yes, I have reported the issue in #1681, and it's nothing to do with this patch.

I'll merge this now. Thanks.

dmcdougall added a commit that referenced this pull request Jan 18, 2013
added QtGui.QMainWindow.closeEvent() to make sure the close event
@dmcdougall dmcdougall merged commit 999f1b2 into matplotlib:master Jan 18, 2013
@tacaswell tacaswell deleted the qt4_closeevent branch January 21, 2013 05:27
tacaswell pushed a commit to tacaswell/matplotlib that referenced this pull request Jan 26, 2013
added QtGui.QMainWindow.closeEvent() to make sure the close event
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

2 participants