Maskscatter into roi tool layout by sergey-yaroslavtsev · Pull Request #1223 · silx-kit/pymca

sergey-yaroslavtsev · 2026-06-18T19:50:00Z

The generalized solution discussed in #1219

sergey-yaroslavtsev · 2026-06-19T14:17:39Z

Some summary:

New "Scatter plot" checkbox in the HDF5 stack wizard - if ticked - initiates MaskScatterView to present the stack
Wizard now validates axis/signal sizes - notify about padding and wrong selection
Add image in scatter mode reuse regular grid guess from silx to push image to RGBcorrelator
MaskScatterView now is standalone widget (with few new methods) - to be called in plugin and in the layout (ROI tool)

Extra:

fix bug shown in Roi axis stack #1219 (mcaInde vs self.info["McaIndex"])
fix small bug in QNexusWidget
HDF5Stack1D can handle a "normal" scan - when data is presented as N x Ch and motors are arrays of length i and j where N=i*j (see comment below)

Things I failed to do better:

HDF5Stack1D now also have padding mechanism - it is hard to move it to QStackWidget where we introduced it before for non scatter mode.
Padding mechanism is reused for N x Ch vs i and j which is potentially fragile logic if used somewhere else. Currently it seems not to brake anything. But if you like we can Either remove padding from this case - limiting flexibility. Or add a flag from wizard which will complicates the logic...

sergey-yaroslavtsev · 2026-06-19T17:57:29Z

Actually, to keep padding in one place it can go deep to DataObject because both HDF5Stack1D and QStackWidget inherits it.

Do you think it is cleaner logic?

But if you like we can Either remove padding from this case - limiting flexibility. Or add a flag from wizard which will complicates the logic...

please also let me know your thoughts about this one.

After these two resolved i think this PR can be finalized.

sergey-yaroslavtsev · 2026-06-19T20:54:04Z

The last commit:

move padding to DataObject
add protector
fix "normal" scan with reversed order (why not to)
add extra validation for scatter mode (should be only 2 dimensions)

If you do not like it we can remove the last commit.

woutdenolf

I added some comments. In addition, just like I started doing for silx , I think it is very helpful to reviewer to produce a MWE, especially when it comes to GUI features.

I do not see what you see in the screenshot of ROI maging:

Screencast.from.2026-06-21.14-16-12.webm

This is the demo code. AI can help you generate code fast. Useful for unit tests as well.

import sys

import h5py
import numpy
from silx.gui import qt

from PyMca5.PyMcaGui.pymca.PyMcaPostBatch import PyMcaPostBatch
from PyMca5.PyMcaGui.pymca.QStackWidget import QStackWidget
from PyMca5.PyMcaIO.HDF5Stack1D import HDF5Stack1D


def create_hdf5(filename="demo_scan.h5"):
    nx, ny = 20, 15
    npoints = nx * ny
    nchannels = 200
    order = "C"  # X fast axis
    # order = "F"  # Y fast axis

    x = numpy.linspace(0, 10, nx)
    y = numpy.linspace(0, 5, ny)
    x, y = numpy.meshgrid(x, y)
    x = x.ravel(order=order)
    y = y.ravel(order=order)

    energy = numpy.linspace(0, 10, nchannels)
    mca = numpy.zeros((npoints, nchannels), dtype=numpy.float32)

    # Data which makes it clear if the user gets the shape wrong
    for i in range(npoints):
        xv = x[i]
        yv = y[i]

        center = 3.0 + 0.4 * xv + 0.8 * yv
        peak1 = numpy.exp(-((energy - center) ** 2) / (0.8 + 0.02 * xv))

        peak2 = 0.4 * numpy.exp(-((energy - (7.5 - 0.3 * yv)) ** 2) / 1.5)

        amplitude = 1.0 + 0.5 * numpy.sin(xv * 0.8) + 0.2 * yv
        drift = 0.02 * (xv - yv)

        mca[i] = amplitude * (peak1 + peak2) + drift

    # CTRL-C of scan: datasets have different lengths
    x = random_trim(x)
    y = random_trim(y)
    mca = random_trim(mca)

    with h5py.File(filename, "w") as f:
        pymcaroitool = f.create_group("pymcaroitool")
        pymcaroitool.attrs["NX_class"] = "NXdata"
        pymcaroitool.attrs["mca00"] = "mca"
        pymcaroitool.attrs["axes"] = ["y", "x"]  # ESRF scatter convention, not NeXus
        pymcaroitool.create_dataset("x", data=x)
        pymcaroitool.create_dataset("y", data=y)
        pymcaroitool.create_dataset("energy", data=energy)
        pymcaroitool.create_dataset("mca00", data=mca)

        pymcapostbatch = f.create_group("pymcapostbatch")
        pymcapostbatch.attrs["NX_class"] = "NXdata"
        pymcapostbatch.attrs["signal"] = "roi00"
        pymcapostbatch.attrs["auxiliary_signals"] = ["roi01", "roi02"]
        pymcapostbatch.attrs["axes"] = ["y", "x"]  # ESRF scatter convention, not NeXus
        pymcapostbatch.create_dataset("x", data=x)
        pymcapostbatch.create_dataset("y", data=y)
        pymcapostbatch.create_dataset("roi00", data=mca[:, 100])
        pymcapostbatch.create_dataset("roi01", data=mca[:, 120])
        pymcapostbatch.create_dataset("roi02", data=mca[:, 130])

        f.attrs["default"] = "pymcapostbatch"

    if order == "C":
        message = f"""
HDF5 created: {filename}
Number of rows: {ny}
Number of columns: {nx}
X is fast axis
"""
    else:
        message = f"""
HDF5 created: {filename}
Number of rows: {nx}
Number of columns: {ny}
Y is fast axis
"""

    print(message)


