Ticket #8590 (closed: fixed)
Refl_gui Improvements - Live Data fix
Reported by: | Keith Brown | Owned by: | Keith Brown |
---|---|---|---|
Priority: | critical | Milestone: | Release 3.1 |
Component: | Reflectometry | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | #7377 | Tester: | Owen Arnold |
Description
Before Owen dealt with it in #8372, quick did something bizarre and bad.
In quick.coadd it used to call startlivedata internally inside quick if the run number was 0 then used the output for processing. Owen has since removed this functionality and it needs put back, but in the right way.
What should happen is that the GUI should be the one to call startlivedata and then pass the resulting workspace to quick as quick can now accept a workspace as a parameter
Change History
comment:2 Changed 7 years ago by Keith Brown
Refs #8590. Changed the way the check happens
The LoadInstrumentData script has been repurposed as load_live_data and doen't do a zero check when you ask for live data.
Changeset: afd9570efd251437db31c4c05cab26a25c83f255
comment:3 Changed 7 years ago by Keith Brown
Refs #8590 continuing work on integrating startlivedata
The user is now prompted for what Accumulation method they want to use for the live data.
However there is a problem where quick crashed after it tries to get the workspace defualts on the live data workspace. I will need to investigate this.
I plan on merging master into this branch in order to gain the benefit of some of the newer edits to quick, in order to see if they fix this.
Changeset: eaa56fb78c6dfb5c57790bdcd6998abdb22b805f
comment:4 Changed 7 years ago by Keith Brown
Refs #8590. Merge remote-tracking branch 'origin' into feature/8590_Refl_gui_Live_Data_fix
Changeset: 1e9ee5233df2cffdbd33173e81cc39ba3a7225a8
comment:5 Changed 7 years ago by Keith Brown
Refs #8590. Remove debug code
FakeEventDataListener wasn't doing the job as the workspace it generated didn't have an accociated paremeter file so it caused an error later in quick when it tried to get those parameters.
Changeset: 4370b23e1b1e1f972f4ab2835077e7e27ddb865d
comment:6 Changed 7 years ago by Keith Brown
- Status changed from inprogress to verify
- Resolution set to fixed
This should only be tested by Owen.
This will likely need to be tested in debug as the fake event listeners don't appear in a release build. The wrapper script get_live_data() will also need modifying slightly so that you can force the fake event listeners, otherwise it'll try and connect to an instrument and fail.
Run FakeISISEventDAE and FakeISISHistoDAE to initialise the listeners
Load a tbl file and change the run number of one to 0, then hit process (processing all rows). It should then ask what accumulation mode you require. At this point the Live Data Listener should start and create a workspace. Quick.py will then fail as the fake event listener won't have the appropriate parameters
comment:10 Changed 7 years ago by Owen Arnold
I can see what you've done here, and I think what you've done should work. However, the following should be addressed within this ticket.
- Dead code line 371 etc. Dead code should be deleted.
- Menu option for accumulation method rather than pop-up. Store choice in qtsettings or similar so that it is remembered. Users will definitely get annoyed if they have to select this each time.
- Add menu option above to documentation (screenshot with help text)
- Line 520. Replace logic seems unnecessary. Should be able to load via the run number.
Once these things are done, this should be fine to go into master.
comment:11 Changed 7 years ago by Owen Arnold
- Status changed from verifying to reopened
- Resolution fixed deleted
comment:12 Changed 7 years ago by Owen Arnold
The decision logic does seem to be working as I have checked that. The other issues will still need to be resolved.
comment:13 Changed 7 years ago by Keith Brown
Update from owen:
Remove dead code
The accumulation method can be saved for another ticket
Ignore the logic change at 520, owen is changing that area anyway
comment:14 Changed 7 years ago by Keith Brown
- Status changed from reopened to inprogress
Refs #8590 Merge remote-tracking branch 'origin' into feature/8590
Conflicts:
Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py
Changeset: 0e46b72dfffb13f906db7c51ebca79979c88eb35
comment:15 Changed 7 years ago by Keith Brown
Refs #8590 Dead Code removed
Dead code has been removed
Changeset: fa73b4d289e73bd5a6bc7ecf98595480d8c07929
comment:16 Changed 7 years ago by Keith Brown
Owen should really be the one to test this.
To Tester
Make sure the existing functionality works: http://www.mantidproject.org/Testing_ISIS_Reflectometry_GUI
Make sure an attempt to start a live data service for the selected instrument is actually attempted. This attempt should fail as the live data listeners won't be able to connect properly.
The logic is there, and should work provided a connection is available, but we have no way of making sure.
comment:17 Changed 7 years ago by Keith Brown
- Status changed from inprogress to verify
- Resolution set to fixed
comment:19 Changed 7 years ago by Owen Arnold
Hasn't been fully merged into develop. Once you've done this, I can test it.
comment:20 Changed 7 years ago by Keith Brown
Refs #8590 Merge branch 'feature/8590' into develop
Conflicts:
Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py
Changeset: f09050426d6c23bf9ca4b45cdf9548446399281c
comment:21 Changed 7 years ago by Owen Arnold
- Status changed from verifying to reopened
- Resolution fixed deleted
Had to reopen load_live_runs is undefined. Can you please fix this and retest it before adding it back to the testing pool. Thanks, Owen.
comment:22 Changed 7 years ago by Keith Brown
I think that was caused by the movement of modules in your other ticket as the module won't have moved on my branch. I just need to put it in the proper place
comment:23 Changed 7 years ago by Owen Arnold
I've also seen that you're using uppercase parameter names in some of the new functions. Our standard is to only use names starting with upper case to indicate a type. See http://google-styleguide.googlecode.com/svn/trunk/pyguide.html. You can write a separate ticket to clean this up at a later date.
comment:24 Changed 7 years ago by Keith Brown
- Status changed from reopened to inprogress
refs #8590 Replaced Import Statement
The import had dissapeared, persumably from when owen cleaned some stuff up and git got confused
Changeset: c9b51bc9f9b6e7e201898193399b6e455a532456
comment:25 Changed 7 years ago by Keith Brown
- Status changed from inprogress to verify
- Resolution set to fixed
comment:26 Changed 7 years ago by Owen Arnold
- Status changed from verify to closed
Merge remote-tracking branch 'origin/feature/8590_Refl_gui_Live_Data_fix'
Full changeset: 975a4d59d35ed2f9024b349dfb14c7fa127ecbf6
comment:27 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 9434
Refs #8590. Working on alternate loading functionality
I've created a script that wraps Load and StartLiveData which will invoke the one apropriate for the run number supplied
Changeset: f05ee9e28de6c0f794178b56eda00bec53df48c9