Skip to content

Commit 42d4e32

Browse files
committed
image roi and line plot widget right click menu is only semi working
1 parent 31e36ff commit 42d4e32

1 file changed

Lines changed: 163 additions & 0 deletions

File tree

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
# LinePlotWidget Implementation Plan
2+
3+
## 1. Scope and Files
4+
5+
- **New file:** `image_line_widget/line_plot_widget.py` — contains `LinePlotWidget` and plot-dict / relayout logic.
6+
- **Existing:** `models.py` — add `LineConfig` next to `ImageRoiConfig`; reuse `AxisEvent` and `AxisEventType` for `on_axis_change`.
7+
- **Dependencies:** `nicegui` 3.7.1, `numpy`; all plot changes via the plotly dict and `self.plot.update()` (no `plotly.graph_objects` or high-level Figure APIs).
8+
9+
---
10+
11+
## 2. Design: Inheritance vs Member
12+
13+
**Recommendation: inherit from `ui.column` (same pattern as ImageRoiWidget).**
14+
15+
- **ImageRoiWidget** is `class ImageRoiWidget(ui.column)`: it calls `super().__init__()`, sets `self.classes(...)`, then builds a toolbar row and a single `ui.plotly` as direct children. The widget is the column; placing `ImageRoiWidget(...)` in a layout embeds the whole block.
16+
- **LinePlotWidget** has a simpler UI (one plot, no toolbar in the spec). Keeping the same pattern gives:
17+
- Consistent embedding: `with ui.column(): widget = LinePlotWidget(...)` is unnecessary; `widget = LinePlotWidget(...)` is the column and can be used in `ui.row`, `ui.column`, etc.
18+
- Same mental model as ImageRoiWidget for future toolbar/controls (e.g. legend toggles) and for testing.
19+
- **Alternative (member):** e.g. `self.container = ui.column(); self.plot = ui.plotly(...)` inside it. Then the caller must hold the container to embed the widget. That diverges from ImageRoiWidget and makes “drop-in” usage slightly less direct.
20+
21+
**Conclusion:** Implement `LinePlotWidget(ui.column)`, with the plotly element as the only child (or first child if later we add a small toolbar). No separate container attribute for the root.
22+
23+
---
24+
25+
## 3. LineConfig
26+
27+
- **Location:** Define in `models.py` next to `ImageRoiConfig`.
28+
- **Definition:** As specified:
29+
- `line_width: int = 2`
30+
- `line_color: str = 'blue'`
31+
- **Usage:** One global default from `__init__(cfg=LineConfig())`. Per-line config optional in `add_line`/`update_line` via `config: LineConfig | None`; when `None`, use the widget’s default `cfg`.
32+
33+
---
34+
35+
## 4. Data Model and plot_dict
36+
37+
- **Trace list:** `self.plot_dict['data']` is a list of Plotly trace dicts. Each trace is a line: `type: 'scatter'`, `mode: 'lines'`, `x`, `y`, `name`, and optional `line: { width, color }`, `visible`. Traces may be attached to the left y-axis (default) or right y-axis via trace `yaxis: 'y'` (default) or `yaxis: 'y2'`.
38+
- **Right y-axis:** When a trace uses the right axis, set trace `yaxis: 'y2'`. Ensure `self.plot_dict['layout']['yaxis2']` exists with e.g. `overlaying: 'y'`, `side: 'right'`, `anchor: 'x'`. Create `yaxis2` on first use when adding a line with `y_axis='right_y_axis'`.
39+
- **Name → index:** Keep a mapping `self._name_to_trace_index: Dict[str, int]` from line name to index in `self.plot_dict['data']`, so CRUD and `toggle_visible` can find the trace without scanning. Update this map on add/delete/clear (and on update_line if we ever support rename).
40+
- **Axis labels:** Single plot-level x and left-y axis titles. Store in layout: `self.plot_dict['layout']['xaxis']['title']`, `self.plot_dict['layout']['yaxis']['title']`. When `x_label`/`y_label` are provided in init/add_line/update_line, update these (last set wins). Right y-axis can have its own title in `layout.yaxis2.title` if needed later.
41+
- **Layout:** Include `dragmode: 'pan'`, `scrollZoom: True`, and config like `displayModeBar: False`, `editable: True` as in ImageRoiWidget so axis interaction works and relayout events fire.
42+
43+
---
44+
45+
## 5. Constructor and Seed Line
46+
47+
- **Signature:** Require `name: str` in `__init__` (caller always specifies at least `(x, y, name)` for any line). Fix typo in spec: `np.ndarray` (not `np.ndarry`).
48+
- **Seed line:** The first line is created from `x`, `y`, `name` (required). All lines are at least `(x, y, name)`.
49+
- **Steps in `__init__`:**
50+
1. `super().__init__()`, `self.classes('w-full h-full p-0')` (or similar).
51+
2. Store `x_label`, `y_label`, `cfg`, `on_axis_change`.
52+
3. Build `self.plot_dict` with one trace from `x`, `y`, `name` and layout with axis titles; apply `cfg` to the trace’s `line`; trace uses left y-axis by default.
53+
4. `self._name_to_trace_index = {name: 0}`.
54+
5. `self.plot = ui.plotly(self.plot_dict).classes('w-full grow')`.
55+
6. `self.plot.on('plotly_relayout', self._handle_plotly_relayout)`.
56+
- **Future:** Leave a short comment for a later `on_event` callback (e.g. line CRUD events), not implemented.
57+
58+
---
59+
60+
## 6. _handle_plotly_relayout and AxisEvent
61+
62+
- **Goal:** When the user changes x or y axis range (zoom/pan), emit `AxisEvent(AxisEventType.AXIS_CHANGE, x_range=..., y_range=...)` and call `on_axis_change(event)` if set.
63+
- **Pattern (from edit_roi_app and ImageRoiWidget):**
64+
- Handler receives `e` with `e.args` = relayout payload (dict).
65+
- Axis range can appear as `'xaxis.range'` (value `[min, max]`) or as `'xaxis.range[0]'` and `'xaxis.range[1]'`; same for y.
66+
- **Implementation sketch:**
67+
- If any key in `e.args` starts with `'xaxis.range'` or `'yaxis.range'`:
68+
- Compute `x_range`: if `'xaxis.range' in e.args` use `e.args['xaxis.range']`, else `[e.args['xaxis.range[0]'], e.args['xaxis.range[1]']]` (no `.get()`; fail fast if keys missing when we expect them).
69+
- Compute `y_range` analogously.
70+
- Optionally update `self.plot_dict['layout']['xaxis']['range']` and `yaxis` so dict stays in sync with what Plotly shows.
71+
- If `self.on_axis_change` is set, call `self.on_axis_change(AxisEvent(AxisEventType.AXIS_CHANGE, x_range=x_range, y_range=y_range))`.
72+
- No other relayout handling required for the minimal version (e.g. no shapes).
73+
- **Placeholder:** If we want a stub first, the function can only do the axis-range branch above and ignore other keys.
74+
75+
---
76+
77+
## 7. CRUD and Visibility API
78+
79+
- **add_line(x, y, name, x_label=None, y_label=None, config=None, y_axis: str = 'left_y_axis')**
80+
- If `name in self._name_to_trace_index`: log error and **overwrite** the existing line (replace that trace and keep same index; update `_name_to_trace_index` accordingly).
81+
- Otherwise append a new trace.
82+
- Trace: `name`, `x`, `y`, line style from `config or self.cfg`. If `y_axis == 'right_y_axis'`, set trace `yaxis: 'y2'` and ensure `layout.yaxis2` exists (create on first right-axis line: `overlaying: 'y'`, `side: 'right'`, `anchor: 'x'`).
83+
- Update plot-level axis titles if `x_label`/`y_label` provided (last set wins).
84+
- `self.plot.update()`.
85+
86+
- **delete_line(name: str)**
87+
- If `name not in self._name_to_trace_index`: raise (e.g. `KeyError` or `ValueError`).
88+
- Remove the trace at that index from `self.plot_dict['data']`, and rebuild `_name_to_trace_index` (indices shift).
89+
- If no traces remain on right axis, optionally remove `layout.yaxis2` (or leave it; leaving is simpler).
90+
- `self.plot.update()`.
91+
92+
- **update_line(name, x, y, x_label=None, y_label=None, cfg=None)**
93+
- Raise if `name not in self._name_to_trace_index`.
94+
- Replace the trace’s `x`, `y` and, if `cfg` provided, `line`; if `x_label`/`y_label` provided, update layout axis titles.
95+
- (Do not change y_axis side on update_line for now; optional future extension.)
96+
- `self.plot.update()`.
97+
98+
- **toggle_visible(name: str, visible: bool)**
99+
- Method name: `toggle_visible`.
100+
- Raise if `name not in self._name_to_trace_index`.
101+
- Set trace’s `visible` to `True` or `False` per Plotly convention; `self.plot.update()`.
102+
103+
- **clear_lines()**
104+
- Set `self.plot_dict['data'] = []`, `self._name_to_trace_index = {}`. Optionally clear `layout.yaxis2` if present.
105+
- `self.plot.update()`.
106+
107+
All mutations via `self.plot_dict` only; no defensive `.get()` for required keys; raise on invalid name or missing required keys.
108+
109+
**Right y-axis and events:** Axis change event emission (`on_axis_change`) focuses on the default left y-axis and bottom x-axis only. User interactions that change the right y-axis do not need to be reflected in existing event emission (x-axis linking with ImageRoiWidget is the primary use; callers connect signals/slots for that).
110+
111+
---
112+
113+
## 8. Set X-Axis and Y-Axis API
114+
115+
- **Intent:** Programmatic setting of axis range for linking: e.g. ImageRoiWidget and LinePlotWidget stacked vertically; caller connects LinePlotWidget’s `on_axis_change` to ImageRoiWidget’s `set_x_axis_range` (and vice versa) in a loose signal/slot style. Two separate methods.
116+
- **API:**
117+
- `set_x_axis_range(self, x_range: Sequence[float])` — set `self.plot_dict['layout']['xaxis']['range'] = list(x_range)`; then `self.plot.update()`. Do **not** invoke `on_axis_change` (programmatic set; avoid loops).
118+
- `set_y_axis_range(self, y_range: Sequence[float])` — set `self.plot_dict['layout']['yaxis']['range'] = list(y_range)`; then `self.plot.update()`. Do not invoke `on_axis_change`.
119+
- **Linking:** For now only x-axis linking is implemented between siblings; y-axis linking is out of scope.
120+
121+
---
122+
123+
## 9. Implementation Order
124+
125+
1. Add `LineConfig` in `models.py` next to `ImageRoiConfig`.
126+
2. Minimal `LinePlotWidget(ui.column)` in `line_plot_widget.py`: build `plot_dict` with one seed trace (required `name`), subscribe to `plotly_relayout`.
127+
3. Implement `_handle_plotly_relayout` for axis-range only and `on_axis_change` emission (left/bottom axes only).
128+
4. Implement add_line (with `y_axis='left_y_axis'` default and right y-axis support), delete_line, update_line, toggle_visible, clear_lines (with `_name_to_trace_index`). On add_line duplicate name: log and overwrite.
129+
5. Add `set_x_axis_range(x_range)` and `set_y_axis_range(y_range)` (no callback invocation).
130+
6. Optional: small demo script to embed the widget and test CRUD, axis callback, and x-axis linking.
131+
132+
---
133+
134+
## 10. Rules (from spec)
135+
136+
- No defensive “defaults” from dict: e.g. avoid `e.args.get('xaxis.range', default)` when we intend to fail if the key is missing; use direct access and let it raise.
137+
- Fail fast: raise on unknown line name, duplicate name on add (after logging), etc., as specified.
138+
139+
---
140+
141+
## 11. Out of Scope / Later
142+
143+
- `on_event` for line CRUD: not implemented; comment in `__init__` only.
144+
- Y-axis linking between widgets: not in scope; only x-axis linking (e.g. LinePlotWidget with ImageRoiWidget) for now.
145+
- Right y-axis in relayout/events: user changes to the right y-axis do not drive `on_axis_change`; events focus on default (left/bottom) axes.
146+
147+
---
148+
149+
## 12. Decisions (Answered)
150+
151+
| # | Question | Decision |
152+
|---|----------|----------|
153+
| 1 | Seed line name | Required `name: str` in `__init__`; caller always specifies at least `(x, y, name)`. |
154+
| 2 | add_line when name exists | Log and **overwrite** the existing line. |
155+
| 3 | set_axis_range API | Two methods: `set_x_axis_range(x_range)` and `set_y_axis_range(y_range)`. Programmatic set does **not** invoke `on_axis_change`. X-axis linking with ImageRoiWidget (vertically stacked) via caller connecting signals/slots. |
156+
| 4 | Axis labels scope | Only update single plot-level x/y axis title (last set wins). |
157+
| 5 | toggle_visible spelling | Use method name `toggle_visible`. |
158+
| 6 | LineConfig location | In `models.py` next to `ImageRoiConfig`. |
159+
160+
**Additional requirement — Right y-axis:**
161+
- `add_line(..., y_axis: str = 'left_y_axis')`. Allowed value for right axis: `'right_y_axis'`.
162+
- Plotly supports this via dict: trace `yaxis: 'y2'` and layout `yaxis2: { overlaying: 'y', side: 'right', anchor: 'x' }`. Implement it; no TODO.
163+
- Do **not** change existing event emission for right-axis interaction; `on_axis_change` remains focused on default (left/bottom) axes.

0 commit comments

Comments
 (0)