[ENH] OWScatterPlotBase: Animate dot resize

[VesnaT]

Issue

Description of changes

Includes

• Code changes
• Tests
• Documentation

[janezd]

The reason why I suggested putting the class into the function is that it can get data from the closure, like this:

            current_size_data = self.scatterplot_item.data["size"].copy()
            diff = size_data - current_size_data
            widget = self

            class Timeout:
                # 0.5 - np.cos(np.arange(0.17, 1, 0.17) * np.pi) / 2
                factors = [0.06962899, 0.25912316, 0.51570538, 0.7679134 ,
                           0.94550326, 1]

                def __init__(self):
                    self._counter = 0

                def __call__(self):
                    factor = self.factors[self._counter]
                    self._counter += 1
                    size = current_size_data + diff * factor
                    if len(self.factors) == self._counter:
                        widget._timer.stop()
                        size = size_data
                    widget.scatterplot_item.setSize(size)
                    widget.scatterplot_item_sel.setSize(size + SELECTION_WIDTH)

But I forced this onto you. :) If you don't like getting data from closure, there's also no reason to have this class inside a function -- as I can see, the class gets all the data it needs. You can consider putting it inside the class OWScatterPlotBase or even at the module level.

[janezd]

Easing from jQuery is computed as 0.5 - cos(x * pi) / 2, where 0 < x < 1 (see jQuery code).

We can use

                # 0.5 - np.cos(np.arange(0.1, 1, 0.1) * np.pi) / 2
                factors = [0.02447174, 0.0954915, 0.20610737, 0.3454915,
                           0.5, 0.6545085, 0.79389263, 0.9045085, 0.97552826, 1]

or

                # 0.5 - np.cos(np.arange(0.17, 1, 0.17) * np.pi) / 2
                factors = [0.06962899, 0.25912316, 0.51570538, 0.7679134,
                           0.94550326, 1]

Keep the comment so we know how it's computed.

[janezd]

... or you can just use the formula :)

The formula I put in the comment is a bit wrong, the resulting matrix doesn't end with 1.

[VesnaT]

Honestly, I cannot spot the difference. But let it be..

[janezd]

Set the widget's _timer to None. I believe that otherwise the timer's signal is still connected to an instance of Timeout(), which is inside the bound method, which holds a reference to self, which contains _timer -- and we have circular references and a potential memory leak.

[janezd]

Think about any situations in which the timer has to be stopped (and set to None). I suppose it should suffice doing this when self.scatterplot_item or self.scatteplot_item_sel is set to None, which is in clear (when reset_graph sets it to something else, it first calls clear). You should also probably do it when the graph is deleted, so QTimer does not by any chance call a disposed object.

The second issue to consider is that of memory leaks. It should probably suffice if you set self._timer to None after stopping it, but carefully consider if anybody is still holding a reference to anything that can create a circle.

[janezd]

I played with sizes on networks. It's cool that it ... just works. :)

However, I encountered the same thing you have on some tests:

File "/Users/janez/Dropbox/orange3/Orange/widgets/visualize/owscatterplotgraph.py", line 758, in __call__
    self._scatter_item.setSize(size)
File "/Users/janez/anaconda3/envs/o3/lib/python3.6/site-packages/pyqtgraph/graphicsItems/ScatterPlotItem.py", line 522, in setSize
    self.updateSpots(dataSet)
File "/Users/janez/anaconda3/envs/o3/lib/python3.6/site-packages/pyqtgraph/graphicsItems/ScatterPlotItem.py", line 562, in updateSpots
    self.fragmentAtlas.getAtlas() # generate atlas so source widths are available.
File "/Users/janez/anaconda3/envs/o3/lib/python3.6/site-packages/pyqtgraph/graphicsItems/ScatterPlotItem.py", line 204, in getAtlas
    self.buildAtlas()
File "/Users/janez/anaconda3/envs/o3/lib/python3.6/site-packages/pyqtgraph/graphicsItems/ScatterPlotItem.py", line 189, in buildAtlas
    self.symbolMap[key].setRect(y, x, h, w)
File "/Users/janez/anaconda3/envs/o3/lib/python3.6/weakref.py", line 137, in __getitem__
    o = self.data[key]()KeyError: ('o', 21.0, 5061836248, 5061931304)

[codecov]

Codecov Report

Merging #3436 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3436      +/-   ##
==========================================
+ Coverage   83.22%   83.23%   +<.01%     
==========================================
  Files         362      362              
  Lines       64106    64142      +36     
==========================================
+ Hits        53352    53387      +35     
- Misses      10754    10755       +1

[codecov]

Codecov Report

Merging #3436 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3436      +/-   ##
==========================================
+ Coverage   83.22%   83.24%   +0.01%     
==========================================
  Files         362      362              
  Lines       64106    64145      +39     
==========================================
+ Hits        53352    53395      +43     
+ Misses      10754    10750       -4

[VesnaT]

Yes, I could put the class outside and somehow reuse it for changing shapes and colors as well. :)
...even though I like getting data from closure. But there seems to be an issue with self.scatterplot_item which I don't quite understand (see the modified code).