-
Notifications
You must be signed in to change notification settings - Fork 80
silx.gui.data.NXdataWidgets.ArrayImagePlot: Allow to change axes for nxdata stack of images #3624
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
Conversation
|
As it is, it is not working because then |
|
Moving this to next release. It's probably worth merging "Image" and "Stack" views at the same time. |
|
Yes, please! |
65b9509 to
09459e3
Compare
a5068f4 to
a6280da
Compare
| self._axesSelector = NumpyAxesSelector(self) | ||
| self._axesSelector.setNamedAxesSelectorVisibility(False) | ||
| self._axesSelector.selectionChanged.connect(self._updateImage) | ||
| self._axesSelector.selectedAxisChanged.connect(self._updateImageAxes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there are two updating methods:
self._updateImageis called when the slider value changes (the axes stay the same)self._updateImageAxesis the new method called when the displayed axis changes. It takes care of resetting the plot properly before slicing the image accordingly.
In normal operation, self._updateImage will always be preceded by a call to self._updateImageAxes.
|
@t20100 Now ready for review |
src/silx/gui/data/NXdataWidgets.py
Outdated
| signals: list[h5py.Dataset], | ||
| axes: list[h5py.Dataset] | None = None, | ||
| signals_names: list[str] | None = None, | ||
| axes_names: list[str] | None = None, | ||
| axes_scales: list[Literal["linear", "log"] | None] | None = None, | ||
| title: str | None = None, | ||
| isRgba: bool = False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This class is not reference in the doc and from a quick search the only place where it is used is:
https://github.com/silx-kit/pymca/blob/a07233366d76ad8e490a48e5b800493ee9db77be/src/PyMca5/PyMcaGui/io/hdf5/Hdf5NodeView.py#L139 and it should not be impacted by this change, so OK for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I didn't really think about it but do you wish to go through a proper deprecation step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I think it is OK to change the API here.
As I said: This is not in the doc, doesn't seems to be used directly... and it's a major release.
src/silx/gui/data/NXdataWidgets.py
Outdated
| x_axis_index = axis_indices["X"] | ||
| y_axis_index = axis_indices["Y"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
X and Y are always available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If setImageData was called beforehand, yes.
And it should always be the case since the connection to the callback _updateImageAxes is done in setImageData. So if setImageData was not called, the _updateImageAxes slot is not connected to anything.
I will add additional checks for good measure though.
|
Thanks for having take care of this! |
f86d5da to
38e7bd1
Compare
|
@t20100 Should be ready for a second review |
|
LGTM! Since I opened the PR in the first place I cannot approve it through github 😀 |
|
Actually, I have to rework this a bit. It can raise errors for RGBA images |
38e7bd1 to
83468b8
Compare
Disabled the feature for RGBA images since it does not seem that useful: 83468b8 |
This needs more testing to make sure it works under all condition this widget is used