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

Add more support for qt signals in futures #93

Closed
wants to merge 9 commits into from

Conversation

Wesmania
Copy link

This set of commits adds support for awaiting on PyQt signals, as well as emitting a signal once a future / task is done. While it's a non-standard addition, I believe it's very useful for any code utilizing Qt - with that, interaction with regular signalling Qt code becomes much more convenient. Now we can await on dialogs or signals in user code, and a future can be connected with Qt style code with its 'done' signal.

Fixes #90.

Signed-off-by: Igor Kotrasinski <[email protected]>

import quamash

import pytest


from PyQt5.QtCore import QObject, pyqtSignal
Copy link
Owner

Choose a reason for hiding this comment

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

can't have a hard dep on PyQt5. I might drop support for Qt4 soon though (for simplicity).

Copy link
Author

Choose a reason for hiding this comment

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

Oh, sorry for that! I must've missed that when rebasing. I'll fix it right away.

@Wesmania
Copy link
Author

I did not document any of the features yet - I'll do that tomorrow morning.

@harvimt
Copy link
Owner

harvimt commented Jun 23, 2018

documentation would be really helpful.

In particular, can you explain how this differs from, this

import functools
from asyncio import Future

# myobject  is an instance of QObject or a subclass of QObject & myobject.mysignal is a signal
future = asyncio.Future()
myobject.mysignal.connect(functools.partial(future.set_result))
await future

@harvimt
Copy link
Owner

harvimt commented Jun 23, 2018

(I'd also really prefer if the tests pass in CI before merging, but the tests are quite finicky especially the appveryor tests)

@Wesmania
Copy link
Author

Wesmania commented Jun 23, 2018

It doesn't actually differ much, beyond being able to await on multiple signals at a time (which can be done just by connecting to multiple signals), queueing results and keeping references to lambdas (since PyQt for example doesn't allow us to disconnect them). The example you've provided is probably a better solution than adding an extra method to the event loop. I don't know if 'done' signals for futures can be extracted that way as well.

I'll look at the tests.

@harvimt
Copy link
Owner

harvimt commented Jun 23, 2018

I mean... you can await multiple futures with asyncio.wait, but I guess it's kind of awkward.

@Wesmania
Copy link
Author

Since awaiting of signals is implementable without touching the event loop, I can reduce the PR just to 'done' signals for futures, if you'd like. Of course with some manual wrapping and 'add_done_callback' this can be shoved outside as well.

Copy link

@norraxx norraxx left a comment

Choose a reason for hiding this comment

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

docstring for file, then docstring for classes and methods are fine ;-)

quamash/_qt.py Outdated
class QtModule:
def __init__(self):
try:
self.name = os.environ['QUAMASH_QTIMPL']
Copy link

Choose a reason for hiding this comment

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

self.name = os.environ.get('QUAMASH_QTIMPL', None) - doesn't generate stacktrace
if self.name:
....

Igor Kotrasinski added 8 commits June 24, 2018 10:21
Signed-off-by: Igor Kotrasinski <[email protected]>
Signed-off-by: Igor Kotrasinski <[email protected]>
Everything the class allocates or references will be cleaned up once it
goes out of scope anyway, since signals aren't keeping a Python object
alive.

Signed-off-by: Igor Kotrasinski <[email protected]>
Regular code may have use cases where a future assigned to an attribute
is cancelled and replaced by another. Since PyQt signals don't like
lambdas, this will let us grab and handle the replaced future.

Signed-off-by: Igor Kotrasinski <[email protected]>
Signed-off-by: Igor Kotrasinski <[email protected]>
@Wesmania
Copy link
Author

So I got more familiar / comfortable with Python's asyncio and figured out this has no business being in the event loop, indeed. Sorry for taking your time, at least it was a learning experience for me :)

@Wesmania Wesmania closed this Aug 24, 2018
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.

3 participants