Skip to content

Avoid Qt receiving nan values#1058

Closed
2xB wants to merge 3 commits intopyqtgraph:developfrom
2xB:fix-1057
Closed

Avoid Qt receiving nan values#1058
2xB wants to merge 3 commits intopyqtgraph:developfrom
2xB:fix-1057

Conversation

@2xB
Copy link
Copy Markdown
Contributor

@2xB 2xB commented Oct 17, 2019

Since Qt 5.13.1, paths containing non-finite values are not drawn. This PR avoids passing non-finite values to Qt. Also, as long as connect is neither 'all' nor an array, it also draws paths unconnected at places where a non-finite value occurs.

Fixes #1057.

@2xB 2xB mentioned this pull request Oct 17, 2019
@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Oct 17, 2019

Btw. as @j9ac9k noted in #1011, this might be a third-party bug (not in PyQt5, as it was observed in both PySide2 and PyQt5), so it might also be a good idea to instead of using this workaround, fixing the original bug. Now I sadly can't afford spending that time currently, so I will leave it as it is now.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 18, 2019

I'm reluctant to say this is a 3rd party "bug"; as the np.NaN behavior was always sort of a happy accident from what I can remember (as that was the most efficient way to plot a bunch of lines at once).

Looking at the Qt Changelog from 5.13.0 to 5.13.1, we have the following:
https://code.qt.io/cgit/qt/qtcharts.git/tree/dist/changes-5.13.1/?h=v5.13.1

****************************************************************************
*                                Bug Fixes                                 *
****************************************************************************

- QCandlestickSeries: remove sets on destroy to prevent leaks
- [QTBUG-70987] Fix crash on ChartView component destroy
- [QTBUG-76271] Revert "Fix rendering lines with Antialiasing render hint"
- [QTBUG-60384] Force entire chart update when points label clipping changes

There is also an update in QGraphicsView

 - QGraphicsView:
   * Ignore disabled items when setting the mouse cursor.

Not sure if any of those changes are what's causing this behavior.

@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Oct 23, 2019

Somehow tests are failing for Python 2.7/Qt 4.8...

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 24, 2019

@2xB if failing for an unrelated reason, would you mind opening a new issue?

Maintaining the python2-qt4 pipeline has been a real chore...

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Oct 24, 2019

I checked the CI builds, Python 2.7 builds are fine otherwise so something in this diff is breaking the test. It may be worth pinging @campagnola on slack to take a look at this one.

@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Oct 28, 2019

Turns out: The reason this test fails only for PyQt4 is probably that there is a larger tolerance in assertImageMatch for images generated by PyQt5 than for PyQt4.

@2xB
Copy link
Copy Markdown
Contributor Author

2xB commented Oct 28, 2019

The higher tolerance for PyQt5 is not intended to be permanent, therefore I created issue #1064 to remember that something has to be done there.

It seems btw. that one pixel is off with this change in Python2/PyQt4. If someone is interested in tracking down the reason, feel free to do so!

@spauka
Copy link
Copy Markdown
Contributor

spauka commented Jan 27, 2020

I'm also interested in getting this fixed in the latest version, as it's causing some issues with my plots as well. I will highlight that, while this solution works, it is causing a performance regression for plots with 10000-1000000 points.

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 6, 2020

Closing/Opening to re-trigger CI pipelines

@j9ac9k j9ac9k closed this Mar 6, 2020
@j9ac9k j9ac9k reopened this Mar 6, 2020
@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Mar 6, 2020

Here is the CI run, looks like all python2 pipelines failed:

https://dev.azure.com/pyqtgraph/pyqtgraph/_build/results?buildId=633&view=results

/shakes-fist-at-python2

We could put this PR on ice until we ship out 0.11 and drop py2 support.... thoughts @2xB ?

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented May 31, 2020

Closing/Opening again, CI pipeline should be more robust now...

raise Exception('connect argument must be "all", "pairs", "finite", or array')

arr[1:-1]['c'][non_finite_values] = 0
arr[1:-1]['c'][np.roll(non_finite_values,-1)] = 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there's a bug here--roll(-1) moves the first element to the end of the array ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The roll was used because the last entry of arr[1:-1]['c'] doesn't matter. Code to test:

import numpy as np
import pyqtgraph as pg

app = pg.mkQApp()

data = np.random.normal(size=10)
data2 = np.copy(data)
data2[0] = np.nan

pg.plot(data, title="no NaN")
pg.plot(data2, title="one NaN")

app.exec_()

@j9ac9k
Copy link
Copy Markdown
Member

j9ac9k commented Nov 23, 2020

Merged #1287 which takes care of this issue, did some timing information and documented it in #1057 showing that implementation being faster. If this PR addresses a use case that #1287 does not, let me know and we'll re-address. In the meantime, going to close this out.

@j9ac9k j9ac9k closed this Nov 23, 2020
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.

plots not showing with NaN

4 participants