napari / napari

napari: a fast, interactive, multi-dimensional image viewer for python
https://napari.org
BSD 3-Clause "New" or "Revised" License
2.21k stars 422 forks source link

Properties handling should be unified across layers #2866

Open jni opened 3 years ago

jni commented 3 years ago

πŸš€ Feature

We have a bunch of layers with properties now, including points, labels, tracks, and (I think?) shapes. Really any layer with unique IDs could have properties, with rows corresponding to IDs. (ie vectors should also have properties.)

Currently, each layer's properties were implemented independently and share almost no code/infrastructure. This has resulted in weird layer-specific bugs and also unexpected behaviour where things that work in one layer (e.g. points.face_color = 'property-name') don't work in another (labels.color = 'property-name' doesn't work, iirc). Long story short, we should make a big lift to unify all property handling for all layers.

Some random comments about what to do here. I've made them tick boxes as many of them can be separate tasks.

Related issues (list should be updated as we come across more): #2755 #2756.

brisvag commented 3 years ago

Other related issue: #2756.

On dataframes: xarray advertises its DataSet object as an N-dimensional alternative to pandas DataFrames, and heavily inspired by it. I never used them, but mayeb they are worth taking a look at, if this can help with point number 4. EDIT: http://xarray.pydata.org/en/stable/user-guide/data-structures.html#dataset

jni commented 3 years ago

On dataframes: xarray advertises its DataSet object as an N-dimensional alternative to pandas DataFrames, and heavily inspired by it. I never used them, but mayeb they are worth taking a look at, if this can help with point number 4.

Worth exploring! I don't know whether that analogy goes further than skin deep though β€” ie if I pass in a DataSet to functions expecting DataFrames, I expect bad things to happen. πŸ˜‚ But we should investigate, absolutely. πŸ‘

alisterburt commented 3 years ago

While there is some unification effort underway (@andy-sweet @brisvag) I'll add a discussion point here

Currently, if you add points onto a points layer which is coloured by property the new point will 'inherit' the properties of the last added point. Here's an example you can play with

import napari
import numpy as np

n = 200
points = np.random.normal(size=(n, 2))
properties = {k: np.random.normal(size=n) for k in ('prop1', 'prop2')}

v = napari.Viewer()
layer = v.add_points(points, properties=properties, face_color='prop1', size=0.1)

# add some points manually then look at layer.properties['prop1'] and layer.properties['prop2']

This is not always wanted and makes it hard to tell what was added and what is from the data - it would be nice to be able to define what happens for newly added points e.g. provide a NaN type value or a per-column default

jni commented 3 years ago

it would be nice to be able to define what happens for newly added points

I think that's controlled by layer.current_properties. Of course, there is no way to access those from the UI, because that opens up hard design issues, and who needs those? πŸ˜‚ Also, if I remember correctly, even if you update them from the console, they will get updated back if you change the selection or the last point's properties. So that's definitely something to consider. We definitely went with "let's think more about this later" back when @kevinyamauchi was first implementing this. πŸ˜…

But, yes, absolutely hear you, and again, this would be made easier by unified handling across layers β€” currently, you would have to fix this issue for each layer type individually.

kevinyamauchi commented 3 years ago

Great discussion everyone!

I think that's controlled by layer.current_properties. Of course, there is no way to access those from the UI, because that opens up hard design issues, and who needs those? πŸ˜‚ Also, if I remember correctly, even if you update them from the console, they will get updated back if you change the selection or the last point's properties. So that's definitely something to consider. We definitely went with "let's think more about this later" back when @kevinyamauchi was first implementing this. πŸ˜…

I agree that this behavior needs to be re-visited! As @jni said, the behavior was meant to be a proposal that was then revisited after some user testing. I have a listed a few thoughts below that might give some context for the decision:

We have a bunch of layers with properties now, including points, labels, tracks, and (I think?) shapes. Really any layer with unique IDs could have properties, with rows corresponding to IDs. (ie vectors should also have properties.)

@jni, I believe the Vectors layer has properties and colors can be set by those properties (see this example). Now, whether the behavior is perfectly consistent with the other layers is another question πŸ˜†

Documentation

Another thing I would like to add to the "task list" is improving the documentation around properties. We have some tutorials on napari.org and some examples in the repo that show the usage of properties, but I believe we are missing documentation that explains how properties work (i.e., the "Explanation" type of documentation from the "four types of documentation"). I would be happy to help with this!

andy-sweet commented 3 years ago

I've made a lot of notes on this topic so far, but they mostly unintelligible. One thing I can share now that might serve as a useful reference is the following table that tells us what property related values are stored in various layers.

Screen Shot 2021-06-25 at 1 54 54 PM

The vectors layer does not have a current_properties attribute, but it does make them when needed for ColorManager.

This doesn't include more specific properties (e.g. Points.face_color), but maybe it should. I see Labels.label_index as a specific implementation/optimization detail of the Labels layer, but it does relate to some shared code already, so thought it was worth listing.

jni commented 3 years ago

Looks like I never added what I think should be the canonical reference here:

Tamara Munzner (2014) Visualization Analysis & Design

It's a reasonably thick book for a semester-long course, but everything is nicely summarised in this blog post:

https://maelfabien.github.io/machinelearning/Dataviz/

andy-sweet commented 3 years ago

Here's a doc I've been working on that is my understanding of the behavior, implementation, and API of properties and related concepts: https://docs.google.com/document/d/1ZSlUPmGop6wuVEPTVC_DCb49E0orhyplrEyV302-sdU/edit?usp=sharing

It doesn't include any proposals or plans, but feel free to comment on any misunderstandings or anything that's missing.

I'm also thinking about converting the doc to Markdown and hosting on hackmd instead.

imagesc-bot commented 2 years ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/using-layer-properties-to-pass-variables/63342/2

imagesc-bot commented 1 year ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/exloring-image-segment-features-in-napari/75222/12

andy-sweet commented 1 year ago

Posting here to make sure this work/progress isn't lost.

The latest hackmd doc is probably the best way to consume the altair/vega-style style encoding proposal: https://hackmd.io/IOuBuY11QtS56dyrdH85aQ?view

That is in the form of a NAP, but didn't actually go through the NAP process. So the Accepted status is just to indicate that there was some more informal consensus around this approach.

We did the work to define string and color encodings, but we only use them in TextManager right now. I did draft a PR for using them in Points, but that's the effort paused.

brisvag commented 1 year ago

That is in the form of a NAP, but didn't actually go through the NAP process. So the Accepted status is just to indicate that there was some more informal consensus around this approach.

Would be great to retroactively add it to the NAPs :)

imagesc-bot commented 1 year ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/exloring-image-segment-features-in-napari/75222/21

imagesc-bot commented 1 year ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/changing-color-of-labels-when-using-volume-rendering/75289/3

imagesc-bot commented 4 months ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/error-while-adding-text-to-napari/76960/6