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

Fix bug with qtwebengine being optional #23703

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

larsoner
Copy link
Contributor

@larsoner larsoner commented Feb 10, 2025

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

This is mostly an issue-as-PR, it sounds like #22196 removed the dependency on QtWebEngine but then it came back in #22387. So the goals of this PR are:

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:

Eric Larson

@larsoner larsoner changed the title BUG: Fix bug with qtwebengine Fix bug with qtwebengine being optional Feb 10, 2025
@larsoner
Copy link
Contributor Author

Okay test failing successfully!

...
    File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/spyder/plugins/application/plugin.py", line 29, in <module>
      from spyder.plugins.application.container import (
    File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/spyder/plugins/application/container.py", line 30, in <module>
      from spyder.plugins.application.widgets import AboutDialog, InAppAppealStatus
    File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/spyder/plugins/application/widgets/__init__.py", line 11, in <module>
      from .status import InAppAppealStatus  # noqa
    File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/spyder/plugins/application/widgets/status.py", line 26, in <module>
      from spyder.widgets.browser import WebView
    File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/spyder/widgets/browser.py", line 18, in <module>
      from qtpy.QtWebEngineWidgets import (WEBENGINE, QWebEnginePage,
    File "/home/runner/micromamba/envs/test/lib/python3.8/site-packages/qtpy/QtWebEngineWidgets.py", line 36, in <module>
      raise QtModuleNotInstalledError(
  qtpy.QtModuleNotInstalledError: The QtWebEngineWidgets module was not found. It must be installed separately as PyQtWebEngine.
  Spyder failed with error code 1 (should be 124 for timeout)

I'll see if I can figure out the correct fix next

@larsoner
Copy link
Contributor Author

Okay this seems to be working!

To go the extra mile, WDYT about adding a test-linux run that is TEST_TYPE=noweb or similar? I could add that to the CIs and ensure all tests pass, skipping the ones that need to be skip, nesting imports, etc. It's less hacky and more complete than what I have currently, which uninstalls pyqtwebengine then just spins up the GUI and ensures it doesn't error out for 10s.

@ccordoba12
Copy link
Member

ccordoba12 commented Feb 12, 2025

Okay this seems to be working!

Thanks for your work on this @larsoner. Unfortunately, I'm afraid it's not the right solution because we also have a command line option called --no-web-widgets, which prevents using them even if they are installed. So, the WEBENGINE constant you added to qthelpers is not enough to cover that.

Instead, I propose to move the in-app appeal message to its own plugin and declare in it the constant REQUIRE_WEB_WIDGETS = True. That way it'd be disabled automatically at startup (see PR #22196 for its usage).

The message needs to be in its own plugin because we can't disable the Application one (it has a lot of core stuff). And since this plugin wouldn't provide any functionality to be used by other plugins, it needs to be private (i.e., it needs to be called _appealmessage).

You can take a look at our tutorial for plugin development to better understand how they work:

https://spyder-ide.github.io/spyder-api-docs/plugin_development.html

Also, the pythonpath plugin is similar to what I have in mind for this simple plugin (i.e. one with a single menu entry and no dockable pane).

To go the extra mile, WDYT about adding a test-linux run that is TEST_TYPE=noweb or similar?

Sure, I think that's a good idea.

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