silx.gui: support imageaggregate in _plot2d #4174

EdgarGF93 · 2024-09-19T17:32:15Z

Checklist:

The PR title is formatted as: <Module or Topic>: <Action> <Summary> (see contributing guidelines)

This PR enables:

ImageAggregate filters on _plot2dview
ImageAggregate filters on Nexusimageview
ImageAggregate filters on StackView

EdgarGF93 · 2024-09-19T17:35:44Z

Not sure if the slot _aggregationModeChanged and setAggregatedImage are well placed into into the class Plot2View

t20100

Thanks for the PR!

I left a first round of feedbacks that aims at hopefully simplifying the implementation.

src/silx/gui/plot/PlotWindow.py

t20100 · 2024-09-23T06:59:31Z

src/silx/gui/plot/actions/image.py

+:mod:`silx.gui.plot.actions.image` provides a set of QAction relative to data processing
+and outputs for a :class:`.PlotWidget`.
+
+The following QAction are available:
+
+- :class:`CopyAction`
+- :class:`PrintAction`
+- :class:`SaveAction`


docstring in not consistent with the module content

src/silx/gui/plot/items/image_aggregated.py

src/silx/gui/plot/actions/image.py

src/silx/gui/data/DataViews.py

EdgarGF93 · 2024-09-26T11:41:08Z

how about this approach? If yes, I will use it for the rest of the classes

t20100

By avoiding calling addImage there is no need for setData **kwargs

src/silx/gui/plot/items/image_aggregated.py

src/silx/gui/data/DataViews.py

EdgarGF93 · 2024-10-06T10:19:12Z

About the _ComplexImageView class, the setData method updates the array and also the Complex mode, so I'm not sure how to implement the filter without changing the ImageComplexView class

t20100

Let's keep the complex number for another PR!

I left a few more comments. The main point is to put the aggregation mode action in the widget that adds/updates the aggregated plot item rather in the widget that is using this widget.

src/silx/gui/data/DataViews.py

t20100 · 2024-10-07T07:53:45Z

src/silx/gui/data/NXdataWidgets.py

+        imageItem = ImageDataAggregated()
+        imageItem.setName(legend)
+        self._plot.addItem(imageItem)
+


it doesn't sounds to be the right place to add the image item since it can be a scatter that is added

Suggested change

imageItem = ImageDataAggregated()

imageItem.setName(legend)

self._plot.addItem(imageItem)

t20100 · 2024-10-07T07:56:10Z

src/silx/gui/data/NXdataWidgets.py

+            imageItem.setData(image)
+            imageItem.setOrigin(origin)
+            imageItem.setScale(scale)
+            imageItem.setColormap(self._plot.getDefaultColormap())


It sounds this is where to create and add the image item:

Suggested change

imageItem.setData(image)

imageItem.setOrigin(origin)

imageItem.setScale(scale)

imageItem.setColormap(self._plot.getDefaultColormap())

imageItem = ImageDataAggregated()

imageItem.setName(legend)

imageItem.setData(image)

imageItem.setOrigin(origin)

imageItem.setScale(scale)

imageItem.setColormap(self._plot.getDefaultColormap())

self._plot.addItem(imageItem)

t20100 · 2024-10-22T08:11:04Z

src/silx/gui/data/DataViews.py

+        self.__aggregationModeAction = AggregationModeAction(parent=widget)
+        widget.getPlotWidget().toolBar().addAction(self.__aggregationModeAction)
+        self.__aggregationModeAction.sigAggregationModeChanged.connect(self._aggregationModeChanged)
        return widget

+    def getAggregationModeAction(self) -> AggregationModeAction:
+        """Action toggling the aggregation mode action
+        """
+        return self.__aggregationModeAction
+
+    def _aggregationModeChanged(self):
+        plot = self.getWidget().getPlotWidget()
+        item = plot._getItem("image")
+
+        if item is None:
+            return
+
+        if isinstance(item, ImageDataAggregated):
+            aggregationMode = self.getAggregationModeAction().getAggregationMode()
+            item.setAggregationMode(aggregationMode)
+


Let's add the AggregationModeAction in the widget which controls the item in the plot.

For _Plot2dView above, it is the widget that controls the plot item.

Here, the widget controlling the plot item is the StackView, so best to control it's plot item from this widget. Otherwise it adds implicit relations: When looking at the StackView code, one can wonder why an `ImageDataAggregated item is used there.

t20100 · 2024-10-22T08:14:16Z

src/silx/gui/data/DataViews.py

+        self.__aggregationModeAction = AggregationModeAction(parent=widget)
+        widget.getPlot().toolBar().addAction(self.__aggregationModeAction)
+        self.__aggregationModeAction.sigAggregationModeChanged.connect(self._aggregationModeChanged)
        return widget
+
+    def getAggregationModeAction(self) -> AggregationModeAction:
+        """Action toggling the aggregation mode action
+        """
+        return self.__aggregationModeAction
+
+    def _aggregationModeChanged(self):
+        plot = self.getWidget().getPlot()
+        item = plot._getItem("image")
+
+        if item is None:
+            return
+
+        if isinstance(item, ImageDataAggregated):
+            item.setAggregationMode(self.getAggregationModeAction().getAggregationMode())


Same here, the widget controlling the plot item is ArrayImagePlot, so the let it handle the aggregation mode as well (this would allow to enable/disable this action according to the displayed item image/scatter)

EdgarGF93 · 2024-10-27T17:34:56Z

Do you mean adding the Aggregation items in the class where the self._plot is? (StackView and ArrayImagePlot as you said)
In the case of _Plot2dView the plot is what is called widget, but yes, _Plot2dView is the class creating the plot anyway

Check it if I understood correctly

t20100 · 2024-11-04T12:14:49Z

