Add inspector tool #29

nvaytet · 2022-09-22T15:26:35Z

This is a tool for inspecting the 3rd dimension of 3d data by placing dots on a 2d image.
The dots indicate where to slice out the displayed dimensions, only to leave the 3rd dim.
The resulting 1d plot is shown on a new figure.
Points can be moved around, and removed.

Example:

%matplotlib widget
import plopp as pp

da = pp.data.dense_data_array(ndim=3)
pp.inspector(da)

Fixes #28

SimonHeybrock · 2022-09-23T06:55:29Z

docs/inspector-plot.ipynb

+   "source": [
+    "# Inspector plot\n",
+    "\n",
+    "The `inspector` plot is for three-dimensional data.\n",


Good name, I think.

For the future: I could imagine using this for 2-D data, in two ways:

1-D plot with 1-D profile.

2-D plot with inspected values displayed in a table.

I think "1-D plot with 1-D profile." as we had it in the old plotting, with adding vertical line markers was pretty useless.
For that, I am thinking more of having a superplot (like Lamp's superplot), which is like the slicer plot: you slice 2d data to get a 1d plot with a slider. In addition, you have buttons to save the current line. I think it's better than clicking on the figure to see a cut in the other dimension.

"2-D plot with inspected values displayed in a table.": good idea 👍

I think "1-D plot with 1-D profile." as we had it in the old plotting, with adding vertical line markers was pretty useless.

Not necessarily: Consider {'spectrum':1_000_000, 'tof':1000}. We cannot make a 2-D plot anymore, but someone might want to make a 1-D plot of the counts per spectrum and then inspect and compare TOF-spectra from different pixels.

What you suggest is a little different, because the inspector plot does no summing along the third dimension.
So in your example, the first plot wouldn't be the counts per spectrum, it would be the counts per spectrum in the first tof bin.

I guess you could easily make this by adding a summing node in the graph.

What you suggest is a little different, because the inspector plot does no summing along the third dimension.

Without sum this does not seem very useful? In many cases a single slice does not contain useful values to display?

So are you saying that inspector should also sum along the third dimension?

Should we allow different operations, such as sum, mean, min, max ?

So are you saying that inspector should also sum along the third dimension?

Yes.

SimonHeybrock · 2022-09-23T06:56:52Z

docs/inspector-plot.ipynb

+    "- Left-click to make new point\n",
+    "- Left-click and hold on point to move point\n",


Suggested change

"- Left-click to make new point\n",

"- Left-click and hold on point to move point\n",

"- Click to make new point\n",

"- Drag existing point to move it\n",

or something like that?

SimonHeybrock · 2022-09-23T06:58:08Z

src/plopp/__init__.py

@@ -14,7 +14,7 @@
 plt.ioff()

 from .core import Node, node, input_node, widget_node, show_graph, View
-from .functions import figure, plot, slicer
+from .functions import figure, plot, slicer, inspector


We should consider our import strategy: I feel some things should stay optional dependencies. I do not know if mpltoolbox is one of them, but pythreejs (for 3D scatter plots) should be.

currently mpltoolbox is optional. It only gets imported when inspector is called.

And we should probably think about reviewing the code in mpltoolbox, and also making it available on conda.

SimonHeybrock · 2022-09-23T07:01:33Z

src/plopp/functions/inspector.py

+from scipp import Variable, scalar
+from scipp.typing import VariableLike


I importing without namespace good style? As this is not part of scipp anymore we should maybe just import scipp as sc?

I don't know, i'm also importing things from ipywidgets like that. e.g. from ipywidgets import VBox.
I thought it is cheaper than importing the whole of ipywidgets, but maybe it makes no difference in practise because it still has to run through the whole __init__.py?

I guess it is a question of style, and we have never made a clear decision on that.

SimonHeybrock · 2022-09-23T07:02:31Z

src/plopp/functions/inspector.py

+
+    event_nodes = {}
+
+    def make_node(change):


What is the reason for the design choice of nested functions?

No particular reason. It was the easiest. I can refactor.

SimonHeybrock · 2022-09-23T07:03:31Z

src/plopp/graphics/interactive.py

@@ -81,23 +81,23 @@ def home(self):
        self.crop(**self._crop)
        self.draw()

-    def pan(self):
+    def pan(self, *_):


Silently ignoring other args? Why and where is this required?

I wanted the ToggleButton tool to send its current value when it is pressed, so that the PointsTool can decide whether to start or stop.

In this particular case, we use the tool state from the matplotlib figure to determine which zoom or pan tool should be active. So we don't actually care about the value of the tool for pan and zoom (we could care for logx, logy but not necessarily, cf the current implementation).

That said, in the current implementation the PointsTool is actually a ToggleTool and it knows its current value, so I could just do

def start_stop(self): if self.value: self.points.start() else: self.points.stop()

and no need to send the value when the button is toggled.

I guess I was thinking of future toggle tools, that might call a method that is not part of the class, and that might need the value in the callback message.

But that could be in the future. I am happy to just remove the value from the call, and remove the *_ from the fig.py.

That, or at least add some documentation about what is going on.

…tion operation along the third dimension

nvaytet added 8 commits September 21, 2022 17:31

start adding profiler

e0bd7b7

connect points tool to make lines appers on dot creation

4061bcc

now update of profile also works

995df9e

add remove method to node

8f4323f

can now remove dots

a6766a4

fix for any dims

5596282

rename to inspector

b5240ca

better icon for tool button

e2bc4a1

nvaytet marked this pull request as draft September 22, 2022 15:26

nvaytet added 3 commits September 22, 2022 19:13

add notebook in docs

0063e55

flake8

81e3dd1

add mpltoolbox to requirements

31e8d5a

nvaytet marked this pull request as ready for review September 22, 2022 17:21

add controls instructions

f991e38

SimonHeybrock reviewed Sep 23, 2022

View reviewed changes

nvaytet added 5 commits September 23, 2022 09:52

add docstring and make edges of markers white

dfc4241

remove value in togglebutton callback

3a9830c

change inspector to no longer have slider and instead perform a reduc…

ffca32f

…tion operation along the third dimension

refactor inspector to replace inline functions with event handler class

35acb71

flake8

b303ec2

SimonHeybrock approved these changes Sep 23, 2022

View reviewed changes

nvaytet merged commit 0072c11 into main Sep 23, 2022

nvaytet deleted the add_profiler branch September 23, 2022 10:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add inspector tool #29

Add inspector tool #29

nvaytet commented Sep 22, 2022 •

edited

Loading

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022 •

edited

Loading

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022

nvaytet Sep 23, 2022

SimonHeybrock Sep 23, 2022

SimonHeybrock Sep 23, 2022

SimonHeybrock Sep 23, 2022

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022

nvaytet Sep 23, 2022

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022

SimonHeybrock Sep 23, 2022

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022

SimonHeybrock Sep 23, 2022

nvaytet Sep 23, 2022

SimonHeybrock Sep 23, 2022

		"- Left-click to make new point\n",
		"- Left-click and hold on point to move point\n",

		from scipp import Variable, scalar
		from scipp.typing import VariableLike

Add inspector tool #29

Add inspector tool #29

Conversation

nvaytet commented Sep 22, 2022 • edited Loading

Choose a reason for hiding this comment

nvaytet Sep 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nvaytet commented Sep 22, 2022 •

edited

Loading

nvaytet Sep 23, 2022 •

edited

Loading