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

Always use PyQT/PySide6 for GitHub CI #23597

Closed
wants to merge 1 commit into from
Closed

Conversation

oscargus
Copy link
Contributor

@oscargus oscargus commented Aug 10, 2022

PR Summary

#23584 (comment)

Also, inkscape and the extra test dependencies are added to OSX so the number of skipped tests is drastically decreased.

ffmpeg can be installed, but is rather time consuming (lots of additional dependencies).

dvipng cannot be installed via brew (but basictex can and GhostScript is already installed).

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

@@ -70,6 +72,8 @@ def test_bold_font_output():
ax.set_title('bold-title', fontweight='bold')


@pytest.mark.skipif(sys.platform == 'darwin',
reason="Fails on darwin")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is related to the svg to png conversion. The resulting svg file is identical to a local test, but the png is slightly different.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the diff:
image

Copy link
Member

Choose a reason for hiding this comment

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

I think this will be fixed by one of the last commits in #23559 which adds quotes to the font name in the expected svg.

I think they were always mal-formed (missing quotes on the family name), but inkscape (probably actually librsvg) is getting crankier about it. At some point we fixed the bug of not putting in quotes so the rest image is correct and the expected image is wrong 🤦🏻 .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this will be fixed by one of the last commits in #23559 which adds quotes to the font name in the expected svg.

Seems like that didn't help. Should I simply go back to skipping (or possible better xfail-ing if that can be done on a platform-specific basis)?

Copy link
Member

Choose a reason for hiding this comment

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

xfail for now and I'll look into if I can reproduce this on a local mac.

@@ -114,6 +114,7 @@ def interrupter():
print('SUCCESS', flush=True)


@pytest.mark.skipif(sys.platform == 'darwin', reason="Does not work on darwin")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly the wrong signal is sent on OSX?

@@ -114,6 +114,7 @@ def interrupter():
print('SUCCESS', flush=True)


@pytest.mark.xfail(sys.platform == "darwin", reason='Fails on OSX')
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should fail on darwin? It may on CI, but I think we should put that somewhere else if possible... Especially because this will fail me locally.

Running on darwin I get:

lib/matplotlib/tests/test_backend_qt.py::test_sigint[show-kwargs0] PASSED            [  5%]
lib/matplotlib/tests/test_backend_qt.py::test_sigint[pause-kwargs1] PASSED           [  7%]

Copy link
Contributor

Choose a reason for hiding this comment

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

I have pyqt5, version 5.15.4 installed in this env if that is any help.

Copy link
Member

Choose a reason for hiding this comment

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

You can check for 'CI' in os.environ if you think it's going to be CI-specific.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, I thought I saw these locally as well. I'll try to check tomorrow when I get home.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it started failing with qt6. The current CI uses qt5 and there it passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now it XPASS:es, but it is not clear which qt-version is used.

XPASS lib/matplotlib/tests/test_backend_qt.py::test_sigint[pause-kwargs1] Fails on OSX
XPASS lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[{'MPLBACKEND': 'qtagg', 'QT_API': 'PySide2'}] 
XPASS lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[{'MPLBACKEND': 'qtcairo', 'QT_API': 'PyQt5'}] 
XPASS lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[{'MPLBACKEND': 'qtcairo', 'QT_API': 'PyQt6'}] 
XPASS lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[{'MPLBACKEND': 'qtcairo', 'QT_API': 'PySide6'}] 
XPASS lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[{'MPLBACKEND': 'qtcairo', 'QT_API': 'PySide2'}] 

I'll remove that line in the web interface and see what happens (so please squash merge...).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There seems to be some xfail that can be removed in test_interactive_thread_safety as well, but I cannot really figure out which...

@oscargus
Copy link
Contributor Author

So it seems to be possible to build wxPython on OSX, but quite time consuming (about an hour).

I guess it really doesn't make sense to even try to install it (still a 73 MB download) for the OSX tests. Will sort it out properly from a computer with a recent git (which this one isn't).

@QuLogic
Copy link
Member

QuLogic commented Aug 24, 2022

Why are we building wxPython now; there should be wheels?

@oscargus
Copy link
Contributor Author

There are no wheels for OSX. Here, I was basically just trying it out, but bad idea and better to skip even trying to install wx on OSX.