def random_trim(array):
    max_trim = max(1, int(0.05 * len(array)))
    ntrim = numpy.random.randint(1, max_trim + 1)
    return array[:-ntrim]


def launch_pymcaroitool(filename):
    app = qt.QApplication.instance() or qt.QApplication(sys.argv)

    selection = {
        "x": [],
        "y": ["/pymcaroitool/mca00"],
        "index": -1,
        "scatter": True,
        "allowPadding": True,
    }

    stack = HDF5Stack1D([filename], selection)

    w = QStackWidget()
    w.setStack(stack)

    w.resize(900, 700)
    w.move(100, 100)

    w.show()
    app.exec()


def launch_pymcapostbatch(filename):
    app = qt.QApplication.instance() or qt.QApplication(sys.argv)

    w = PyMcaPostBatch()
    w.addFileList([filename])

    w.resize(900, 700)
    w.move(100, 100)

    w.show()
    app.exec()


if __name__ == "__main__":
    import logging

    logging.basicConfig(level=logging.INFO)

    filename = "demo_scan.h5"
    create_hdf5(filename)
    launch_pymcaroitool(filename)
    launch_pymcapostbatch(filename)

woutdenolf · 2026-06-21T09:46:29Z

+
+
+        except Exception:
+            print("WARNING: Fail to identify number of scan points and/or axes sizes")


Suggested change

print("WARNING: Fail to identify number of scan points and/or axes sizes")

_logger.warning("Fail to identify number of scan points and/or axes sizes")

This module use print everywhere. Should I change all of them then to keep consistency?

Well if it says print("WARNING: I would say that is a clear _logger.warning(

Production code very rarely uses print. Up to you.

woutdenolf · 2026-06-21T10:16:05Z

                        dataset = xDatasetList[i].reshape(-1)
                        datasize = self.data.shape[i]
-                        if i == mcaIndex:
+                        if i == self.info["McaIndex"]:


mcaIndex vs. self.info["McaIndex"] is beyond my understanding like in the previous PR so at leave it at your discretion.

woutdenolf · 2026-06-21T12:27:20Z

Modify the X and Y axis values in the demo and run the script. It does not seem like the axes are taken into account.

x = numpy.linspace(0, 10, nx) + 50
y = numpy.linspace(0, 5, ny) - 10

Also, the checkbox is not checked when the window pops up although

selection = {
    "x": [],
    "y": ["/pymcaroitool/mca00"],
    "index": -1,
    "scatter": True,
    "allowPadding": True,
}

sergey-yaroslavtsev · 2026-06-22T11:52:04Z

I added some comments. In addition, just like I started doing for silx , I think it is very helpful to reviewer to produce a MWE, especially when it comes to GUI features.

I do not see what you see in the screenshot of ROI maging:

Do you want MWE as a video or as a code?

sergey-yaroslavtsev · 2026-06-22T12:05:12Z

MWE. I used generated data. First time I've selected per-point motor positioners (both with dimensions =15). Second time I've selected regular motors (dimensions of 3 and 5).

Screen.Recording.2026-06-22.140152.mp4

sergey-yaroslavtsev · 2026-06-22T13:16:29Z

selection = {
"x": [],
"y": ["/pymcaroitool/mca00"],
"index": -1,
"scatter": True,
"allowPadding": True,
}

Is it useful to have possibility to use scatter plot without positioners? My logic was that it should not happen.
Currently if there are no x-axes the scatter plot can not be shown - the idea was to use scatter plot only if "positioners" are provided. That is one of the reasons why validation happens in the wizard.

sergey-yaroslavtsev · 2026-06-22T16:04:05Z

Fix all requested by @woutdenolf
But keep the logic for axes - if none are provided then scatter mode can not be used.

woutdenolf · 2026-06-22T16:19:30Z

Do you want MWE as a video or as a code?

Both. At least code. Add a screenshot when the problem/feature is static. Add a screen recording when the problem/feature is dynamic.

woutdenolf

Nice work!

Some tweaking of the messages shown to the user but LGTM.

woutdenolf · 2026-06-22T16:30:39Z

        self.mainLayout.addWidget(self.stackIndexWidget, 0)

+        self._scatterCheckBox = qt.QCheckBox(
+            "Scatter plot (X, Y coordinates are set per image point)", self)


When "1D is last dimension" this checkbox toggles between

Grid (False): x(Nx), y(Ny), data(Ny, Nx, Nchan)

Scatter (True): x(Npoints), y(Npoints), data(Npoints, Nchan)

Perhaps this is better?

"Scatter plot (X, Y coordinate per data point)"

Not sure. Up to you. I just though "image point" was trying to mix two concepts: "image pixel" with "data point".

After discussion: "Scatter plot (X, Y coordinate per 1D data)" since the toggle boxes above also use "1D data".

woutdenolf · 2026-06-22T16:36:40Z

+                "Most probably the selected motor positions hold one value per scan point."
+                "The regular grid can not be defined."
+                "Enable 'Scatter plot' or select different axes.")


