Skip to content

PR: Raise error when no bindings are found at __init__#379

Merged
dalthviz merged 2 commits intospyder-ide:masterfrom
dalthviz:fixes_issue_367
Nov 7, 2022
Merged

PR: Raise error when no bindings are found at __init__#379
dalthviz merged 2 commits intospyder-ide:masterfrom
dalthviz:fixes_issue_367

Conversation

@dalthviz
Copy link
Copy Markdown
Member

Following suggestion at #367 (comment) by @tgwoodcock

Fixes #367

@dalthviz dalthviz added this to the v2.3.0 milestone Oct 20, 2022
@dalthviz dalthviz self-assigned this Oct 20, 2022
Copy link
Copy Markdown
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

I have a small doubt about this (mentioned below), otherwise looks good to me.

@dalthviz dalthviz modified the milestones: v2.4.0, v2.3.0 Oct 31, 2022
Copy link
Copy Markdown
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Great work, thanks @dalthviz!

@dalthviz
Copy link
Copy Markdown
Member Author

dalthviz commented Nov 6, 2022

Note: This is missing and update to take into account the modules added at PR #382

Co-authored-by: tgwoodcock <tgwoodcock@users.noreply.github.com>
@dalthviz
Copy link
Copy Markdown
Member Author

dalthviz commented Nov 7, 2022

Merging this to proceed with the release

@dalthviz dalthviz merged commit 3f2ddd5 into spyder-ide:master Nov 7, 2022
@CAM-Gerlach
Copy link
Copy Markdown
Member

CAM-Gerlach commented Dec 17, 2022

Sorry I didn't get the chance to review this at the time.

The correct fix (using raise QtBindingsNotFoundError from None) we discussed on the issue that solves the two problems I pointed out with the initial naive implementation was not implemented, making the error/traceback message and behavior more appropriate and less confusing, as it is currently treated and printed as an unexpected error during error handling rather than a intentional wrapper error (with raise from), and furthermore prints the error message and traceback from importing pyside6, making users think it was incorrectly trying to use that binding instead of the one they selected.

Here's how the error output looks now:

>>> import qtpy.QtCore
Traceback (most recent call last):
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\qtpy\qtpy\__init__.py", line 252, in <module>
    from PySide6 import __version__ as PYSIDE_VERSION  # analysis:ignore
ModuleNotFoundError: No module named 'PySide6'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\qtpy\qtpy\__init__.py", line 259, in <module>
    raise QtBindingsNotFoundError()
qtpy.QtBindingsNotFoundError: No Qt bindings could be found

and here's how it should look with the correct raise QtBindingsNotFoundError from None:

>>> import qtpy.QtCore
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\C. A. M. Gerlach\Documents\dev\SpyderDev\qtpy\qtpy\__init__.py", line 259, in <module>
    raise QtBindingsNotFoundError from None
qtpy.QtBindingsNotFoundError: No Qt bindings could be found

@CAM-Gerlach
Copy link
Copy Markdown
Member

I've opened issue #390 to document this, and PR #391 to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Import behaviour if no Qt bindings are installed

3 participants