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

Make feature maps work for all layers #80

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

jo-mueller
Copy link
Contributor

Hi @haesleinhuepf,

I made a little addon to the results table. I created a function create_featuremaps that generalizes the functionality that was previously in the _on_double_click() function of the TableWidget, which is triggered when the table header is doubleclicked and which generated a featuremap.

The _on_double_click() function now just calls the new create_featuremaps function and passes the layer and the feature column to it. The new function then visualizes the feature accordingly for every layer:

  • As intensity image for Labels layer
  • As points layer with face_color according to feature
  • As vectors layer with edge_color according to feature
  • As surface layer with surface[2] overwritten with the values of the selected feature.

Since viewer.add_surface(data, features=features) is not allowed whereas viewer.add_surface(data, metadata = {'features':features}) is, I added a little snipped in the __init__ of the TableWidget, which moves the features from the metadata to layer.features.

QA

  • Added tests to cover function for all intended layers
  • Checked whether everything works from the viewer

Let me know what you think!

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2023

Codecov Report

Patch coverage: 84.90% and project coverage change: +1.55 🎉

Comparison is base (e38cc72) 86.08% compared to head (9731298) 87.63%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   86.08%   87.63%   +1.55%     
==========================================
  Files          13       14       +1     
  Lines        1272     1351      +79     
==========================================
+ Hits         1095     1184      +89     
+ Misses        177      167      -10     
Impacted Files Coverage Δ
napari_skimage_regionprops/_table.py 78.06% <14.28%> (+6.63%) ⬆️
napari_skimage_regionprops/_parametric_images.py 93.12% <90.00%> (-1.38%) ⬇️
...ari_skimage_regionprops/_tests/test_featuremaps.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@haesleinhuepf haesleinhuepf left a comment

Choose a reason for hiding this comment

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

Hi Johannes @jo-mueller ,

thanks for working on this! I have some doubts about backwards-compatibility. I think this PR is changing former behaviour. Could you comment on this? See comments below.

Thanks!

Best,
Robert

napari_skimage_regionprops/_table.py Show resolved Hide resolved
Comment on lines -74 to -116
if "label" in self._table.keys():
selected_column = list(self._table.keys())[self._view.currentColumn()]
print("Selected column", selected_column)
if selected_column is not None:
if isinstance(self._layer, napari.layers.Labels):
from ._parametric_images import visualize_measurement_on_labels, map_measurements_on_labels
new_layer = self._viewer.add_image( map_measurements_on_labels(self._layer, selected_column, self._viewer) ,
name=selected_column + " in " + self._layer.name ,
affine=self._layer.affine ,
scale=self._layer.scale,
rotate=self._layer.rotate
)
new_layer.contrast_limits = [np.min(self._table[selected_column]), np.max(self._table[selected_column])]
new_layer.colormap = "jet"
elif isinstance(self._layer, napari.layers.Points):
features = self._layer.features
new_layer = self._viewer.add_points(self._layer.data,
features=features,
face_color=selected_column,
face_colormap="jet",
size=self._layer.size,
name=selected_column + " in " + self._layer.name,
affine=self._layer.affine,
scale=self._layer.scale,
rotate=self._layer.rotate
)
new_layer.contrast_limits = [np.min(self._table[selected_column]), np.max(self._table[selected_column])]

if "vertex_index" in self._table.keys():
selected_column = list(self._table.keys())[self._view.currentColumn()]
print("Selected column (T)", selected_column)
if selected_column is not None and isinstance(self._layer, napari.layers.Surface):
values = np.asarray(self._table[selected_column])
data = self._layer.data
data = [np.asarray(data[0]).copy(), np.asarray(data[1]).copy(), values]

new_layer = self._viewer.add_surface(data,
name=selected_column + " in " + self._layer.name)
new_layer.contrast_limits = [np.min(self._table[selected_column]), np.max(self._table[selected_column])]
if "annotation" in selected_column or "CLUSTER_ID" in selected_column:
new_layer.colormap = "hsv"
else:
new_layer.colormap = "jet"
Copy link
Owner

Choose a reason for hiding this comment

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

Are you sure, you can remove this entire code? It seems that this breaks backwards compatibility. E.g. where is the new_layer.colormap going?

Copy link
Contributor Author

@jo-mueller jo-mueller Jul 11, 2023

Choose a reason for hiding this comment

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

I moved the setting of the colormap into the new create_feature_map function. The colormaps for the respective different layers are set at the following locations:

  • Here for labels layers (which will be returned as ImageData)
  • Here for PointsData
  • Here for VectorsData
  • Here for SurfaceData

The reason for doing it this way is that the color displays for different layers are controlled by different attributes (e.g., colormap, face_colormap, edge_colormap)

@jo-mueller
Copy link
Contributor Author

jo-mueller commented Jul 11, 2023

@haesleinhuepf Thanks for having a look at this. I think what I'm still missing here is taking the associated transformations (scale, affine, rotation) into the newly created feature map. Are those attributes part of what concerns you regarding backwards-compatibility?

Otherwise I would say what I did is 70% refactoring, mainly.

@jo-mueller
Copy link
Contributor Author

I added the scale, affine, rotation to the properties of the returned feature map and also amended the tests to make sure that these parameters are always set.

@jo-mueller
Copy link
Contributor Author

Hi @haesleinhuepf , is there something I could do to see this through? I would be really happy to see this as part of the plugin :)

content = layer.features.to_dict('list')

# to accomodate passing features to surfaces through the metadata
elif 'features' in layer.metadata.keys():
Copy link
Owner

Choose a reason for hiding this comment

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

Can you give an example (plugin) where there is data in layer.metadata['features'] but not in layer.features? Might it be possible to talk to the developers of these plugins so that they use layer.features? It would make more plugjns compatible to each other if we all use the same API 😉

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