search

flatironinstitute / CaImAn

Computational toolbox for large scale Calcium Imaging Analysis, including movie handling, motion correction, source extraction, spike deconvolution and result visualization.
https://caiman.readthedocs.io
GNU General Public License v2.0
640 stars 370 forks source link

updated vis tools to work with new bokeh #1129

Closed EricThomson closed 1 year ago

EricThomson commented 1 year ago

Description

Updating visualization tools to handle updated bokeh as discussed in issue #1127. Basically two problems: the way it handled y origin was changed, and plot_height was deprecated. Also, I fixed an unrelated check for None that was throwing an error.

Fixes #1127

This is not yet finished, as discussed at the issue nb_view_patches3d() is still acting weird and I'm not sure how to put in the simple fix. It is probably something really simple I'm missing :smile:

Type of change

EricThomson commented 1 year ago

Currently seems to be working except one relatively minor thing in nb_view_patches3d(), when max_projection is set to True, the column projection appears in the wrong place. I wasn't able to quickly figure out a fix, but I think this is the last obvious problem.

Here is what it looks like in max_projection mode.

bokeh_3d_view_max

EricThomson commented 1 year ago

Oh I guess it was supposed to look like that. Hmm...that's counterintuitive, but ok. I'll resolve conflicts

EricThomson commented 1 year ago

Reason for, perhaps counterintuitive, handling of previous:

if self.R is None or self.R == b'NoneType'

Is this started to throw an error when self.R was an ndarray. The problem is self.R is sometimes b'NoneType' which is not the same as None, so the first condition will not throw a True. The solution I came up with was:

if self.R is None or not isinstance(self.R, np.ndarray):

This is related to #967 and #969

j-friedrich commented 1 year ago

Oh I guess it was supposed to look like that. Hmm...that's counterintuitive, but ok. I'll resolve conflicts

Yes, supposed to look like that if you use the current default axis=0, I'd call it with axis=2, so that the main projection in the top right is along the z-axis not the x-axis. Might want to change the default to 2, assuming z is usually the shortest dimension

EricThomson commented 1 year ago

I had tried that, but then panels got ordered correctly, but the image got ordered wrong: bokeh_axis2

j-friedrich commented 1 year ago

Had the same issue, but fixed it in dev

EricThomson commented 1 year ago

Sorry let me look I thought I had the same problem in dev but maybe I used axis=1. let me check, and edit this space.

EricThomson commented 1 year ago

Ok right that isn't a problem now. New problem -- you committed directly to dev, while I had already made a PR based on dev, so things are a bit weird. Let me try to make changes you made to dev play nicely with my PR. My fix shifted things back to origin, but yours did too, so now they are shifted up to negative.

EricThomson commented 1 year ago

In the fixing funky visualization commit, I fixed the problems with the 2d plotters with the most recent changes from the commit clashes, but the 3d has some residual problems. E.g., when plotting layered (no max projection) the image and patches don't seem properly matched : one of them seems flipped on the horizontal axis (note this isn't just a layer artifact -- this is the best layer for that patch):

bokeh_layered_view

Then with the max projection view (with axis=2) things are very funky, clearly the panels are shifted, I'm not sure exactly what has gone wrong.

bokeh_max_axis2

When working on this, please work from this branch (even if you have to tear down some of my changes :smile:) as otherwise conflicting commits can get confusing to figure out. The problem is fixing the 2d viewers seems to induce breaking changes in the 3d viewers, and vice-versa. So we just need to find a combination of fixes that gets everything playing nice.