search

maximilianh / cellBrowser

main repo: https://github.com/ucscGenomeBrowser/cellBrowser/ - Python pipeline and Javascript scatter plot library for single-cell datasets, http://cellbrowser.rtfd.org
https://github.com/ucscGenomeBrowser/cellBrowser/
GNU General Public License v3.0
104 stars 40 forks source link

Improve selection #178

Closed mxposed closed 4 years ago

mxposed commented 4 years ago

Hi Max,

I reworked selection a bit. Main changes: — selCells in MaxPlot is not a Set() instead of an array — onSelChange function now updates the checkboxes in the legend based on the cells that are selected — tpLegendClear checkbox now toggles behaviour: it selects all checkboxes or deselects according to the current state

Please, let me know what you think

mxposed commented 4 years ago

Oh, also, I think there is a bug with zoom-in by double click: on first double click it zooms in, but then you need only one click to continue zooming (if you haven't moved your mouse). I fixed that in a separate commit, but maybe it was a feature : )

maximilianh commented 4 years ago

I think the zoom thing was indeed a bug! Many thanks!

maximilianh commented 4 years ago

I'm very unsure if Set() is the right data type here. To save time, I made it an array. It's very fast to iterate over an array, I'm quite sure that iterating over a set is not as fast. Why do you want to make it a set? I agree that a set seems the most natural choice, that's what it is after all, but you want to make drawing fast and drawing means iteration and iteration means array...

Instead of overdrawing the selection at the end, we could also avoid double-drawing. This would make some sense... once the selection is a set, instead of drawing everything and then drawing the selection on top, we could check if the current circle is in the selection and then change the color. This would means that sometimes the selection is not drawn on top, but it would allow other things, like selections of different colors, which may be good for your future idea of having a background selection...

mxposed commented 4 years ago

I chose Set because it logically is a set, to avoid duplicates. I needed to avoid duplicates to figure out which checkboxes to select/unselect based on selection.

I haven't benchmarked exact time, but iterating over set should be only marginally slower, because internally this is just a sparse array, and when I tested it on my toy 50k dataset the speed was the same. If you have time, maybe you could try this branch with you data. The most expensive operation I see is drawing the violin plot (which I'd like to make asynchronous)

maximilianh commented 4 years ago

OK let's try it then.

Let's not forget that the we can now restructure the drawing code to a "if element is in selection, change color", but this of course would require some benchmarking, too.

maximilianh commented 4 years ago

Thanks Nikolay! These will be very useful!