search

spyder-ide / qtawesome

Iconic fonts in PyQt and PySide applications
https://qtawesome.readthedocs.io/en/latest/index.html
MIT License
802 stars 105 forks source link

Import error when importing `PYSIDE6` while using `QtPy` <= 1.x #227

Closed duylp closed 1 year ago

duylp commented 1 year ago

A recent change made qtawesome depend on PYSIDE6, which is not necessary in my opinion.

File ~/python-venv/lib/python3.8/site-packages/qtawesome/iconic_font.py:25
     22 import warnings
     24 # Third party imports
---> 25 from qtpy import PYSIDE2, PYSIDE6
     26 from qtpy.QtCore import (QObject, QPoint, QRect, Qt,
     27                          QSizeF, QRectF, QPointF, QThread)
     28 from qtpy.QtGui import (QColor, QFont, QFontDatabase, QIcon, QIconEngine,
     29                         QPainter, QPixmap, QTransform, QPalette, QRawFont,
     30                         QImage)

ImportError: cannot import name 'PYSIDE6' from 'qtpy'

A simple try/catch would fix this.

dalthviz commented 1 year ago

Hi @duylp thank you for the feedback! Could you update to the latest qtpy and check again? I think we are missing bumping the constraint for qtpy to be at least >=2.0.0 at https://github.com/spyder-ide/qtawesome/blob/master/setup.py#L35

duylp commented 1 year ago

Hi @dalthviz, thanks for solution, that should work. Reading from the changelog, I don't see any features aside from this depend on PYSIDE6. Would it be better to check if PYSIDE6 exists rather than pinning qtpy>=2.0.0?

In our usage, qtpy is pinned to be 1.x.x so we're currently pinning qtawesome.

dalthviz commented 1 year ago

Thanks for the info @duylp ! I see, so maybe you are still using Qt4? Checking maybe we could change the validation using qtpy.PYSIDE2 or qtpy.PYSIDE6 at https://github.com/spyder-ide/qtawesome/blob/1623f2a7dc5b4564b3f3e9d26d5fa1b7e4b59b20/qtawesome/iconic_font.py#L535 to use qtpy.PYSIDE_VERSION so:

    if PYSIDE_VERSION:

What do you think @ccordoba12 ? In that way we should not need to constrain qtpy or add try-catch logic, right?

Also @duylp could you check if the change I'm suggesting works in your setup with qtpy <=1.x ? Thank you!

ccordoba12 commented 1 year ago

What do you think @ccordoba12 ? In that way we should not need to constrain qtpy or add try-catch logic, right?

Yeah, sounds good to me.

duylp commented 1 year ago

Thanks, I've tested it and your solution works for me.