search

decred / tinydecred

Python tools for Decred
ISC License
27 stars 14 forks source link

tinywallet: migrate from PyQt5 to PySide2 #170

Closed teknico closed 1 year ago

teknico commented 4 years ago

This migrates the tinywallet package from PyQt5 to PySide2, while changing its license from ISC to LGPLv3. Needs running poetry install to update the dependencies in the virtual environment.

EDIT: needs running poetry install, not poetry update.

Fixes #166.

teknico commented 4 years ago

Sorry, my bad: you need to run poetry install, not poetry update. Looking into the other problem.

teknico commented 4 years ago

Also, when trying to restore a wallet from seed:

The constructor of the Q.ThreadUtilities class is not being invoked. It looks like PySide2 is breaking multiple inheritance via super, while PyQt5 doesn't do that.

An immediate workaround is to call both superclasses' constructors directly rather than using super, see the 08586ab commit.

A better solution, in my opinion, is avoiding multiple inheritance altogether: its headaches are not worth it, and composition is clearer anyway: the TinyWallet class can have instance of the Q.ThreadUtilities class as an attribute.

The only other instance of multiple inheritance in the TinyWallet codebase is screens.SVGWidget. Its constructor also doesn't use super, but calls the first superclass constructor directly. The second superclass constructor is not called, not sure if that's intentional.

buck54321 commented 4 years ago

I see you still have the LGPLv3 in the tinywallet directory. We do distribute the license with PySide2, but that requirement is satisfied by PyPI, I think.

teknico commented 4 years ago

Lost drop shadows.

Not sure why this is happening. Probably not relevant but this is the only hint I can find in the PySide2 docs:

"Graphics effects are not supported for OpenGL-based widgets, such as QGLWidget , QOpenGLWidget and QQuickWidget ."

buck54321 commented 4 years ago

I spent some time poking around, but I couldn't find a solution either. I guess we just get rid of them. Maybe just replace them with a border. Or we create a QWidget subclass with a custom paintEvent and do them ourselves.

Should open an issue @ Qt, but I probably won't.