Suggested change

"Most probably the selected motor positions hold one value per scan point."

"The regular grid can not be defined."

"Enable 'Scatter plot' or select different axes.")

"Most probably the selected motor positions hold one value per scan point. "

"The regular grid can not be defined. "

"Enable 'Scatter plot' or select different axes.")

Needs spaces.

Perhaps also harmonize with "Scatter plot (X, Y coordinates are set per image point)".

You use the terms "image point" and "scan point" for the same thing. In my suggestion about I called it "data point".

woutdenolf · 2026-06-22T16:37:38Z

+        if scatter and not all(size == axisSize for size in axisSizes):
+            self.showMessage(
+                "Axes should have same number of positions "
+                "(as they hold one value per scan point)")


"scan point" -> to be harmonized with the others.

Perhaps suggest unchecking the scatter checkbox or select other axes? Like you did in another message.

woutdenolf · 2026-06-22T16:38:02Z

+                "dimension besides the channels).")
+            return False
+
+        # the axes hold one value per scan point so they must have equal sizes


"scan point" -> to be harmonized with the others.

a try to implement maskscatter in roi tool layout

925f0ee

sergey-yaroslavtsev added the work in progress label Jun 18, 2026

organizing

cb9cdf9

sergey-yaroslavtsev requested review from vasole and woutdenolf June 19, 2026 14:20

cleaner not clearer

afb555e

sergey-yaroslavtsev removed the work in progress label Jun 19, 2026

woutdenolf requested changes Jun 21, 2026

View reviewed changes

separate long method, fix buttons case, clean grammer

d810d5a

woutdenolf self-requested a review June 22, 2026 16:42

woutdenolf approved these changes Jun 22, 2026

View reviewed changes



		except Exception:
		print("WARNING: Fail to identify number of scan points and/or axes sizes")

	print("WARNING: Fail to identify number of scan points and/or axes sizes")
	_logger.warning("Fail to identify number of scan points and/or axes sizes")

Conversation

sergey-yaroslavtsev commented Jun 18, 2026

Uh oh!

sergey-yaroslavtsev commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-yaroslavtsev commented Jun 19, 2026

Uh oh!

sergey-yaroslavtsev commented Jun 19, 2026

Uh oh!

woutdenolf left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

woutdenolf commented Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-yaroslavtsev commented Jun 22, 2026

Uh oh!

sergey-yaroslavtsev commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergey-yaroslavtsev commented Jun 22, 2026

Uh oh!

sergey-yaroslavtsev commented Jun 22, 2026

Uh oh!

woutdenolf commented Jun 22, 2026

Uh oh!

woutdenolf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sergey-yaroslavtsev commented Jun 19, 2026 •

edited

Loading

woutdenolf left a comment •

edited

Loading

woutdenolf commented Jun 21, 2026 •

edited

Loading

sergey-yaroslavtsev commented Jun 22, 2026 •

edited

Loading