search

glue-viz / glue

Linked Data Visualizations Across Multiple Files
http://glueviz.org
Other
742 stars 153 forks source link

Selecting nodes in the link editor is broken with PyQt5 #2374

Closed jfoster17 closed 1 year ago

jfoster17 commented 1 year ago

Describe the bug The link editor find_object function is broken with 3+ datasets and at least some versions of PyQt5 and the latest dev version of glue. The problem is that 71ca079 changed to use event.position() which seems correct for Qt6, but is not (generally?/always?) available for earlier versions.

To Reproduce Add 3+ datasets and open the link editor with Qt5 installed. Attempting to click on a node in the link editor fails with

Traceback (most recent call last): File "/Users/jfoster/python-repos/glue/glue/dialogs/link_editor/qt/data_graph.py", line 374, in mouseMoveEvent selected = self.find_object(event) File "/Users/jfoster/python-repos/glue/glue/dialogs/link_editor/qt/data_graph.py", line 366, in find_object if obj.contains(event.position()): AttributeError: 'QMouseEvent' object has no attribute 'position'

This error actually happens with 2 datasets too, but since the two datasets are already selected by default it does not block user action.

Details:

Glue: 1.7.1.dev21+geeb78251.d20230307 Python: 3.10.9 PyQt5: PyQt: 5.15.7 - Qt: 5.15.6 PyQt6: Not installed PySide2: Not installed PySide6: Not installed qtpy: 2.3.0 setuptools: 67.5.1 echo: 0.8.0 numpy: 1.24.2 matplotlib: 3.7.1 pandas: 1.5.3 astropy: 5.2.1 dill: 0.3.6 h5py: 3.8.0 xlrd: 2.0.1 openpyxl: 3.1.1 mpl-scatter-density: 0.7 casa-formats-io: 0.2.1 glue-core: 1.7.1.dev21+geeb78251.d20230307 glue-vispy-viewers: 1.0.7 IPython: 8.11.0 qtconsole: 5.4.0 ipykernel: 6.21.2 traitlets: 5.9.0 pygments: 2.14.0 pyzmq: 25.0.0 scipy: 1.10.1 scikit-image: 0.19.3 PyAVM: Not installed spectral-cube: 0.6.0 astrodendro: Not installed mock: Not installed pytest: 7.2.2 plotly: Not installed

Additional context We don't test this interaction (using a mock instead), so I'm not sure the best way to add a regression test.

jfoster17 commented 1 year ago

Oh, I realize this is possibly an Apple Silicon/ARM64 problem. In another thread @dhomeier said:

But on Ventura/arm64 as I mentioned elsewhere, PyQt5 seems not really well supported

astrofrog commented 1 year ago

Oops, we should change the use of position() to use a try...except to support both the old and the new API. Would you have time to do a pull request for this? (I am off work until Monday)