How about adding the type hint

[longqzh]

Hi,
I noticed that there is no type hint in PyQtGraph.
The type hint give us the huge help to avoid typo and improve the development speed in IDE
how about adding the type hint?
If this suggestion is accepted, I will send a PR about it :)

Any suggestion is appreciated!

[j9ac9k]

Hi @longqzh

Type annotations would be a great thing to add to the library. I believe there are a few places in the code-base that use them already.

While I think type-annotating the whole library would be way too large of an effort, but starting to annotate some of the higher use methods/functions would certainly be a good start.

I would ask that any PR that includes a substantial amount of type annotations includes a mechanism to verify the type-annotations are correct. This can be tricky to do, but there is a blog post from the urllib3 devs on how they incrementally added type annotations to their library and I suspect something like that would be the way to go in our case:

https://sethmlarson.dev/tests-arent-enough-case-study-after-adding-types-to-urllib3#how-we-developed-and-iterated-on-type-hints

[longqzh]

@j9ac9k , thanks very much for sharing the guide. I'll send the PR to verity the type annotations before adding type hint :)

[ksunden]

I have machinery from matplotlib for e.g. testing stubs against implementation (it is mostly mypy's stubtest, but with CI configs etc.) if we choose to go the route of stub files rather than inline type hints.

In general, I prefer inline, but stub files do help avoid risks of changing runtime behavior unintentionally, so that is why we went that route on mpl.

[j9ac9k]

Inline annotations would be preferred due to comparability with pylance/pyright.

…

On Thu, Oct 12, 2023 at 18:12 Kyle Sunden ***@***.***> wrote: I have machinery from matplotlib for e.g. testing stubs against implementation (it is mostly mypy's stubtest, but with CI configs etc.) if we choose to go the route of stub files rather than inline type hints. In general, I prefer inline, but stub files do help avoid risks of changing runtime behavior unintentionally, so that is why we went that route on mpl. — Reply to this email directly, view it on GitHub <#2833 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE5Z7WYTBLU5TMYK3ULVXLX7CIOPANCNFSM6AAAAAA5OD3W2M> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ksunden]

I mean, pyright works with stubs, but yeah, inline gets you more benefit (but also more to fix as implementing).

[j9ac9k]

Does it? We have stubs for the Qt.py module (pyqtgraph/Qt/init.pyi) but I haven't seen vscode incorporate it, only PyCharm seems to recognize them.

[ksunden]

I mean, you need to add the py.typed signal file, (and if incomplete may get complaints from people running eg mypy --strict because of untyped things)

[j9ac9k]

Hmmm; I wouldn’t be opposed to having the signal file and adding it to .gitignore so it doesn’t get committed if pylance/pyright can use those stubs and we get linting inside the editor. Could even add the file as part of a CI check. I didn’t think mypy supported verifying the codebase types are correct against stubs of the same library tho.

…

On Fri, Oct 13, 2023 at 09:18 Kyle Sunden ***@***.***> wrote: I mean, you need to add the py.typed signal file, (and if incomplete may get complaints from people running eg mypy --strict because of untyped things) — Reply to this email directly, view it on GitHub <#2833 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE5Z7XU3HTIWVB4Q6G3K2TX7FSVJANCNFSM6AAAAAA5OD3W2M> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ksunden]

It is a separate entry point distributed with the mypy package

https://mypy.readthedocs.io/en/stable/stubtest.html

We have ci set up to even add annotations on the indicated lines and such

It's not perfect, but catches missing/extra stub methods/classes, and inconsistent default param values. Can miss class attributes though, because those are added at instantiating

[longqzh]

@j9ac9k , @ksunden
I send PR to add mypy setting into CI, please review it :)

[spchamp]

Hello! I think type hints can be really helpful for understanding the structure of a complex codebase, as well as for developing new code.

Of course, with editor tools such as PyCharm, VS Code and even Elpy (for Emacs) as well as shell tools such as IPython and qtconsole, type hints can help with code completion on input. Towards understanding the syntax and structure of a framework, type hints can also help with finding the source location for a definition of a type or a function.

I think it's not too daunting to add type hints to class and function definitions in pyqtgraph. There might be a number of possible approaches, however? pytgraph is already using .pyi files -- there's some documentation of this format, via Stack Overflow -- such that the file <module>.pyi can provide additional type information external to a <module>.py. Type hints can also be added inline, under an if TYPE_CHECKING clause [stdlib] or directly in a function signature or class definition.

Of course, there's also the variety of Qt frameworks - any single one of which may be in use in an application. For this concern, I wonder if there may be any thoughts about integrating with support for qtpy? Perhaps it could be of interest for integrating with applications such as qtconslole (jupyter-qtconsole) which is using qtpy for Qt support. Albeit, Jupyter Kernel integration could introduce a whole other range of complex concerns to a project, but there's the GUI side though?

If it might contribute to the codebase to support qtpy in pyqtgraph, beside the support for integrating with external applications, qtpy might also help to provide type hints for the Qt framework. Editor tools might even be able to detect - to some extent - what Qt framework is in use, from what's installed in a virtual environment.

For what it's worth, I've managed to hack out a sort of a patch that pulls in qtpy without overriding the Qt library shims in pyqtgraph. It uses the pyqtgraph Qt support first, then some additional patching to ensure qtpy should load the right Qt framework from what was selected in pyqtgraph. It then pulls in the Qt library definitions and type hints, via qtpy, under TYPE_CHECKING clauses in each pyqtgraph/Qt/Qt*/__init__.py package definition. It seems to work out in VS Code and PyCharm, after testing this with both IDEs on an openSuSE machine.

During the tests here, candidly I've seen some floating-point weirdness at a certain QPointF comparison in one of the tests, using pyqt5 and pyqt6 in a certain development environment, otherwise the patched thing, it passes the tests tho, tested with tox.

If it could sound like a plausible idea to add support for qtpy, I'll try to clean up the patch and publish it to a patch branch. Outside of the single QPointF thing the tests passed in the patched code, testing with Python 3.12 (edge??) and Python 3.11 with both of each of the Qt 5 and Qt 6 frameworks on an openSuSE LEAP 15.5 machine. Of course, it would probably need further testing tho

[j9ac9k]

A few comments, but first I want to point out that I'm no wizard to type-annotations, there may be better ways to roll out support than what I've come up with, or some of my opinions/concerns may be now outdated. In those cases, I would welcome being corrected.

Broadly speaking, we have opted not to use .pyi files, while they are useful for users of pyqtgraph, it's not easy to actually verify the library itself respects the type annotations specified in the stubs. Basically, the stubs are intended for consumption by pyqtgraph users, not pyqtgraph maintainers. We do have the one stub file, but it was added since enough people working on the library use pycharm, which was able to make use of it, pylance/pyright last I checked effectively ignored the stub file which is unfortunate.

It seems like the best usage of type annotations, where the library can verify itself, users can also use it, and the documentation can make use of annotations as well is performing inline annotations, so that's where I've landed ourselves.

Regarding the use of qtpy. QtPy does indeed provide type annotations that are useful, so that would be one advantage of incorporating it. I'm open to the idea of ditching pyqtgrpah/Qt module and dropping in qtpy in its place. That said, pyqtgraph was able to support Qt6 well before qtpy (thanks to @pijyoi !) so there would be some reluctance to adopt qtpy as a dependency unless a pyqtgraph maintainer had some influence on being able to ensure qtpy is working w/ new-releases of Qt as they come out (@ksunden you looking for another project? :D ). Specific to the qtconsole, I believe we have an example that shows how to embed the jupyter-qtconsole into pyqtgraph, I see no reason to have the jupyqter-qtconsole be a pyqtgraph dependency (I don't think that's what you were advocating for, just making that clear).

That's awesome you got type-hints to work by being about to patch up pyqtgraph/Qt with qtpy and have that work across all editors, that would be the goal. That would definitely not be a workflow I would want to advocate other developers use for sure.

During the tests here, candidly I've seen some floating-point weirdness at a certain QPointF comparison in one of the tests, using pyqt5 and pyqt6 in a certain development environment, otherwise the patched thing, it passes the tests tho, tested with tox.

I haven't seen this at all, if you can reproduce on any platform, I would love to take a closer look. Also if it's only happening on one platform, Phil at Riverbank might be interested in knowing about it too.

[spchamp]

fwiw I was thinking pyi files could be helpful with the small number of mixin-like classes in pyqtgraph, for instance PlotItem and PlotWidget in the methods that are forwarded to ViewBox.

After beginning to study the source tree while hacking out a few type annotations, I noticed that there are these couple of core classes that are forwarding attrs/methods to some other class, although not expressly applying the other class in the sending class' type hierarchy. With the addition of a pyi file to define a prototype for each of those forwarded methods under the forwarding class, it would prevent that the type-hinted method prototypes would be visible to the runtime code while still visible in the editing environment and any source analysis tools? For instance, there's a gist illustrating the forwarded methods for PlotItem, forwarded to ViewBox. This was defined after the documentation string for PlotItem, with some short study for inferring the types of the args for the forwarded methods.

Of course, whether adding the method prototypes and type annotations in a pyi file or inline under TYPE_CHECKING clauses, the type hints might be duplicated for each forwarding class. For instance, there are the ViewBox methods on both PlotItem and PlotWidget. Compared to other possible approaches though (mixin classes??) the type hints duplication might be more straightforward?

For type hints and qtpy support, it could even be added as a sort of an optional feature? something that would be applied when qtpy is installed. Using importlib, it's possible to test for this without actually importing any modules.

I'll try to adapt the patch I was hacking on, to make the qtpy integration an optional feature. For the patch at present, the type hints for the Qt modules are each imported under a TYPE_CHECKING clause, simply by way of from qtpy.<m> import * there. It would be trivial to check a qtpty-available-p flag first, under the same type checking clause, at least insofar as would be evaluated within any type checking systems.

The only effective difference in runtime code would be for the added import qtpy handling and some a couple of environment variables set for library parity (a) after importing Qt via pyqtgraph and (b) before importing qtpy. If this integration support could be available in pyqtgraph but as an optional feature, maybe it would not add much more complexity.

---

Albeit departing from the type hints discussion, at the quirks I was seeing under tests: For the couple of floating point things I've been seeing, I wonder if it may be due to the operating system environment? Both of the items might be generally like an assertion failure for QRectF value comparisons, after a state change in the UI.

This first quirk shows up here with any Python 3.9-3.12, Qt 6.4.2, openSUSE LEAP 15.5 amd64 installation (py39-pyside6 test here, seeing similar under other Python versions on this particular virtual machine)

>       assert r_ebi.height() > r_empty_ebi.height()                                                                                                                   
E       assert 4.387322709707185 > 4.387322709707185                               
E        +  where 4.387322709707185 = <built-in method height of QRectF object at 0x7f4164ee6cf0>()                                                                    
E        +    where <built-in method height of QRectF object at 0x7f4164ee6cf0> = PyQt6.QtCore.QRectF(9.83203411518198, 9.806338645146408, 4.335931769636041, 4.3873227
09707185).height                                                                                                                                                       
E        +  and   4.387322709707185 = <built-in method height of QRectF object at 0x7f4165e8e270>()                                                                   
E        +    where <built-in method height of QRectF object at 0x7f4165e8e270> = PyQt6.QtCore.QRectF(9.83203411518198, 9.806338645146408, 4.335931769636041, 4.3873227
09707185).height                                                                                                                                                       
                                                                                                                                                                       
tests/graphicsItems/test_ErrorBarItem.py:34: AssertionError

The second item, it might be due to the outdated Qt 5 build distributed with openSUSE LEAP 15.5? The rest of the tests pass though.

    def assertMapping(vb, r1, r2):
>       assert vb.mapFromView(r1.topLeft()) == r2.topLeft()
E       assert PySide2.QtCore.QPointF(200.000000, 600.000000) == PySide2.QtCore.QPointF(0.000000, 400.000000)
E        +  where PySide2.QtCore.QPointF(200.000000, 600.000000) = <bound method ViewBox.mapFromView of <pyqtgraph.graphicsItems.ViewBox.ViewBox.ViewBox(0x22ab250, pa$
E        +    where <bound method ViewBox.mapFromView of <pyqtgraph.graphicsItems.ViewBox.ViewBox.ViewBox(0x22ab250, parent=0x23f0940, pos...gs=(ItemIsFocusable|ItemC$
E        +    and   PySide2.QtCore.QPointF(0.000000, -5.000000) = <built-in method topLeft of PySide2.QtCore.QRectF object at 0x7f2ae1778140>()
E        +      where <built-in method topLeft of PySide2.QtCore.QRectF object at 0x7f2ae1778140> = PySide2.QtCore.QRectF(0.000000, -5.000000, 10.000000, 20.000000).t$
E        +  and   PySide2.QtCore.QPointF(0.000000, 400.000000) = <built-in method topLeft of PySide2.QtCore.QRectF object at 0x7f2ae17780c0>()
E        +    where <built-in method topLeft of PySide2.QtCore.QRectF object at 0x7f2ae17780c0> = PySide2.QtCore.QRectF(0.000000, 400.000000, 800.000000, -400.000000)$

tests/graphicsItems/ViewBox/test_ViewBox.py:11: AssertionError

If it could be of interest, I'll try to open a separate discussion page for the first item, with details about my testing environment .

Everything else works, the GUI tests also run fine here.

[spchamp]

Published a patch for qtpy integration and type hints via qtpy for feedback/source review.

Tested on openSUSE 15.5 and with a local PyQt6 build on FreeBSD 13.2. Open for feedback about the particulars lol

[spchamp]

For the qtpy import in Qt.init.py, this would be independent to the imports for type hints in submodules. On review, not actually sure about the logic, "Import qtpy if it's installed"

[pijyoi]

Originally, a quick way to get PyQt6 up and running was to alias the scoped enums up one level, as done in #1502. Later, in #1818, this band-aid was removed, and the whole pyqtgraph library was migrated to used scoped enums. As stated in #1818, one of the reasons was that this aliasing was not cost-free in terms of application startup time.

However, qtpy has chosen to copy this enum aliasing band-aid. Thus importing qtpy would incur this startup overhead.
(Note: this is by no means the only reason that pyqtgraph is doing its own compatibility layer)

[j9ac9k]

I do want to say that the qtpy devs are ok with removing this aliasing for the scoped enums now that the full scope enums work across the board. Also they have some benchmarking to indicate that the performance penalty isn't so bad (although I don't remember exact numbers and I'm on mobile now so I can't look it up).