Refl_gui Improvements - RB functionality

[Keith Brown]

There is a QLineEdit widget on the gui marked "RB" the functionality of this was broken between 2.6 and 3.0, make it produce the same behaviour as 2.6

[Keith Brown]

Refs #8560. Resolve Merge conflict between feature/8532 and develop

Changeset: d2d043081386226f4404c578e6e7bd164513d518

[Keith Brown]

Added blocking tickets as they need to be pushed to master before I can finish this ticket as I'll need to merge them in.

[Keith Brown]

Refs #8560. Changed script to hold a single tuple instead of map

Latest_isis_runs.py no longer cares about runs past the most recent.

It used to hold a dictionary of <cycle,[journal_path,cycle_path]> where it not holds only the [journal_path,cycle_path] of the most recent cycle

It will also return a list of strings in the form of [run_no]: [title]

Changeset: 9677b0eafb819f1a98dbc3547655bf23956accd2

[Keith Brown]

Refs #8560. Functionality fixed, waiting for blocking tickets

The script now returns the data in the format we want, and does a check to make sure the user has entered only an integer to save checking for an eID that won't exist

The ui has had a minor tewk in that the central widget has had it's name changed and a sensible name given to the layout it hosts. The ListWidget has also been expanded slightly. Compared to the old functionality,(as of 2.6 when this wokred last) the list box will no logner resize when given data, scrollbars will be used instead.

This ticket is now wating for #8531, #8532 and #8549 to be pushed to master so i can merge them in and make sure that the functionality doesn't break.

Changeset: 28d34917bf08ba5ff61e33afa06eba1a3ffdceae

[Keith Brown]

Refs #8549. Working on Latest_isis_runs

As the scientists don't seem interested in other cycles, the latest_isis_runs script is bieng altered to return a list rather than a map

Changeset: c583d2a12fc108882bf7e95580452017cce75eaf

[Keith Brown]

above commit referenced the wrong ticket and has been copied over here, this should have been the first commit on the ticket

[Keith Brown]

Refs #8560. Removed comboCycle and added blank RB case

The comboWidget comboCycle hasbeen removed from the ui, and it's related code deleted or disabled.

If the RB box is empty, the listWidget will be populated with all runs form that cycle.

Changeset: 2533319cf9822aa5842ebb4b32ea1a603a4a3d23

[Keith Brown]

Refs #8560. Merge remote-tracking branch 'origin' into feature/8560_Refl_gui_RB_functionality

Changeset: 05415dc0e1a23865d8d940182811844d8a49aa65

[Keith Brown]

Refs #8560. Merge origin/master into feature/8560

Previous ticket has now been merged into this ticket to reduce the chance of further merge conflicts

Changeset: 491c8472c12b6bbec76453952b2308d368a52ff3

[Keith Brown]

Refs #8560. Added a minimum size to the QLineEdits

Not related to the core issue, but samll enough to include.

It was noticed on Mac that the RB lineEdit was rather small on the defualt window size, so I've added a minimum width to both LineEdits.

Changeset: c232f67047f229e270ef87c9470a91378f2137f6

[Keith Brown]

Refs #8560. Fixed intrument selection after breaking in another ticket

When I changed the signal connections from the old C++ style to the new pyqt style notation i inadvertently broke the instrument selection combobox.

This was because the default "Activated" signal carried the index of the new item, rather than the new contents like it used to. The same method couldn't be kept as combobox would only return unicode strings, which the config manager didn't like as they weren't compatible with C++ std::string.

The solution was to a new class-member level list of instruments in all caps and use it to populate the listbox in the subclass (removing the hard coded values from the UI file) and set the config manager meaning that the values used would be python strings (compatible with C++ std::string)

Changeset: f927e5a226f04308f02e966bef5ab4d9c80d7ef7

[Keith Brown]

I believe I've accomplished what was asked by this ticket with a few other related fixes.

Points to test:

• You can enter an RB and get runs with a corresponding Experiment ID back populating the listbox.
• The instrument selection works as expected, populating the listbox with all the recent cycle runs for that instrument when the instrument is switched (this will also clear the RB box)
• The Cycle selection box is gone.
• On a Mac (and perhaps linux) the line edit widgets are now a sensible size.

