Skip to content

use single generic template for all bindings#2226

Merged
j9ac9k merged 1 commit intopyqtgraph:masterfrom
pijyoi:generic_template
Aug 26, 2022
Merged

use single generic template for all bindings#2226
j9ac9k merged 1 commit intopyqtgraph:masterfrom
pijyoi:generic_template

Conversation

@pijyoi
Copy link
Copy Markdown
Contributor

@pijyoi pijyoi commented Mar 15, 2022

Currently, each .ui file has a separate generated .py file for each of the 4 bindings.

This PR demonstrates that the currently supported bindings are sufficiently similar that a single generic "generated" file can be used for all bindings. The generic "generated" file is actually a hand-modified version of the files churned out by PyQt6's pyuic6.

One benefit of doing this would be to make it easier for users who use py2exe, pyinstaller or similar programs. These programs seem to have issues with bundling files imported using the following statement. (#2122, #2179, #1981, #1934, #1811, #1801)

ui_template = importlib.import_module(
    f'.exportDialogTemplate_{QT_LIB.lower()}', package=__package__)

Disclaimer: Not being a user of any of these freezing / bundling programs, I can't say for certain that this PR would make those programs include the needed files.

Note: flowchart, canvas, console modules were left out since they are not much used.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 15, 2022

@pijyoi thanks so much for this. This has been a thorny issue for a while. It's been tough for me to evaluate the downside here as I don't use Qt designer or templates in my own projects, and I really don't know how prevalent their usage is in general.

An issue I can see is that unless we remove the .ui files, ./tools/rebuildUi.py will create new versions of binding-specific template files. We could leave those .ui files in the repo and/or instead add the generated files (the not _generic) ones to .gitignore. Then again, is having those binding specific templates still be in the repo that bad, probably not; but probably not worth keeping.

@pijyoi pijyoi force-pushed the generic_template branch from 68131c0 to 6680bc5 Compare March 16, 2022 22:00
@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Mar 16, 2022

By deleting the non-generic files, it becomes clear from the git history that the generic files are just a rename of the *_pyqt6.py files with a one-line modification.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 16, 2022

The only other thing I can think of that should probably be part of this PR is that rebuildUi.py should be updated to only work on PyQt6 bindings, write out to *_generic.py and change the import statements so they import from pyqtgraph's abstraction layer.

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Mar 16, 2022

The automatic rewrite of the import line would need to handle different depth relative imports (for the library) and absolute imports (for the examples).

It would also be relying on pyuic6 output to remain the same style.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 16, 2022

Forgot about the relative import, that would indeed be trickier (and frankly just add un-needed complexity here). Probably not worth the effort.

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Mar 18, 2022

On a whim, I tried using pyinstaller (for the first time) on one of my scripts in a clean environment with pyqtgraph 0.12.4, PySide6 and numpy. It seemed to just work (* with a little tweak) and I didn't encounter the issues listed before about missing template files and missing icon files.

(*) The colormap files were missing, so I changed my script to not use them.

Digging a bit, it seems that someone has taught pyinstaller about pyqtgraph template files.
https://github.com/pyinstaller/pyinstaller-hooks-contrib/blob/1649c5e412e02f5ebee4c542093064c8ba1c96bb/src/_pyinstaller_hooks_contrib/hooks/stdhooks/hook-pyqtgraph.py

@j9ac9k j9ac9k mentioned this pull request May 10, 2022
Closed
@pijyoi pijyoi force-pushed the generic_template branch from 6680bc5 to 7b7683d Compare June 21, 2022 11:54
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 21, 2022
View reviewed changes
pyqtgraph/graphicsItems/ViewBox/axisCtrlTemplate_generic.py


from PyQt6 import QtCore, QtGui, QtWidgets
from ...Qt import QtCore, QtGui, QtWidgets

Check notice

Code scanning / CodeQL

Unused import

Import of 'QtGui' is not used.
pyqtgraph/GraphicsScene/exportDialogTemplate_generic.py


from PyQt6 import QtCore, QtGui, QtWidgets
from ..Qt import QtCore, QtGui, QtWidgets

Check notice

Code scanning / CodeQL

Unused import

Import of 'QtGui' is not used.
pyqtgraph/graphicsItems/PlotItem/plotConfigTemplate_generic.py


from PyQt6 import QtCore, QtGui, QtWidgets
from ...Qt import QtCore, QtGui, QtWidgets

Check notice

Code scanning / CodeQL

Unused import

Import of 'QtGui' is not used.
@pijyoi pijyoi marked this pull request as ready for review August 24, 2022 23:34
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Aug 26, 2022

Thanks for this @pijyoi

Hopefully, this will make life easier for the folks doing freezing.

@j9ac9k j9ac9k merged commit bfb929f into pyqtgraph:master Aug 26, 2022
@pijyoi pijyoi deleted the generic_template branch August 26, 2022 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pijyoi @j9ac9k