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
Add table view for group peak workspace #37166
base: main
Are you sure you want to change the base?
Conversation
d8ec136
to
47c076e
Compare
47c076e
to
cc1d261
Compare
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.
Thanks for this, not had chance to look through the code in detail - but have tried this out quickly. Overall it behaves well, sorting, plotting etc. just a couple of minor things:
- Sorting seems to work on the Qt view only - perhaps the user would expect
SortPeaksWorkspace
to be called on the individual tables (in common with the behaviour of other algorithms)? - Deleting a row in one of the tables works even if the combined table is open, and the view is updated which indicates the ADS observer is working if an algorithm operates on one of the constituent tables. However there is a crash if you call an algorithm (e.g.
FilterPeaks
) replacing the group itself (i.e.InputWorkspace=OutputWorkspace="group_ws"
)
FilterPeaks-[Notice] FilterPeaks started
Error occurred in handler:
Traceback (most recent call last):
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\observers\ads_observer.py", line 23, in wrapper
func(*args, **kwargs)
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\observers\ads_observer.py", line 70, in replaceHandle
self.presenter.replace_workspace(name, workspace)
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\workspacedisplay\table\presenter.py", line 165, in replace_workspace
self.presenter.model = TableWorkspaceDisplayModel(workspace)
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\workspacedisplay\table\model.py", line 63, in __init__
self.supports(ws)
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\workspacedisplay\table\model.py", line 55, in supports
raise ValueError("The workspace type is not supported: {0}".format(ws))
ValueError: The workspace type is not supported: peaks_out
FilterPeaks-[Notice] FilterPeaks started (child)
FilterPeaks-[Notice] FilterPeaks successful, Duration 0.00 seconds
FilterPeaks-[Notice] FilterPeaks started (child)
FilterPeaks-[Notice] FilterPeaks successful, Duration 0.00 seconds
FilterPeaks-[Notice] FilterPeaks successful, Duration 0.02 seconds. Processed as a workspace group
Python-[Error] Traceback (most recent call last):
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\workspacedisplay\table\table_model.py", line 127, in flags
editable = self._data_model.is_editable_column(col)
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\workspacedisplay\table\group_model.py", line 107, in is_editable_column
return self.get_column_headers()[icol] in self.EDITABLE_COLUMN_NAMES
File "c:\mantid-conda\mantid\qt\python\mantidqt\mantidqt\widgets\workspacedisplay\table\group_model.py", line 82, in get_column_headers
return ["Group Index", "WS Index"] + self.ws[0].getColumnNames()
RuntimeError: Variable invalidated, data has been deleted.
Python-[Fatal] Terminated by user.
It looks like FilterPeaks
is run on both workspaces, it's just a invalidated variable when updating the view?
3. There is a column called WS Index
- is this necessary? there is already a PeakNumber
column which some algorithms (e.g. PredictPeaks
) populate
4. Small thing, but if you double-click on a table workspace it opens the view, this doesn't seem to work for the group workspace, you have to right-click > Show data
Thanks for the feedback.
|
ec78a38
to
ffa6d39
Compare
ffa6d39
to
86ba65d
Compare
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.
Thanks for the fixes - running FilterPeaks
on the group peak workspace works now.
Just a couple new issues though!
a. Double-clicking on a group of peaks workspaces opens the table view, but now double-clicking on a single peaks workspace prints this error to the log
The workspace type is not supported: peaks_ub0
Could not open workspace: peaks_ub0 with neither MatrixWorkspaceDisplay nor TableWorkspaceDisplay.
The table view does open though!
b. There is an issue sorting the columns with the group table view open - it seems to update the view with the first table only and the column WS Index
re-appears.
Thanks for the review: |
Description of work
This pull request introduces a new table view for group peaks workspaces. The new group table view will display the same data as the table view for single peak workspaces but with additional columns for the index of the peak workspace in the group and the index of the spectrum in the peak workspace. Moreover, it has the same features as the single peak workspace table view like sorting, deleting, copying, and plotting.
Fixes #36592.
Further detail of work
The work is implemented without adding a new data structure for the group peak workspace as it is not needed and all of the table view operations can be done with the existing data structure. The new view has been implemented by extending the existing table display MVP classes with new presenters and models for the group workspace.
To test:
Load and Group Workspaces:
Verify Group Data View:
Interact with the Table View:
Reviewer
Please comment on the points listed below (full description).
Your comments will be used as part of the gatekeeper process, so please comment clearly on what you have checked during your review. If changes are made to the PR during the review process then your final comment will be the most important for gatekeepers. In this comment you should make it clear why any earlier review is still valid, or confirm that all requested changes have been addressed.
Code Review
Functional Tests
Does everything look good? Mark the review as Approve. A member of
@mantidproject/gatekeepers
will take care of it.Gatekeeper
If you need to request changes to a PR then please add a comment and set the review status to "Request changes". This will stop the PR from showing up in the list for other gatekeepers.