[Keith Brown]

Refs #8560. Merge remote-tracking branch 'origin' into feature/8560_Refl_gui_RB_functionality

Changeset: da96ae81a99949365117fc48cbc48d398b5da005

[Keith Brown]

Refs #8560 develop. Merge branch 'feature/8560' into develop

Conflicts:

Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py Code/Mantid/scripts/Interface/ui/reflectometer/refl_window.py

Changeset: 5cda62d38f8829c38c3d54611c8e80e134aeea37

[Keith Brown]

Met with reflectometry guys, been told two things:

• They are interested in all runs, all the time. So RB functionality should search them all, rather than a specific cycle
• They want it fast, so the old index-based (rather than purely DOM based) functionality is there to stay as the python module combined with the way I had to search was VERY slow

[Keith Brown]

Refs #8560. Corrected RB functionality

Lots of changes:

• The RB search is no longer triggered by textRB's editingFinished signal. It's now linked to textRB's returnPressed and the new buttonSearch's clicked. So this will now only search when enter is pressed in the text box or the search button is clicked.
• A search won't be attempted if the RB box is empty
• transfer() and the RB search will display an error message in a messagebox if something goes wrong.
• The main window now has a status bar, used for showing search status.
• The RB search no longer uses cycles, and will search every journal from the beggining of time to the present day
• The RB search is much faster (it's practically instant) due to using summary.txt (located alongside the xml journals) instead of the xml journals. It's guessed that the speed gain has come from only opening one file over a network rather than many, the xml module might have also been slightly to blame.
• The RB search no longer adds cycle directories to the managed user directories - this is a side effect of using the summary.txt as that doesn't include the cycle so the fucntion can't add the cycle directory.

A decision is being made on which direction to take in terms of seraching, as both xml and txt methods are in the code and it's a case of changing a single function call to switch them. txt is instant, but doesn't add cycles to the managed directories, while xml takes about 40 seconds but adds cycle directories to the managed directories

Changeset: 648a6adcb1adca98e71c794168f2140da8f85700

[Keith Brown]

For now, search is remaining using the txt file

Opening for testing.

[Owen Arnold]

Keith. I've been speaking to the reflectometry scientists and we think that the best thing to do here would be to use the xml version of the lookup, but limit the number of cycles that it goes backwards when it does the search to 4 by default. We could then provide a spin box next to the RB search to increase/decrease the number of cycles to look back in. That way we would get fast searching, but would also be able to also automatically have them added to the search directories.

Given you're previous comment (comment 17) this doesn't seem like it would be particularly hard to do, or take very long to do.

[Keith Brown]

Referenced wrong ticket in commit. This is the ticket it actually corresponds to.

Refs #8633. Changed search to use xml and a fixed cycle depth

The search now uses xml again and is fixed to 1 cycle into the past (although this will be fixed to a spinbox in another ticket to allow the user to set how many cycles into the past they want to search)

Changeset: 8328f140d06a78b7b24d3a1c48e168881b7e2fdb

[Keith Brown]

Refs #8560. Updated for the new API

The provided file used the old API. The code has been updated to use the new API.

The file also needed some global functions defined in it too

Changeset: 90aae6526aa6caa6349a993cc04ccb75a0d479d8

[Keith Brown]

Above commit referenced wrong ticket. Should have referenced #8633 instead.

[Keith Brown]

This is fixed, the search now uses xml. was reopened due to a couple of commits with wrong references

[Owen Arnold]

Need the spin box implemented here otherwise the changes will not be useful and scientist/users are looking at the nightly builds.

[Keith Brown]

Refs #8560. Added spinbox for RB search depth

I've added a spinbox for setting the depth of the search and made a few general improvments to the UI at the same time.

The widgets are now spread across two rows as they would have been cramped with the new widgets.

The Instrument combobox now has a label.

Buddies have been set, each label is now paired with a widget.

Tab order is now set properly.

Changeset: 4791612e7cd1b23f27250ef848357c391fd0ab55

[Keith Brown]

Previous commit also removed option 3 from the polarisation correction combobox in preperation for another ticket

[Keith Brown]

Refs #8560. Spin box now sets search depth.

The spin box value is now attached to the call to getJournalRuns, so the search depth can now be adjusted.

Changeset: 18373c8c7a13ea3fa48dae8f6ecce0b65f138338

[Keith Brown]

Reopening as i forgot to attach a signal

[Keith Brown]

I thought a qspinbox also had the returnpressed signal, but it doesn't. So I'm not attaching anything else

[Owen Arnold]

Unfortunately I have to reopen this. Here are some specific points that need to be addressed.

• No indication that the search is happening. Not even a log message to indicate success/failure.
• The expriement id 24088082 is not found even though it does exist and should contain INTER00013470.nxs INTER00013460.nxs etc. I set the number of cycles to to search to 10. How did you test this?

You should be able to verify that 24088082 is the RB number that corresponds to the runs from this cycle by looking at the journal.xml files.

[Keith Brown]

Looking at the journals, INTER00013470.nxs and INTER00013460.nxs come under RB/Experiment ID 1120015 and I've checked and made sure that that number brings up those runs and more besides. And assuming that the RB/Experiment IDs are sequential, the most recent run has only 1320341 so I doubt 24088082 will bring up anything

I will however print out more notifications for search start and completion.

[Keith Brown]

Refs #8560. Added more prints to the log

Added more print notices for search start and end.

Changeset: fb0da72f1840e56a4778473130d606aa67f80571

[Keith Brown]

This ticket actually fixes the problem from 8721 as well, and it's rather urgent as the bug limits the use of the gui rather heavily

[Owen Arnold]

Replying to Owen Arnold:

Unfortunately I have to reopen this. Here are some specific points that need to be addressed.

• No indication that the search is happening. Not even a log message to indicate success/failure.
• The expriement id 24088082 is not found even though it does exist and should contain INTER00013470.nxs INTER00013460.nxs etc. I set the number of cycles to to search to 10. How did you test this?

You should be able to verify that 24088082 is the RB number that corresponds to the runs from this cycle by looking at the journal.xml files.

My mistake: 1320341 is the actual RB number. 24088082 is the internal ICAT primary key for this record.

[Owen Arnold]

Tester:

Also -

• Check that the Instrument selection now works (ask Keith)
• Check that 'Process' still works. Keith should be able to demonstrate this working and producing plots.

[Owen Arnold]

Samuel will test this.

[Samuel Jackson]

Couple of small issues before passing this ticket:

• There's still no output to tell me when the search is working. This could just be something as simple as telling me what cycle we're currently looking at so I can see progress is being made.

• Perhaps a log message actually confirming a success of failure, rather than just saying "complete".

• Placing an upper bound on the number of cycles searched. While this doesn't break functionality, it does seem odd that I can search an infinite number of cycles.

• A few minor bits of python re-factoring as discussed.

But otherwise this seems to work as intended. Complete these minor points and I will be happy to pass this ticket.

[Owen Arnold]

Comments above seem sensible, particularly around progress reporting and success/failure in searching.

[Keith Brown]

Refs #8560. Added maximum to spinbox and improved complete message.

The cycle depth spinbox is now more inteligent, its max value is now set from the number of cycle journals logged in the main jounral file

The search complete message now contains a value for the ammount of results found. Searches that yeild nothing are still deemed complete as it searched the amount it was told to but nothing existed.

Refactored a few lines in the txt search method to explain cnstant values, and added a comment about a concatentaion line.

Changeset: 772b33ae5efef33f02647d1d6de1d731a4301329

[Keith Brown]

I attempted to add more output logs when searching, but hit a problem where the messages weren't appearing until the search completed, thus voiding the effort.

This is the the same snag I encountered when I tried setting messages on the statusbar the recommended way (using addWidget()/removeWidget() rather than showMessage()/clearMessage()) and the reason why the listbox doesn't appear to clear before the search even though the call is made before the search begins.

[Keith Brown]

I've fixed all I can of the points that the tester mentioned, and those I haven't I've explained.

[Samuel Jackson]

I'm afraid I now get the following issue when running git test start:

ERROR: Branch 'feature/8560_Refl_gui_RB_functionality' has not been fully merged to develop,
meaning it is not possible to know whether the code is valid on all environments.

Contact Keith Brown on keith.brown@stfc.ac.uk (or via skype) and ask them to merge the code
to develop using 'git checkbuild' from their branch feature/8560_Refl_gui_RB_functionality and let you know when
it passes testing on the buildservers so you can start testing.

Fix and I'll test.

[Keith Brown]

Basically I forgot to checkbuild. Just as well as I found a point where an error could throw and fixed it.

[Keith Brown]

Refs #8560. Fixed a point where an exception could throw.

If a journal had no 'isis_cycle' tag then the search would crash. The serach now skips that journal if that is the case.

Changeset: 40adf777b1262f4fb40133716a4e9cefd176b390

[Samuel Jackson]

Unfortunately, I've still having problems with this ticket:

• Attempting to open a table file throws an error.
• Attempting to press process with manually entered values causes and error on line 496 of refl_gui because it's checking for a float type, but finding a nump.float64 type.

[Keith Brown]

Fixing former error now

Latter error was already present in script as i've not touched that method in all the time I've been working on this gui. I'll open a new ticket for it.

[Keith Brown]

Refs #8560. Fixed a widget naming issue

The error was because the open dialog was parented to a control that had been renamed. Fixed naming issues

Changeset: 82a0ffff7d9c803da44baddc4ad54229c52112d9

[Keith Brown]

Refs #8560. fixed saving issue

The save action haddn't been defined right. I'd forgotten to add self.

Changeset: 52dbad48f757f83ef3b3eef489b462064ceee444

[Keith Brown]

Refs #8560. Merge remote-tracking branch 'origin/master' into feature/8560_Refl_gui_RB_functionality

Changeset: 3de264da3bdfd73b6441fb94d740a9e981e9f50c

[Keith Brown]

Merged master in to make sure nothing else is gonna break as well as make sure there were no conflicts as this ticket has been open a while.

[Keith Brown]

Note that this ticket contains the following fixes in addition to the RB behaviour:

• The Open dialog is now parented properly
• The search no longer crashes on a search which would look at a journal with no cycle identification entries - this issue was observed with CRISP on a depth of 22
• the save action now works after it had been broken by the lack of 'self'
• The Instrument selection will change the instrument properly

[Keith Brown]

Refs #8560. Fixed statusbar messages

Discovered that the messages in the statusbar would dissapear when opening a menu. This was because of the temporary messages is was using.

The statusbar now has a standard message "Ready" and only uses temporary messages for the search notification.

Changeset: bebfe5093a9991e9cf891b1e55f3ca5c16c387fb

[Keith Brown]

Refs #8560 Fixed another Parenting issue

There was a spelling error when trying to parent a fileDialog.

Changeset: 87f24678f0cc662536725897f5bc2e19343dba53

[Samuel Jackson]

Merge remote-tracking branch 'origin/feature/8560_Refl_gui_RB_functionality'

Full changeset: ​3a4c69b5876d8451b6ed109b7b231f9eea236776

[Samuel Jackson]

I quickly ran though the previous testing requirements as well as the new changes. While these changes should still be tested on Linux/Mac, it appears to be working as intended on Windows. A minor issue identified (but that can be fixed in another ticket) was:

• trimming whitespace from investigation numbers. I accidentally copy/pasted an investigation number along with a space and was confused as to why it failed to find anything.

[Keith Brown]

Refs #8560. Resolve Merge conflict between feature/8532 and develop

Changeset: d2d043081386226f4404c578e6e7bd164513d518

[Keith Brown]

Refs #8560 develop. Merge branch 'feature/8560' into develop

Conflicts:

Code/Mantid/scripts/Interface/ui/reflectometer/refl_gui.py Code/Mantid/scripts/Interface/ui/reflectometer/refl_window.py

Changeset: 5cda62d38f8829c38c3d54611c8e80e134aeea37

[Stuart Campbell]

This ticket has been transferred to github issue ​9404