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

PR: Handle QtCore.SignalInstance/pyqtBoundSignal #214

Merged
merged 2 commits into from
Jun 19, 2020

Conversation

altendky
Copy link
Contributor

No description provided.

@goanpeca
Copy link
Member

Hi @altendky thanks for the contribution.

What is this exactly addressing 🙃 ?

@altendky
Copy link
Contributor Author

I was trying to type hint a function that takes a signal as a parameter. The object you get when you a_qobject.some_signal isn't a QtCore.Signal but rather a QtCore.SignalInstance or QtCore.pyqtBoundSignal depending.

I'm not actually running mypy on this project yet but I think I'll need to do something over in the stubs as well based on PyCharm being a bit confused as is. (or maybe I just haven't installed the third-party stubs yet and that'll help out).

@goanpeca
Copy link
Member

Ok understood. Anything that we could do on our side with MyPy to help with this?

@altendky
Copy link
Contributor Author

I really don't know. I've only touched mypy a little bit at this point. If I find something that seems like it would be best fixed here, I'll let you know. :] For now, just having a single way to refer to the 'signal instance' class would address my immediate need. (well, I worked around it with a quick if in my own code so I'm not being held up).

@goanpeca
Copy link
Member

@altendky ok, so far this looks ok, will merge. A new release will be available in a couple of weeks while I do some cleanup of other PRs. Thanks again!

@goanpeca
Copy link
Member

We have some simple tests that check that things are imported. Could you add one?

@altendky
Copy link
Contributor Author

Yeah, I saw that but didn't see a lot so I skipped them. Should be an easy add. Maybe in a few hours here.

Copy link
Member

@goanpeca goanpeca left a comment

Choose a reason for hiding this comment

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

Thanks, that test now clarifies everything :-)

Merging!

@goanpeca goanpeca merged commit 673fb93 into spyder-ide:master Jun 19, 2020
@ccordoba12 ccordoba12 changed the title Handle QtCore.SignalInstance/pyqtBoundSignal PR: Handle QtCore.SignalInstance/pyqtBoundSignal Jun 19, 2020
@altendky altendky deleted the patch-1 branch June 24, 2020 13:09
@ccordoba12 ccordoba12 added this to the v1.10.0 milestone Aug 10, 2021
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