Repository navigation
Conversation
swu4bnl
commented
Oct 21, 2026
swu4bnl
commented
- Enhance the color scale and color map control sync mechanism
- Enhance the color scale and color map control sync mechanism
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 527ed00c9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if "cmap" in settings or "scale" in settings: | ||
| for i in range(self.tab_widget.count()): | ||
| tab = self.tab_widget.widget(i) | ||
| if hasattr(tab, "on_plot_style_changed"): | ||
| tab.on_plot_style_changed() |
There was a problem hiding this comment.
The image-tab publishers pass the complete ImageDisplaySettings mapping, so this condition is true even when the user changes only intensity limits or overlay visibility. Each such event consequently invokes on_plot_style_changed() on every reduction and transform tab, rebuilding and drawing potentially large Matplotlib figures; histogram dragging can trigger this continuously. The loop also includes source_tab, causing genuine transform style changes to redraw the initiating plot here and again in its caller. Compare changed values and skip the source before notifying plot tabs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Shared appearance propagation introduces colormap compatibility failures, stale recipe state, and unnecessary synchronous redraws.
4 open issues
What changed in this PR
This PR unifies SciView’s image and plot appearance settings and improves colormap handling for existing SciAnalysis thumbnail support.
Changes:
- Centralizes appearance state and synchronizes colormap and intensity-scale controls across tabs.
- Converts thumbnail colormap names into callable Matplotlib colormaps.
- Documents shared appearance behavior and adds release-writing guidance.
| File | Description |
|---|---|
tabs/transform_tab.py |
Connects Transform controls to shared colormap and scale settings. |
src/sciview/settings/plot_style.py |
Adds canonical appearance settings and a legacy mapping adapter. |
src/sciview/settings/app_settings.py |
Derives display defaults from the appearance model. |
src/sciview/processing/batch.py |
Resolves thumbnail colormaps for SciAnalysis’s exporter. |
main.py |
Owns shared appearance state and propagates updates. |
docs/UI_CONTROL_ARCHITECTURE.md |
Documents shared colors, snapshots, and independent intensity limits. |
.github/skills/write-release-updates/SKILL.md |
Adds guidance for concise, user-focused release updates. |
Review effort: Balanced
| if "cmap" in settings or "scale" in settings: | ||
| for i in range(self.tab_widget.count()): | ||
| tab = self.tab_widget.widget(i) | ||
| if hasattr(tab, "on_plot_style_changed"): | ||
| tab.on_plot_style_changed() |
| tab.on_plot_style_changed() | ||
| style = style if isinstance(style, PlotStyle) else PlotStyle.from_dict(style) | ||
| self.appearance_settings = replace(self.appearance_settings, **style.to_dict()) | ||
| self.publish_shared_display_settings({"cmap": style.colormap}, source_tab=source_tab) |
| self.parent_app.show_status(tr("transform.status.invalid_plot_style", error=exc)) | ||
| return | ||
| self.update_display_settings( | ||
| cmap=style.colormap, |
| self.plot_scale_combo.setCurrentText(self.display_settings["scale"]) | ||
| self.plot_dpi_spin.setValue(style.dpi) | ||
| finally: | ||
| self._updating_plot_style_controls = False |
