Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added bokeh implementation of view_quilt #1365

Merged
merged 3 commits into from
Jul 1, 2024

Conversation

salehtahini
Copy link

@salehtahini salehtahini commented Jun 18, 2024

Description

This PR adds the function nb_view_quilt() which is a 1:1 implementation of the view_quilt() function in Bokeh. The demo_pipeline notebook referred to an interactive version of the view_quilt function but I was unable to find it. The view_quilt function had a todo for implanting a bokeh version.

When this function is evoked, it shows the quilt plot with 2 sliders for changing the rf and stride parameters. I opted for modifying the underlaying parameters with the sliders instead of the patch size and overlap, because some width values are impossible to use. For example, the patches can never have a size of 22 because of the formula patch size = (rf * 2) + 1.

The demo_pipeline notebook was also modified to use the Bokeh version.

  • New feature (non-breaking change which adds functionality)
  • Changes in documentation or new examples for demos and/or use cases.

Has your PR been tested?

caimanmanager demotest ran without any issues.

caimanmanager test failed at caiman.tests.test_sbx... but had no other issues.

The nb_view_quilt() have also been manually tested and it always produced the exact same output as view_quilt(), given identical inputs.

@pgunn
Copy link
Member

pgunn commented Jun 18, 2024

On first glance this looks great - I'm currently at a conference and will give it a more careful look when I get back (also hoping @kushalkolar has time to give this a look).

Bravo on putting in the time on this.

plot.rect(x='center_x', y='center_y', width='width', height='height', source=source, color=color, alpha=alpha)

# Create sliders
stride_slider = Slider(start=1, end=100, value=rf, step=1, title="Patch half-size (rf)")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm getting a NameError: name 'Slider' is not defined from this (and the line below), and it's not obvious whether bokeh.models.Slider or matplotlib.widgets.Slider is meant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, actually, this is definitely meant to be a bokeh.models.Slider by the arguments.

@pgunn
Copy link
Member

pgunn commented Jul 1, 2024

This passes tests and looks great. Thanks for the contribution!

@pgunn pgunn merged commit 90904b3 into flatironinstitute:dev Jul 1, 2024
1 of 3 checks passed
@salehtahini
Copy link
Author

Thanks, happy to contribute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants