Interactive plots and other features #121
Conversation
paracini
commented
Feb 24, 2025
- Interactive plot commands. TO FIX: LogY toggle does not work due to workarounds used to handle plopp plots
- R*Q^4 toggle. TO FIX: handling plopp required workarounds, there are probably better ways to implement this
- individual datasets toggles to hide curves in plotting. TO FIX: error bars are not correctly handled and remain visible
- 'Remove plot' button to remove appended plots. TO FIX: removing a plot also removes all the earlier plots in the
- Added comment box above each plot for annotations
…opy of selected row in Autoreduction table, blank if none selected, moved add/remove row buttons to Manual table
…comment box. contains workarounds, needs better handling of plopp, LogY toggle not working
| def log_progress(self, progress): | ||
| self.progress_log.children = (progress,) | ||
|
|
||
| def log_plot(self, plot): |
There was a problem hiding this comment.
I think we need to take a good look at the code in this function, see if we can do things differently, or in a more compact or efficient way.
There seem to be a lot of workarounds in here.
It would be nice to get an explanation, or a live demo, to see what features are needed.
At the moment, this is all very difficult to read and will be hard to maintain.
src/ess/reflectometry/gui.py
Outdated
| # Create unique ID for this plot group - currently unused | ||
| self.plot_counter += 1 | ||
|
|
||
| # Convert any existing interactive plots to static |
There was a problem hiding this comment.
Why do we need to convert interactive plots to static ones?
src/ess/reflectometry/gui.py
Outdated
| original_draw = plot.view.canvas.draw | ||
|
|
||
| # Define all helper functions first | ||
| def calculate_plot_limits(current_plot, is_transformed=False): |
There was a problem hiding this comment.
If possible, we should rely on plopp to do this for us.
src/ess/reflectometry/gui.py
Outdated
| # Now override the methods | ||
| plot.view.canvas.reset_mode = custom_reset_mode | ||
| plot.view.canvas.zoom = custom_zoom | ||
| plot.view.canvas.panzoom = custom_panzoom | ||
| plot.view.canvas.draw = custom_draw |
There was a problem hiding this comment.
Without having thought too much about it, this feels dangerous.
src/ess/reflectometry/gui.py
Outdated
| plot.view.canvas.draw = custom_draw | ||
|
|
||
| # Create and populate legend controls | ||
| def create_legend_controls(): |
There was a problem hiding this comment.
This seem like more GUI features have crept it, and keeping everything in sync is becoming more and more difficult.
|
I agree, I wasn't sure whether to make the pull request at this point, maybe it's better to just do a demo and discuss. There are a lot of workarounds that complicate things at this stage |
Extract parts of the gui into separate components
nvaytet
left a comment
There was a problem hiding this comment.
Looking good now 👍
I would like to see some type-hints and docstrings (with Parameters) on all functions.
It would also be nice to have some unit tests of basic functionality, such as simply generating the gui, and loading a file and running if possible.
| Known limitations: | ||
| 1. Remove plot button behavior is inconsistent: | ||
| - Removes the target plot with its controls | ||
| - Previous plots disappear but their control buttons remain | ||
| - Previous plots maintain interactivity despite attempted conversion to static | ||
| 2. Dataset toggle does not affect error bars as they are separate matplotlib artists | ||
| 3. Remove row button removes last row instead of selected row | ||
| 4. LogY toggle doesn't work due to workarounds for plopp's axis behavior: | ||
| - Plopp's autoscale was flipping the y-axis orientation | ||
| - We override multiple plopp/matplotlib methods to maintain correct orientation | ||
| - This prevents the LogY toggle from working as it would interfere with our fixes |
There was a problem hiding this comment.
Unless I am mistaken, I think we fixed all these with @jokasimr ?
There was a problem hiding this comment.
Yes these are all fixed!