Skip to content

WidgetGroup: avoid lambda by using self.sender()#2694

Merged
j9ac9k merged 2 commits intopyqtgraph:masterfrom
pijyoi:wg-no-lambda
May 11, 2023
Merged

WidgetGroup: avoid lambda by using self.sender()#2694
j9ac9k merged 2 commits intopyqtgraph:masterfrom
pijyoi:wg-no-lambda

Conversation

@pijyoi
Copy link
Copy Markdown
Contributor

@pijyoi pijyoi commented Apr 15, 2023

workaround #2672 by avoiding the mechanism triggering the leak in PySide6 6.5.0

  1. WidgetGroup was creating lambda closures with references to both self and widget.
    • This is a bad practice and should be avoided.
    • However, prior to PySide6 6.5.0, this didn't cause a leak. (although it would not have been surprising if it had!)
  2. PlotItem leak since PySide6 6.5.0 #2672 (comment) demonstrates that the leak occurs even with a lambda that has no closures.
    • i.e. various techniques using weakrefs would still have leaked
    • further testing shows that using custom def functions (with no closures) also leaks
  3. The whole reason for the closure was to pass in an argument that would allow identification of the widget emitting the signal. This identification can be done with QObject::sender (https://doc.qt.io/qt-6/qobject.html#sender) and avoids the closure.

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Apr 15, 2023

CI is crashing like for #2671, but now also includes Conda/Ubuntu/PySide2.
Removing that particular test does make it pass.

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Apr 15, 2023

#2606 bumped conda python from 3.9 to 3.11, skipping 3.10.
By dropping to 3.10, the conda CI passes.

According to a comment in https://bugreports.qt.io/browse/PYSIDE-1864, Python > 3.10 will not be supported by Qt.
Conda builds it for Python 3.11 by applying an unofficial patch https://github.com/conda-forge/pyside2-feedstock/blob/main/recipe/patches/python3.11.patch

@pijyoi
Copy link
Copy Markdown
Contributor Author

pijyoi commented Apr 18, 2023

#1598 mentions pyqtgraph.WidgetGroup.WidgetGroup.mkChangeCallback as being unsolvable at the time.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 11, 2023

Thanks @pijyoi ...appreciate the touch-up on the CI, testing python 3.11 with pyside2 does feel silly.

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.

2 participants