mantid
mantid copied to clipboard
Fix bugs when exporting MD Histo cuts workspaces in sliceviewer
Description of work
This PR fixes several bugs that happen when exporting 3D MD Histo workspaces in sliceviewer. For more details check #36084
Fixes #36084.
Further detail of work
The issue was that bin_params is None when the workspace is MD Histo workspace. I handled this case by setting the bin width to the dim bin width when the bin_params is None.
To test:
Follow #36084
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
- Is the code of an acceptable quality?
- Does the code conform to the coding standards?
- Are the unit tests small and test the class in isolation?
- If there is GUI work does it follow the GUI standards?
- If there are changes in the release notes then do they describe the changes appropriately?
- Do the release notes conform to the release notes guide?
Functional Tests
- Do changes function as described? Add comments below that describe the tests performed?
- Do the changes handle unexpected situations, e.g. bad input?
- Has the relevant (user and developer) documentation been added/updated?
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.
This is still in draft so I assume that there are more commits to come.
Can you keep an eye on the Doxygen build? It is not failing if there are warnings and currently there are 7 on this build. Those for LoadBankFromDiskTask.cpp and WorkspaceUtils.cpp are being dealt with separately but I don't recognise - as one of the ones being resolved. It may therefore be specific to this PR.
Actually it looks like it is affecting several PRs so is not being caused by this one. I've raised #37286 so you no longer need to deal with it as part of this PR.
I couldn't reproduce 2 after multiple tries. The exporting is working fine using all types of cuts. Could you please send me the file you used to produce 2 or a different script than the one in the issue?
Just had another look at this - so exporting cuts does work if you use the rectangle ROI tool, but if you just open line plots (i.e. cuts through the data at the cursor position) i get the error I posted above
SliceViewerModel.export_cuts_to_workspace_mdevent() missing 1 required positional argument: 'cut'
I'll keep digging into it, but at least hopefully you can reproduce this now!
So the bug is here in export_pixel_cut_to_workspace_md
https://github.com/mantidproject/mantid/blob/42f0fb5940f8197a354d834668502bf740644d4d/qt/python/mantidqt/mantidqt/widgets/sliceviewer/models/model.py#L477
If this returns self.export_cut_to_workspace then we at least get one step closer - now there is a missing argumentdimensions_indices (despite what the error says).
If you add in dimension_indices=data_view.dimensions.get_states() in the call to self.model.export_pixel_cut_to_workspace here
https://github.com/mantidproject/mantid/blob/42f0fb5940f8197a354d834668502bf740644d4d/qt/python/mantidqt/mantidqt/widgets/sliceviewer/presenters/presenter.py#L292
and alter the function definitions in the model accordingly you no longer get an error but the behaviour isn't quite right.
For an MDEvent workspace it seems to get the integration limits as double the bin width (i.e. twice what it should be), for an MDHisto workspace the cut is only 2 bins wide (i.e. 3 bins included along the axis of the cut as opposed to the limits of the data).
Hope that helps!