Avoid all-NaNs warning

[StSav012]

I'm uncertain whether the way is the best. If the data becomes all-NaNs, the bounds don't change, like if the plot is waiting for data.

With the fix, lines like RuntimeWarning: All-NaN slice encountered ... don't appear if the data consists of only NaNs.

And please make your mind up to whether ScatterPlotItem.bounds is a tuple or a list. I'd rather make it a tuple everywhere.

[j9ac9k]

Hi @StSav012

Thanks for the PR. Looks like the CI doesn't like one of the changes, perhaps there is a case where self.bounds returns a non-iterable, which boundingRect() doesn't know how to handle.

And please make your mind up to whether ScatterPlotItem.bounds is a tuple or a list. I'd rather make it a tuple everywhere.

There are a ton of these kinds of inconsistencies throughout the library; really hoping that when we start rolling out type-hinting, it will catch more of them. I'm good with migrating to tuples (or even better ,named tuples); but I would have to make sure that we're not dependent on the mutability of that list somewhere.

[StSav012]

Hi, @j9ac9k,

Thank you. You're absolutely right. Have you happened to know why ScatterPlotItem.bounds becomes just None? I'd like to alter the peace of code.

[j9ac9k]

I don't know off the top of my head, but I would speculate that this value is None when the item isn't visible yet.

[StSav012]

Oh, my bad. ScatterPlotItem.bounds is not a tuple (or a list) of None | float but a tuple (or a list) of None | tuple[float, float]. That's why I get a single None in some cases. I wonder if changing

pyqtgraph/pyqtgraph/graphicsItems/ScatterPlotItem.py

Line 420 in d7cda66

self.bounds = [None, None] ## caches data bounds

to self.bounds = [(None, None), (None, None)] helps.

[outofculture]

So reading through ScatterPlotItem today with @j9ac9k, it's clear that self.bounds is the placeholder for a cached calculation used by self.dataBounds. Whenever the code does self.bounds = [None, None] elsewhere, it's really just invalidating that cache. Which is also why it's a list; it's going to get a value calculated for each axis, but only as needed, so it does change. As for self.bound's data type, yeah, it's currently holding 2 pieces of information: whether the cache is valid and what that calculation last gave us. self.dataBounds shouldn't need to ever return a plain None, as that method should always be able to return the bounds as it understands them. Yet, when we don't have any data to describe bounds on, those boundaries need to be expressive of that. Those bounds could reasonably be NaNs, and I think most math would work out sensibly.

With all that in mind, we have a number of things worth doing:

• remove self.bounds from the public properties; rename it to self._bounds.
• add a deprecation warning on a @property named def bounds telling users to always use self.dataBounds instead.
• document the return type
• if you're feeling up for it, deprecate dataBounds so we can change the behavior to returning NaNs whenever data is missing. the new method could be named dataBoundaries or dataMinMax? something along those lines.

[j9ac9k]

• add a deprecation warning on a @property named def bounds telling users to always use self.dataBounds instead.

I don't think this is needed; I know there is always the spacebar heater workflow but in this case we have a well defined method, so someone querying the bounds attribute directly was already looking for trouble IMO.

• if you're feeling up for it, deprecate dataBounds so we can change the behavior to returning NaNs whenever data is missing. the new method could be named dataBoundaries or dataMinMax? something along those lines.

I know we chatted about this earlier, but at the risk of changing my opinion and pushing you in front of the bus, I don't think this is needed either. This PR doesn't change ScatterPlotItem.dataBounds() in a meaningful fashion, it just uses the cache more consistently throughout, and modifies ScatterPlotItem.bounds