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

Add CI envs for PyQt 6.5-6.7, PySide 6.5-6.7 & Python 3.12; fix Readthedocs and matplotlib 3.9, Qt6 failures #17

Merged
merged 6 commits into from
Aug 16, 2024

Conversation

dhomeier
Copy link
Contributor

@dhomeier dhomeier commented Mar 2, 2024

Description

WIP

@dhomeier dhomeier force-pushed the qt65-6-py312-ci branch 2 times, most recently from 07b7e8b to 1487a9a Compare March 2, 2024 13:46
@dhomeier dhomeier changed the title Add CI envs for PyQt 6.5, 6.6 & Python 3.12 Add CI envs for PyQt 6.5, 6.6 & Python 3.12; fix Readthedocs Mar 2, 2024
@dhomeier
Copy link
Contributor Author

dhomeier commented Mar 2, 2024

Same #16 failures on ScatterRegionLayerArtist.enabled.
But there is a new one with Python 3.12 (for PyQt6?) hanging apparently in glue_qt/viewers/matplotlib/tests/test_data_viewer.py:test_add_data_with_subset
until CI timeout; I could not reproduce this latter issue locally.

@dhomeier dhomeier marked this pull request as ready for review March 2, 2024 14:22
@dhomeier
Copy link
Contributor Author

dhomeier commented Mar 4, 2024

@astrofrog it seems I cannot cancel workflows here (might not have the right permissions?), so would rather not experiment more in the CI at this point (haven't tested any py312+PyQT 5 combinations)

@dhomeier
Copy link
Contributor Author

@astrofrog I think #16 may be closed now through glue-viz/glue#2479, but there are 2 new errors probably introduced with glue-viz/glue#2480, e.g. in _update_attribute_display_unit_choices:
https://github.com/glue-viz/glue-qt/actions/runs/8758215189/job/24038566338?pr=17#step:10:687
and on calling Unit(None) from a session file
https://github.com/glue-viz/glue-qt/actions/runs/8758215189/job/24038566338?pr=17#step:10:430

Also all py312 jobs still hanging in test_data_viewer.

@astrofrog
Copy link
Member

@dhomeier - could you rebase to pick up the fixes in #21? (doesn't fix everything but at least a few jobs)

@dhomeier
Copy link
Contributor Author

dhomeier commented Jul 15, 2024

@dhomeier - could you rebase to pick up the fixes in #21? (doesn't fix everything but at least a few jobs)

I haven't followed the CI updates closely – has the default macOS-latest runner changed to arm64 since? Don't recall that many segfaults from before anyway, but the newer PyQt6 versions seem to be more stable. Same 2-4 failures though, where test_table_widget and test_table_preserve_model_after_selection are new as of PyQt 6.6.

@dhomeier
Copy link
Contributor Author

dhomeier commented Jul 16, 2024

Well ,the first common failure rather seems to be a matplotlib thing, but I am struggling to see how this could ever have worked even with mpl 3.5
https://github.com/glue-viz/glue-qt/actions/runs/9940878742/job/27458723480#step:10:283
– set_xy interface has not changed there, would this mean it has not been using a Rectangle in the older setups??

Actually the second failure assert err.strip() == "" is linked to the same
ValueError: too many values to unpack (expected 2).

@dhomeier
Copy link
Contributor Author

I think it might indeed be axvspan changed from using Polygon to Rectangle in matplotlib/matplotlib#26788!

@dhomeier
Copy link
Contributor Author

dhomeier commented Jul 16, 2024

Darn, forgot the macos-qt65 job is hanging in

Matplotlib is building the font cache; this may take a moment.

until timeout!

I can "fix" the wrong PyQt6.QtCore.QModelIndex.row() for higher PyQT6 version by inserting an extra press_key(Qt.Key_Down), but it does not even seem to be tied to a specific version – on my Mac needing it from 6.2 onwards, but in the Windows CI it still seems to sometimes pass with 6.5, sometimes not...

@dhomeier dhomeier changed the title Add CI envs for PyQt 6.5, 6.6 & Python 3.12; fix Readthedocs Add CI envs for PyQt 6.5-6.7, PySide 6.5-6.7 & Python 3.12; fix Readthedocs and matplotlib 3.9, Qt6 failures Jul 16, 2024
@dhomeier
Copy link
Contributor Author

@astrofrog the two remaining failures are now worked around in a rather hackish manner, that's as much as I could come up with (I tried with a wait in process_events(), but to no avail).
The macOS 14-PyQt5 test could perhaps be saved by skipping TestGlueDataDialog and TestLinkEditor tests, but that would require a bunch of system-version and architecture conditionals.
Re-enabled a test that was probably mistakenly skipped on Qt6; perhaps requires_pyqt_gt_59_or_pyside2 should be updated upstream, or maybe the Qt test helpers should actually be moved into this repo?
PySide6 is now at 6.7.2; haven't looked closely at those failures.

@astrofrog
Copy link
Member

@dhomeier - regarding the Qt test helpers, yes they should be moved to this repo, would you be able to do that here?

I think the hackish fixes will have to do for now!

@astrofrog
Copy link
Member

Can you rebase to take into account #12, as this might fix some of the issues?

@dhomeier dhomeier force-pushed the qt65-6-py312-ci branch 2 times, most recently from c4a6c1a to 0b6458f Compare July 19, 2024 13:30
@dhomeier dhomeier force-pushed the qt65-6-py312-ci branch 3 times, most recently from 65abe64 to cb313a3 Compare July 19, 2024 16:01
@dhomeier
Copy link
Contributor Author

@astrofrog adding a wait to process_events() seemed to resolve one test, but I am leaving the hacks for the remaining ones.

The QT_anything helpers could be removed from glue core at some point. Maybe make_marker should also be completely defined here, so the glue-core version would not need the extra PyQt5/6 foo?

@astrofrog astrofrog merged commit de4f45c into glue-viz:main Aug 16, 2024
36 of 45 checks passed
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.

2 participants