Ticket #7541 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Copy Workspace Name

Reported by: Owen Arnold Owned by: Owen Arnold
Priority: major Milestone: Release 3.0
Component: GUI Keywords: CORE
Cc: Blocked By:
Blocking: Tester: Keith Brown

Description

User has reported that the Copy workspace name option on the GUI doesn't always work as expected. Some ideas:

  • Are workspace types checked i.e. input != output
  • What if the inputworkspace property is not called InputWorkspace?

Attachments

AlgOne.py (648 bytes) - added by Owen Arnold 7 years ago.
AlgTwo.py (972 bytes) - added by Owen Arnold 7 years ago.
AlgThree.py (650 bytes) - added by Owen Arnold 7 years ago.

Change History

comment:1 Changed 7 years ago by Owen Arnold

Pascal mentioned that DiffractionFocusing is one of these offending algorithms.

Last edited 7 years ago by Owen Arnold (previous) (diff)

comment:2 Changed 7 years ago by Owen Arnold

Estimated Duration = 2 Days, Not really sure what's going on here, or how much stuff it affects.

comment:3 Changed 7 years ago by Owen Arnold

  • Status changed from new to inprogress

refs #7541. Fix problem with early exit.

This handler loops through the property dialogs looking for other property dialogs that correspond to input workspaces. If the loop finds an input workspace, then it immediately assumes that the workspace is suitable and exits the loop. The code block then may find that the workspace name returned is empty, but it's too late the loop has been exited, so there's no opportunity to inspect the rest of the properties. Fix is to apply the check for an empty workspace name within the loop.

Changeset: 01c88cae8bbe1fe96bf239ca45dd5a59250adc39

comment:4 Changed 7 years ago by Owen Arnold

  • Keywords CORE added

Tester.

On an old version of mantid try this,

  • Load a Matrix Workspace into MantidPlot
  • Open the DiffractionFocusing algorithm dialog in MantidPlot
  • Set the InputWorkspace to be the same as the loaded workspace
  • Now click the rename workspace button next to the OutputWorkspace property.
  • Notice that the OutputWorkspace box does not get filled!

After building with the new changes. Try the same thing as above, but notice that the OutputWorkspace property is correctly populated.

I'm changing sensitive functionality here. It would be wise to thoroughly test that the rename workspace option has not been broken.

comment:5 Changed 7 years ago by Owen Arnold

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:6 Changed 7 years ago by Russell Taylor

  • Status changed from verify to verifying
  • Tester set to Russell Taylor

comment:7 Changed 7 years ago by Russell Taylor

  • Status changed from verifying to reopened
  • Resolution fixed deleted

A couple of things:

  • It looks like you left in a debugging line: const std::string WSNAME = otherWidget->getValue().toStdString();, since WSNAME is not used.
  • For the DiffractionFocussing dialog, even after the fix if you select a workspace for the GroupingWorkspace property then it picks that one rather than the InputWorkspace, which is a bit unsatisfactory. I accept, though, that this may not be trivial to resolve (prefer an 'InputWorkspace' property if it exists - still not foolproof) so perhaps we just live with it.

comment:8 Changed 7 years ago by Owen Arnold

I think one way to solve this issue would be to collect all input workspace properties, and then use the one called "InputWorkspace" if it is available.

comment:9 Changed 7 years ago by Owen Arnold

  • Status changed from reopened to inprogress

refs #7541. Use candidate list.

Construct a list of candidate workspace properties, then filter those to find a preferred one.

Changeset: 5de5a7571afbb1c51c6438f8e78da45af3cfb121

Changed 7 years ago by Owen Arnold

Changed 7 years ago by Owen Arnold

Changed 7 years ago by Owen Arnold

comment:10 Changed 7 years ago by Owen Arnold

Tester:

Register the three attached algorithms. Then perform the following tests.

Run

a= CreateWorkspace(DataX='1,2,3',DataY='1,2,3',DataE='1,1,1',UnitX='TOF',Distribution='1')
AlgOneDialog(InputWorkspace=a)

Hitting 'Replace Input Workspace' button should take OutputWorkspace propety value from InputWorkspace property, which is 'a'

Run

a= CreateWorkspace(DataX='1,2,3',DataY='1,2,3',DataE='1,1,1',UnitX='TOF',Distribution='1')
b= CreateWorkspace(DataX='1,2,3',DataY='1,2,3',DataE='1,1,1',UnitX='TOF',Distribution='1')
AlgTwoDialog(WorkspaceA=a, WorkspaceB=a, InputWorkspace=b, WorkspaceC=a)

Hitting 'Replace Input Workspace' button should take OutputWorkspace propety value from InputWorkspace, which is 'b'

Run

a= CreateWorkspace(DataX='1,2,3',DataY='1,2,3',DataE='1,1,1',UnitX='TOF',Distribution='1')
AlgThreeDialog(WorkspaceA=a)

Hitting 'Replace Input Workspace' button should take OutputWorkspace property value from WorkspaceA property, which is 'a'

comment:11 Changed 7 years ago by Owen Arnold

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:12 Changed 7 years ago by Russell Taylor

  • Status changed from verify to verifying

comment:13 Changed 7 years ago by Russell Taylor

  • Status changed from verifying to reopened
  • Resolution fixed deleted

Looks like you haven't updated your topic branch on github - the commit in comment:9 only exists in develop.

comment:14 Changed 7 years ago by Owen Arnold

  • Status changed from reopened to verify
  • Resolution set to fixed

Should be updated now

comment:15 Changed 7 years ago by Russell Taylor

  • Status changed from verify to verifying

comment:16 Changed 7 years ago by Russell Taylor

  • Status changed from verifying to reopened
  • Resolution fixed deleted

I think you should use a vector instead of a list for the CollectionOfPropertyWidget. That way if there are multiple candidates, but none are called InputWorkspace, it will pick the first-declared one rather than the first alphabetically. I think that's more likely to produce the desired outcome.

comment:17 Changed 7 years ago by Owen Arnold

  • Status changed from reopened to inprogress

Agree with the above. Will fix as recommended.

comment:18 Changed 7 years ago by Owen Arnold

refs #7541. Change collection type.

Changeset: 33ee2087d466e2d63bc78bf5fc056d27f290c2f4

comment:19 Changed 7 years ago by Owen Arnold

  • Status changed from inprogress to verify
  • Resolution set to fixed

comment:20 Changed 7 years ago by Keith Brown

  • Status changed from verify to verifying
  • Tester changed from Russell Taylor to Keith Brown

comment:21 Changed 7 years ago by Keith Brown

Works as expected and fixes #7265 as well as they were duplicate tickets

comment:22 Changed 7 years ago by Keith Brown

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/7541_replace_workspace'

comment:23 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 8386

Note: See TracTickets for help on using tickets.