Fix bounds handling when data is int16 or similar formats#2515
Fix bounds handling when data is int16 or similar formats#2515j9ac9k merged 8 commits intopyqtgraph:masterfrom
Conversation
While it is true that QRectF only has an overload that takes all float arguments, that is not the issue of #2513. The small integer arguments are automatically promoted to float. In [10]: du = np.array([0, 255], dtype=np.uint8)
In [11]: ds = np.array([-100, 100], dtype=np.int8)
In [12]: du[1] - du[0]
Out[12]: 255
In [13]: ds[1] - ds[0]
<ipython-input-13-d7287f596732>:1: RuntimeWarning: overflow encountered in byte_scalars
ds[1] - ds[0]
Out[13]: -56 |
|
Thank you for the clarification! I didn't look into the exact error in In reality, we already treat the return of the bounds calculation as a float value. An example for this is the adjustments for ScatterPlot symbol size, where we don't make any attempt to maintain format. That we don't usually run into trouble with weird data types seems more of a coincidence (and a fundamentally good implementation on the Python and NumPy side) rather than purposeful design on our end. It probably helps that the majority of applications will just naturally feed data in numpy float types. I switched the bounds data format to float rather than making the rectangle calculations handle the existing input better because I think that is the most robust solution here, helping to avoid problems with invalid developer assumptions in other/future parts of the code. I don't think there's ever a reason to handle this kind of meta-data in the user supplied data format. Support for higher resolution (Decimal?) might have been a motivation, but our plotting is fundamentally limited to what Qt uses internally. |
|
Looks like this PR should be merged, but leave #2513 open? |
|
Thanks @NilsNemitz for the PR and thanks @pijyoi for the feedback on the PR! |
We try to maintain the original format of the data passed to e.g. PlotDataItem. However, when this is something unconventional like
np.int16(#2513), we occasionally run into trouble when quantities derived from the data then have an unexpected data format.This PR addresses the issue with reported bounds for data in short integer format, which are not accepted by
QtCore.QRectF()later. It does so by simply casting the return ofPlotCurveItem.dataBounds()to float.ScatterPlotItem.dataBounds()already performs floating points math on the bounds to account for spot sizes anyway, such that the returned data should be promoted automatically and no explicit cast is required..A possible concern with the change in this PR is the existance of a test that checks that boolean data gets stored as
np.uint8and that the bounds are returned in the same format. However, the assertion in the test already fails when the tested PlotDataItem is set to draw a scatter plot (symbol='o', pen=None) and I tend to think the additional test for the bounds format simply reflected the expected behaviour at the time.I have changed the test to reflect the new behaviour of float bounds, and added some additional test cases.
Closes #2513