Bugs in exporting cuts from MDHisto workspaces in sliceviewer

[RichardWaiteSTFC]

Describe the bug Found during manual testing #36061

Two bugs in the same area:

• [x] When exporting ROI cuts in MDHisto workspace: 'NoneType' object is not subscriptable (Resolved in #37271)

• [ ] When exporting line cut plots for MDHisto workspace: SliceViewerModel.export_cuts_to_workspace_mdevent() missing 1 required positional argument: 'cut'

Happens whether original MDEvent workspace exists or not.

To Reproduce (1) Make the workspace md_3D_histo

md_4D = CreateMDWorkspace(Dimensions=4, Extents=[0,2,-1,1,-1.5,1.5,-0.25,0.25], Names="H,K,L,E", Frames='HKL,HKL,HKL,General Frame',Units='r.l.u.,r.l.u.,r.l.u.,meV')
FakeMDEventData(InputWorkspace=md_4D, UniformParams='5e5') # 4D data
FakeMDEventData(InputWorkspace=md_4D, UniformParams='1e5,0.5,1.5,-0.5,0.5,-0.75,0.75,-0.1,0.1') # 4D data

# Add a non-orthogonal UB
expt_info = CreateSampleWorkspace()
md_4D.addExperimentInfo(expt_info)
SetUB(Workspace='md_4D', c=2, gamma=120)

md_3D_histo = BinMD(InputWorkspace='md_4D', AlignedDim0='H,-2,2,100', AlignedDim1='K,-1,1,100', AlignedDim2='L,-1.5,1.5,100')

(2) Open md_3D_histo in sliceviewer (3) Export cuts from line plot and rectangular selection/ROI tools

Follow steps 3 c-f here https://developer.mantidproject.org/Testing/SliceViewer/SliceViewer.html#matrixworkspace

Expected behavior Would produce workspaces corresponding to the cuts

Screenshots

Platform/Version (please complete the following information):

• Seen in v6.7 so not a regression for 6.8

Additional context

[sf1919]

Adding the Reported by User label after discussions on #36852

[jhaigh0]

I could see where the problems was with issue number 2, but wasn't sure how to recreate.

For the first problem, when the model is set up, if the workspace is an MDHisto, self.export_pixel_cut_to_workspace is set to self.export_pixel_cut_to_workspace_md (the same as if the we type is and MDEvent). here

self.export_pixel_cut_to_workspace_md takes and uses bin_params which are gathered via data_view.dimensions.get_bin_params() here. get_bin_params() returns none in the case that the dimensions aren't binned, which is the case for MDHisto (I think?). So there is some undefined behaviour here, I might need @RichardWaiteSTFC to have a look and let me know what should be happening.

The actual error comes from _dimension_limits() in the model where it claims bin_params is an optional type, but there doesn't seem to be any guarding against it being none. In the previous function _roi_binmd_parameters() I can see a bin_params is not None check, so it looks like that possibility needs to be considered in _dimension_limits() too.

[sf1919]

@zjmorgan tagging you as you also flagged this issue up so you might want to be part of this discussion.

[RichardWaiteSTFC]

I could see where the problems was with issue number 2, but wasn't sure how to recreate.

For the first problem, when the model is set up, if the workspace is an MDHisto, self.export_pixel_cut_to_workspace is set to self.export_pixel_cut_to_workspace_md (the same as if the we type is and MDEvent). here

self.export_pixel_cut_to_workspace_md takes and uses bin_params which are gathered via data_view.dimensions.get_bin_params() here. get_bin_params() returns none in the case that the dimensions aren't binned, which is the case for MDHisto (I think?). So there is some undefined behaviour here, I might need @RichardWaiteSTFC to have a look and let me know what should be happening.

The actual error comes from _dimension_limits() in the model where it claims bin_params is an optional type, but there doesn't seem to be any guarding against it being none. In the previous function _roi_binmd_parameters() I can see a bin_params is not None check, so it looks like that possibility needs to be considered in _dimension_limits() too.

I haven't had a chance to look in detail, but the root of the problem at least for (2) might be that you can't call BinMD on an MDHisto - you have to use IntegrateMDHistoWorkspace - with different type of binning arguments?

[MohamedAlmaki]

I could see where the problems was with issue number 2, but wasn't sure how to recreate. For the first problem, when the model is set up, if the workspace is an MDHisto, self.export_pixel_cut_to_workspace is set to self.export_pixel_cut_to_workspace_md (the same as if the we type is and MDEvent). here self.export_pixel_cut_to_workspace_md takes and uses bin_params which are gathered via data_view.dimensions.get_bin_params() here. get_bin_params() returns none in the case that the dimensions aren't binned, which is the case for MDHisto (I think?). So there is some undefined behaviour here, I might need @RichardWaiteSTFC to have a look and let me know what should be happening. The actual error comes from _dimension_limits() in the model where it claims bin_params is an optional type, but there doesn't seem to be any guarding against it being none. In the previous function _roi_binmd_parameters() I can see a bin_params is not None check, so it looks like that possibility needs to be considered in _dimension_limits() too.

I haven't had a chance to look in detail, but the root of the problem at least for (2) might be that you can't call BinMD on an MDHisto - you have to use IntegrateMDHistoWorkspace - with different type of binning arguments?

As Jonathan said, the get_bin_params returns None value. It returns None because one of the dimensions doesn't support dynamic rebinning (calling multiple BinMD calls on the workspace) which its logic is defined here.

In this line get_bins_params will return None because one of the dimensions will be Dimension widget not a DimensionNonIntegrated widget as only DimensionNonIntegrated defines get_bins. The Dimension is defined DimensionNonIntegrated if can_rebin (the value returned by can_support_dynamic_rebinning) is True as you see in this line

So the solution, possibly, is either we instruct the users to do something to convert all workspace dimensions into non-integrated dimensions, or we handle the bins parameters for the generic Dimension.

The change has been introduced in #30328

[MohamedAlmaki]

First issue has been resolved in #37271