(Unless we want to provide our own OSX wheel, but I have no idea how to do that. Should open an issue at them, asking for OSX wheels though since it seems to, at least, build without problems.)

@tacaswell tacaswell modified the milestones: v3.6.0, v3.7.0 Aug 26, 2022
@tacaswell
Copy link
Member

I moved this to 3.7 so we do not backport it. Lets keep going with the current CI on the bug-fix branch (if anything we need to freeze the packages more aggressively).

@oscargus
Copy link
Contributor Author

It seems like despite it looking to be wheels for wxPython on OSX, the following happens. With:

  # But wheels for OSX
  if [[ "macOS" == 'macOS' ]]; then
    python -mpip install --upgrade wxPython &&
      python -c 'import wx' &&
      echo 'wxPython is available' ||
      echo 'wxPython is not available'
  fi

so plain "pip install wxPython"

Collecting wxPython
  Downloading wxPython-4.2.0.tar.gz (71.0 MB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 71.0/71.0 MB 21.7 MB/s eta 0:00:00
  Preparing metadata (setup.py): started
  Preparing metadata (setup.py): finished with status 'error'
  error: subprocess-exited-with-error
  
  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [8 lines of output]
      Traceback (most recent call last):
        File "<string>", line 2, in <module>
        File "<pip-setuptools-caller>", line 34, in <module>
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-install-h25qkvuc/wxpython_a713a81ec5204ad6bcbe911a560b6c3e/setup.py", line 27, in <module>
          from buildtools.config import Config, msg, opj, runcmd, canGetSOName, getSOName
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-install-h25qkvuc/wxpython_a713a81ec5204ad6bcbe911a560b6c3e/buildtools/config.py", line 30, in <module>
          from attrdict import AttrDict
      ModuleNotFoundError: No module named 'attrdict'
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.
wxPython is not available

@oscargus
Copy link
Contributor Author

I do not fully get why wxPython-4.2.0-cp38-cp38-macosx_11_0_universal2.whl is not deemed appropriate for 11.6.8 and Python 3.8.13 though.

@oscargus oscargus force-pushed the pyqt6 branch 3 times, most recently from 1472a35 to 81bc7cf Compare August 28, 2022 11:48
@oscargus
Copy link
Contributor Author

The latest version has GTK3 installed for OSX, but gives the following failing tests:

FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolbar2-{'MPLBACKEND': 'gtk3cairo'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolmanager-{'MPLBACKEND': 'gtk3cairo'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolbar2-{'MPLBACKEND': 'gtk3agg'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_thread_safety[{'MPLBACKEND': 'gtk3agg'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_interactive_backend[toolmanager-{'MPLBACKEND': 'gtk3agg'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_figure_leak_20490[time_mem0-{'MPLBACKEND': 'gtk3cairo'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_figure_leak_20490[time_mem1-{'MPLBACKEND': 'gtk3agg'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_figure_leak_20490[time_mem0-{'MPLBACKEND': 'gtk3agg'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_blitting_events[{'MPLBACKEND': 'gtk3agg'}]
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_figure_leak_20490[time_mem1-{'MPLBACKEND': 'gtk3cairo'}]

Not much info in the transcript.

@tacaswell
Copy link
Member

Moved to 3.8, but if this is passing we can obviously merge it.

@QuLogic
Copy link
Member

QuLogic commented Dec 17, 2022

Needs a rebase, as the logs are gone, though I may have looked before and found nothing else to add.

@QuLogic
Copy link
Member

QuLogic commented Dec 31, 2022

It looks like there's a Traceback in stderr, but it's too long to see. Might have to run with -vvv to get pytest to print the whole thing?

@jklymak
Copy link
Member

jklymak commented Jan 30, 2023

@oscargus are you going to push this through?

@jklymak jklymak marked this pull request as draft January 30, 2023 23:16
@ksunden ksunden modified the milestones: v3.8.0, v3.9.0 Aug 8, 2023
@QuLogic
Copy link
Member

QuLogic commented Apr 3, 2024

I believe this was effectively replaced by #27723.

@QuLogic QuLogic closed this Apr 3, 2024
@oscargus oscargus deleted the pyqt6 branch April 3, 2024 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants