meas_astrom tests depend on PyQt4 and Qt4 and break with PyQt5 and Qt5

XMLWordPrintable

Details

• Type: Bug
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• Sprint:
• Team:

Description

When building meas_astrom with lsstsw using -b option on CentOS7 with gcc 4.8.5 tests break through a dependency on matplotlib.

 *** error building product meas_astrom. *** exit code = 2 *** log is in /opt/lsstsw/build/meas_astrom/_build.log *** last few lines: ::::: [2016-12-13T22:08:07.868579Z] File "/opt/lsstsw/miniconda/lib/python2 .7/site-packages/matplotlib/backends/backend_qt5.py", line 31, in  ::::: [2016-12-13T22:08:07.868617Z] from .qt_compat import QtCore, QtGui,  QtWidgets, _getSaveFileName, __version__ ::::: [2016-12-13T22:08:07.868716Z] File "/opt/lsstsw/miniconda/lib/python2 .7/site-packages/matplotlib/backends/qt_compat.py", line 137, in  ::::: [2016-12-13T22:08:07.868749Z] from PyQt4 import QtCore, QtGui ::::: [2016-12-13T22:08:07.868763Z] ImportError: No module named PyQt4 ::::: [2016-12-13T22:08:07.868775Z] The following tests failed: ::::: [2016-12-13T22:08:07.869806Z] /opt/lsstsw/build/meas_astrom/tests/.test s/testFitTanSipWcsHighOrder.py.failed ::::: [2016-12-13T22:08:07.869939Z] 1 tests failed ::::: [2016-12-13T22:08:07.870705Z] scons: *** [checkTestStatus] Error 1 ::::: [2016-12-13T22:08:07.875630Z] scons: building terminated because of err ors. 

Activity

Hide
Tim Jenness added a comment -

I'm happy for that fix as well. If someone from your team could do that tweak it would be great.

Show
Tim Jenness added a comment - I'm happy for that fix as well. If someone from your team could do that tweak it would be great.
Hide
Russell Owen added a comment -

I standardized tests and library code to only try to import matplotlib when plotting is first attempted. Furthermore the library code logs a warning and continues without plotting if the import fails. The unit tests raise an exception because plotting is never the norm and if a user specifies it then they can put up with an exception if something is wrong.

I also standardized on import matplotlib.pyplot as plt, including some older code that used pylab instead of pyplot, and removed a few attempts to set the back end because I think that should be up to the user.

I left the examples unchanged.

Show
Russell Owen added a comment - I standardized tests and library code to only try to import matplotlib when plotting is first attempted. Furthermore the library code logs a warning and continues without plotting if the import fails. The unit tests raise an exception because plotting is never the norm and if a user specifies it then they can put up with an exception if something is wrong. I also standardized on import matplotlib.pyplot as plt , including some older code that used pylab instead of pyplot , and removed a few attempts to set the back end because I think that should be up to the user. I left the examples unchanged.
Hide
Tim Jenness added a comment -

Thank you. Looks good to me and a far more comprehensive cleanup than I was expecting. Thanks for clarifying why you removed the backend setting from that test.

Show
Tim Jenness added a comment - Thank you. Looks good to me and a far more comprehensive cleanup than I was expecting. Thanks for clarifying why you removed the backend setting from that test.
Hide
Russell Owen added a comment -

Thank you for the helpful review. Merged and pushed.

Show
Russell Owen added a comment - Thank you for the helpful review. Merged and pushed.
Hide
J Matt Peterson [X] (Inactive) added a comment - - edited

I just confirmed this works on Linux. I had a broken lsstsw install and ran it again with rebuild -r tickets/DM-8656 and it built successfully. Thanks.

EDIT: Updated that it works on Linux specifically.

Show
J Matt Peterson [X] (Inactive) added a comment - - edited I just confirmed this works on Linux. I had a broken lsstsw install and ran it again with rebuild -r tickets/ DM-8656 and it built successfully. Thanks. EDIT: Updated that it works on Linux specifically.

People

• Assignee:
Russell Owen
Reporter:
J Matt Peterson [X] (Inactive)
Reviewers:
Tim Jenness
Watchers:
J Matt Peterson [X] (Inactive), John Parejko, Meredith Rawls, Russell Owen, Simon Krughoff, Tim Jenness