Ticket #9104 (closed: fixed)
Loading a muon file in python creates unwanted output
Reported by: | Roman Tolchenov | Owned by: | Arturs Bekasovs |
---|---|---|---|
Priority: | major | Milestone: | Release 3.2 |
Component: | Framework | Keywords: | CORE |
Cc: | Blocked By: | ||
Blocking: | Tester: | Martyn Gigg |
Description
ws = Load('MUSR15189') creates the data workspace and the grouping table named LoadChildWorkspace.
Loading the same file from the GUI creates only the data.
Change History
comment:3 Changed 7 years ago by Arturs Bekasovs
LoadMuonNexus has two optional output workspace properties. In the algorithm code, these are returned only if their name is set to something. So it's fine when you run
ws = LoadMuonNexus("MUSR15189")
Though the generic Load algorithm sets LoadChildWorkspace to all the unset output workspace properties (to make validator happy), even if the workspace is optional, which causes the described behaviour.
The problem could be solved by fixing Load algorithm to check if the WorkspaceProperty is optional first. I've tried that quickly and it seem to work, though I need somebody more senior to take a look at that.
comment:4 Changed 7 years ago by Arturs Bekasovs
This doesn't happen if we run it from the dialog, because the dialog sets the workspace properties specifically to input value (which is "" if we leave them empty), after they were set to LoadChildWorkspace (which happens when the filename property is changed). Python caller doesn't do that, neither does C++ invocation of Load, which causes LoadChildWorkspaces to be created.
comment:5 Changed 7 years ago by Arturs Bekasovs
- Status changed from assigned to inprogress
Refs #9104. Don't set up optional output ws properties.
Changeset: 748abbddb496ee674f1878a9131ab06b5050f3c3
comment:6 Changed 7 years ago by Arturs Bekasovs
Tester:
Somebody senior please, core stuff.
Please make sure you understand the implications behind the change (I've tried to explain my understanding in previous comments) and that the code update is reasonable and not dangerous. Obviously, system/unit tests should be passing.
Then verify that the additional workspaces are not created when loading Muon file both using Load from Python and load dialog.
comment:7 Changed 7 years ago by Arturs Bekasovs
An issue related to this one: createChildAlgorithm also sets output workspaces to ChildAlgOutput no matter what their isOptional() status is.
comment:8 Changed 7 years ago by Arturs Bekasovs
createChildAlgorithm case is more complicated than the Load one, as suggested by Martyn, so not touching it in this ticket.
Additionally, tests should be added for the Load to check that it is now behaving correctly when optional output workspaces
- are requested
- are not requested
comment:9 Changed 7 years ago by Arturs Bekasovs
Refs #9104. Tests for new functionality
Changeset: 3d9e6c238dd550af4a3acef4075997fb92102f03
comment:10 Changed 7 years ago by Arturs Bekasovs
comment:11 Changed 7 years ago by Arturs Bekasovs
- Status changed from inprogress to verify
- Resolution set to fixed
comment:13 Changed 7 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:14 Changed 7 years ago by Martyn Gigg
I think leaving createChildAlgorithm alone is the correct thing to do as it's a simple utility function that sets up the algorithm appropriately in 99% of circumstances. A caller is free to override what it has created anyway.
comment:15 Changed 7 years ago by Martyn Gigg
The code change looks fine and I confirm that now in Python you only get the single output workspace if that was all you specified but you get the extra ones if you have requested them.
comment:16 Changed 7 years ago by Martyn Gigg
- Status changed from verifying to closed
Merge remote-tracking branch 'origin/feature/9104_load_option_output_ws'
Full changeset: eac5434cb5188934659e449ddcf95184b14d0d1d
comment:17 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9947