Do you mean adding the Aggregation items in the class where the self._plot is? (StackView and ArrayImagePlot as you said)

Yes, StackView and ArrayImagePlot which are the widgets that create and add/remove the ImageAggregatedItem to the plot (there is not necessarily a self._plot in there). Doing so will make those widgets self-consistent.

…ated

it's already done in ArrayImagePlot

t20100 · 2024-11-18T16:13:44Z

I made a PR on your branch with a few updates/fixes: EdgarGF93#3
I let you review it and merge if you agree with it, else let's discuss it.

Apart from that, this looks good to me! Thanks for the work and your patience !

Update for PR silx-kit#4174

EdgarGF93 · 2024-11-26T15:56:03Z

Looks good to me, thanks Thomas!

support imageaggregate in _plot2d

8725dcc

EdgarGF93 changed the title ~~support imageaggregate in _plot2d~~ silx.gui: support imageaggregate in _plot2d Sep 19, 2024

EdgarGF93 marked this pull request as draft September 19, 2024 17:34

t20100 reviewed Sep 23, 2024

View reviewed changes

Edgar Gutierrez Fernandez added 6 commits September 25, 2024 17:10

add aggregation action in _Plot2dView

df68c57

imageaggregate by default

b07bc53

no kwargs needed

14bf6d6

predefined imagedataaggregatd

044a912

suggestions

af98201

blank

3b43874

EdgarGF93 requested a review from t20100 September 26, 2024 11:41

set aggregation mode in set data

73e566f

t20100 reviewed Sep 26, 2024

View reviewed changes

src/silx/gui/plot/items/image_aggregated.py Outdated Show resolved Hide resolved

src/silx/gui/data/DataViews.py Outdated Show resolved Hide resolved

src/silx/gui/data/DataViews.py Outdated Show resolved Hide resolved

src/silx/gui/data/DataViews.py Outdated Show resolved Hide resolved

Edgar Gutierrez Fernandez added 6 commits September 26, 2024 16:47

setdata

fa45a02

correction

6216f46

typo

ba5ec27

imageaggregate in NXdataWidget

5ab1d19

simplify

1e44061

stackview inherits from ImageDataAggregated

ed3e275

EdgarGF93 marked this pull request as ready for review October 6, 2024 10:17

t20100 added this to the Next release milestone Oct 7, 2024

t20100 reviewed Oct 22, 2024

View reviewed changes

Edgar added 2 commits October 27, 2024 18:21

aggregation items to ArrayImagePlot

ea0bd4c

aggregation items to StackView

ef444ad

EdgarGF93 requested a review from t20100 November 4, 2024 14:20

Edgar Gutierrez Fernandez and others added 6 commits November 13, 2024 09:22

again

8f58e87

Merge remote-tracking branch 'origin/main' into silx_view_imageaggreg…

6f074c1

…ated

simpler implementation by using self._stackItem

c8d19dd

remove unused import and useless code in _NXdataImageView

c934c5e

it's already done in ArrayImagePlot

simplify implementation by reusing the same item

bb290ad

Make the image active to enable mask and histogram

e6868fe

t20100 mentioned this pull request Nov 18, 2024

Update for PR #4174 EdgarGF93/silx#3

Merged

Merge pull request #3 from t20100/pr4174

b4b2529

Update for PR silx-kit#4174

t20100 approved these changes Nov 26, 2024

View reviewed changes

t20100 merged commit a9eddc5 into silx-kit:main Nov 26, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

silx.gui: support imageaggregate in _plot2d #4174

silx.gui: support imageaggregate in _plot2d #4174

EdgarGF93 commented Sep 19, 2024 •

edited

Loading

EdgarGF93 commented Sep 19, 2024 •

edited

Loading

t20100 left a comment

t20100 Sep 23, 2024

EdgarGF93 commented Sep 26, 2024 •

edited

Loading

t20100 left a comment

EdgarGF93 commented Oct 6, 2024 •

edited

Loading

t20100 left a comment

t20100 Oct 7, 2024

t20100 Oct 7, 2024

t20100 Oct 22, 2024

t20100 Oct 22, 2024

EdgarGF93 commented Oct 27, 2024 •

edited

Loading

t20100 commented Nov 4, 2024

t20100 commented Nov 18, 2024

EdgarGF93 commented Nov 26, 2024

silx.gui: support imageaggregate in _plot2d #4174

silx.gui: support imageaggregate in _plot2d #4174

Conversation

EdgarGF93 commented Sep 19, 2024 • edited Loading

EdgarGF93 commented Sep 19, 2024 • edited Loading

t20100 left a comment

Choose a reason for hiding this comment

t20100 Sep 23, 2024

Choose a reason for hiding this comment

EdgarGF93 commented Sep 26, 2024 • edited Loading

t20100 left a comment

Choose a reason for hiding this comment

EdgarGF93 commented Oct 6, 2024 • edited Loading

t20100 left a comment

Choose a reason for hiding this comment

t20100 Oct 7, 2024

Choose a reason for hiding this comment

t20100 Oct 7, 2024

Choose a reason for hiding this comment

t20100 Oct 22, 2024

Choose a reason for hiding this comment

t20100 Oct 22, 2024

Choose a reason for hiding this comment

EdgarGF93 commented Oct 27, 2024 • edited Loading

t20100 commented Nov 4, 2024

t20100 commented Nov 18, 2024

EdgarGF93 commented Nov 26, 2024

EdgarGF93 commented Sep 19, 2024 •

edited

Loading

EdgarGF93 commented Sep 19, 2024 •

edited

Loading

EdgarGF93 commented Sep 26, 2024 •

edited

Loading

EdgarGF93 commented Oct 6, 2024 •

edited

Loading

EdgarGF93 commented Oct 27, 2024 •

edited

Loading