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

Enable appending to existing napari viewer in display() #1093

Merged
merged 3 commits into from
Mar 19, 2019

Conversation

kevinyamauchi
Copy link
Collaborator

Overview

This pull request adds the ability to display multiple ImageStack objects in the same napari viewer, as discussed in #1086. Towards this, I added the option to pass in a napari Viewer object to display(). If the user passes in a Viewer object, the stack and/or spots are added as new layers in the existing Viewer. If None (default) is passed, the stack and/or spots are displayed in a new Viewer. Closes #1086.

Question

I am not sure what to do for the viewer type hint (line 111). It seems to me like we are trying to avoid importing napari at the top of _display.py to ease the dependency, so I am not sure how to give the Viewer class as a type hint. Thoughts?

Usage

from starfish import data, FieldOfView, display

# Download the ISS sequencing and dot images
experiment = data.ISS(use_test_data=False)
fov = experiment.fov()
primary_image = fov.get_image(FieldOfView.PRIMARY_IMAGES)
dots = fov.get_image('dots')

# View the dots overlaid on the sequencing images
viewer = display(primary_image)
viewer = display(stack=primary_image, viewer=viewer)

starfish/_display.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@kne42 kne42 left a comment

Choose a reason for hiding this comment

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

LGTM. Once the specified issues have been addressed this should be ready to merge.

You should also include in the documentation somewhere that the viewer argument only works in interactive Python sessions :)

starfish/_display.py Outdated Show resolved Hide resolved
kevinyamauchi and others added 2 commits March 18, 2019 14:59
Co-Authored-By: kevinyamauchi <kevin.yamauchi@gmail.com>
@codecov-io
Copy link

codecov-io commented Mar 18, 2019

Codecov Report

Merging #1093 into master will decrease coverage by 0.07%.
The diff coverage is 38.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1093      +/-   ##
==========================================
- Coverage    88.8%   88.72%   -0.08%     
==========================================
  Files         165      165              
  Lines        6433     6441       +8     
==========================================
+ Hits         5713     5715       +2     
- Misses        720      726       +6
Impacted Files Coverage Δ
starfish/_display.py 28.57% <38.09%> (-0.38%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7192ae2...b81fc9b. Read the comment docs.

@kevinyamauchi kevinyamauchi merged commit c224553 into master Mar 19, 2019
@kevinyamauchi kevinyamauchi deleted the ky-update-display branch March 19, 2019 00:42
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.

3 participants