Skip to content

Fix 'import pyqtgraph.canvas' crash#2934

Merged
j9ac9k merged 1 commit intopyqtgraph:masterfrom
nicoddemus:fix-crash-2838
Feb 11, 2024
Merged

Fix 'import pyqtgraph.canvas' crash#2934
j9ac9k merged 1 commit intopyqtgraph:masterfrom
nicoddemus:fix-crash-2838

Conversation

@nicoddemus
Copy link
Copy Markdown
Contributor

@nicoddemus nicoddemus commented Feb 11, 2024

Just importing pyqtgraph.canvas causes the interpreter to crash.

Saving the SINGLETON after the call to QObject.__init__ solves the issue.

Fix #2838

@nicoddemus nicoddemus changed the title Add regression test for #2838 Fix 'import pyqtgraph.canvas' crash Feb 11, 2024
Just importing pyqtgraph.canvas causes the interpreter to crash.

Saving the `SINGLETON` after the call to `QObject.__init__` solves the issue.

Fix pyqtgraph#2838
@nicoddemus nicoddemus marked this pull request as ready for review February 11, 2024 14:29
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Feb 11, 2024

Thanks @nicoddemus

I'm curious, so you use the canvas module?

@j9ac9k j9ac9k merged commit 0d4990c into pyqtgraph:master Feb 11, 2024
@nicoddemus nicoddemus deleted the fix-crash-2838 branch February 11, 2024 17:28
@nicoddemus
Copy link
Copy Markdown
Contributor Author

Hey @j9ac9k!

Not directly, I just hit pyinstaller/pyinstaller#7991 while playing around with pyinstaller, and decided to contribute a fix. 👍

@nicoddemus nicoddemus restored the fix-crash-2838 branch February 12, 2024 11:31
@Jamesong7822
Copy link
Copy Markdown

Hi @j9ac9k, will this be in the next upcoming release? if so, when is the expected release date? Thank you very much

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Feb 13, 2024

Hi @j9ac9k, will this be in the next upcoming release? if so, when is the expected release date? Thank you very much

😅 I'm awful at predicting when I'm going to do a release. I recently moved across the country and have been dealing with everyone in my household getting sick... there are some PRs I would want to merge before the next release, and ideally I would fix a regression I introduced in the SVG exporter... anyway I have no idea when I'm going to do it. No doubt that my other responsibilities have pulled me away from this project... hoping to resume my work before much longer.

@Jamesong7822
Copy link
Copy Markdown

in that case, for anyone facing this issue, consider installing pyqtgraph from source, instead of via pip. It helped for my project using PyQt6 and pyqtgraph when I was freezing it using PyInstaller.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Feb 13, 2024

I generally try and cut releases shortly after breakages are addressed, i didn't realize this issue was impacting so many pyinstaller users.

@nicoddemus
Copy link
Copy Markdown
Contributor Author

nicoddemus commented Mar 6, 2024

@j9ac9k anything I can help with to get this into a new release? Seems minor but this prevents users from using pyinstaller.

I would like to do some testing with pixi, which uses conda packages and does not support a development install from git (which is how I would workaround this normally with pip). 😁

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 6, 2024

@j9ac9k anything I can help to get this into a new release? Seems minor but this prevents users from using pyinstaller.

I would like to do some testing with pixi, which uses conda packages and does not support a development install from git (which is how I would workaround this). 😁

I had been holding off on a fix until I can resolve #2491. I should point out that the issue is poorly written (I'm the author, I need to spend some more time rewriting and be more specific with the intricacies 🙃 ), but given the series of fixes that have been submitted since the last release (including the pyinstaller issue addressed here), I have come to the conclusion that I should undo the "bug-fix" I made (which included a regression described in #2941) and work on a better way to resolve #2661

Sorry @meganbkratz I'm going to have to roll back the bug fix for your issue for a while

I hope to cut a release in the next 24 hours. Thanks for following up @nicoddemus ...if you have some time/interest, I would love your assistance with re-fixing #2661 after I cut this next release.

@nicoddemus
Copy link
Copy Markdown
Contributor Author

Awesome, thanks a lot @j9ac9k, appreciate it!

if you have some time/interest, I would love your assistance with re-fixing #2661 after I cut this next release.

To be honest, I know almost nothing about pyqtgraph's codebase, so I'm not sure how much help I could provide. My initial offer to get the release out was along the lines of "release-wise" work, like tidying up changelog, helping reviewing some outstanding PRs, that kind of thing.

@meganbkratz
Copy link
Copy Markdown
Contributor

@j9ac9k No worries about rolling back the svg exporter fix. I have a workaround (subtracting out a constant to make the axis values smaller), so it's not urgent. Thank you for your work on it!!!

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 6, 2024

To be honest, I know almost nothing about pyqtgraph's codebase, so I'm not sure how much help I could provide. My initial offer to get the release out was along the lines of "release-wise" work, like tidying up changelog, helping reviewing some outstanding PRs, that kind of thing.

Fair enough, issue referenced isn't something requiring lots of tinkering in the pyqgraph codebase or knowledge of its working. My PR did one attempt to workaround for Qt's implementation of painting QSvgGenerator object

https://github.com/pyqtgraph/pyqtgraph/blob/master/pyqtgraph/exporters/SVGExporter.py#L268-L273

The numbers to the output XML where the precision is lots (so exporting a line that has points with a large numerical value in x (or y), but the values are similar/close together, you'll lost the precision. My attempt to work around this is to move the line/curve to be between (-1, 1) and then write the SVG out.

I suspect there is some more fanciness we can do with applying a transform to the graphics items, and then setting the QPainter::setTransform , and then applying the inverse after the painter writes to the QSvgGenerator,... anyway just need to spend more time with it (EDIT: and I should not hold up the next release until I spend more time with it)

@meganbkratz Thanks for chiming in and for your understanding 👍🏻

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.

import pyqtgraph.canvas fails.

4 participants