Ticket #9167 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

Transfer Button Should only Transfer Run Numbers

Reported by: Owen Arnold Owned by: Keith Brown
Priority: major Milestone: Release 3.2
Component: Reflectometry Keywords:
Cc: Blocked By:
Blocking: Tester: Owen Arnold

Description (last modified by Owen Arnold) (diff)

The Transfer is costly and slow because we load the workspace in order to extract the run number. The code originally looked something like this:

The functionality of the transfer button has been changed and some files are being loaded - this is totally unnecessary. The following code is sufficient, just to populate the table with the run numbers selected:
 def transfer(self):
        col=0
        row=0
        while (self.tableMain.item(row,0).text() != ''):
            row=row+1
        for idx in self.listMain.selectedItems():
            runno=idx.text().split(':')[0]
            item=QtGui.QTableWidgetItem()
            item.setText(runno)
            self.tableMain.setItem(row,col,item)
            item=QtGui.QTableWidgetItem()
            item.setText(self.textRuns.text())
            self.tableMain.setItem(row,col+2,item)
            col=col+5

The purpose of this ticket is the speed up the transfer process.

1) Figure out rough timings for transfering say 4 runs. Record timings.

2) Try short cuts to speed up the transfer process. One option might be to try to parse the run number out of the title (fall back to default current behaviour if this fails).

3) If the short cut is robust and improves the timings from (1) then we will use them.

4) Put an option in, set to ON, to disable loading from the ADS. Max doesn't like the ADS items getting automatically made available for use in his GUI.

Change History

comment:1 Changed 7 years ago by Owen Arnold

  • Status changed from new to assigned

comment:2 Changed 7 years ago by Keith Brown

Adding the ability to turn off ADS workspaces being loaded into the list to this ticket. The live data options will be repurposed as a general options dialog to hold this.

comment:3 Changed 7 years ago by Owen Arnold

What happened to this? Did you fix it and not check it in, or do the work on a separate branch?

comment:4 Changed 7 years ago by Owen Arnold

  • Priority changed from minor to major
  • Description modified (diff)

Update ticket description following conversation with Max.

comment:5 Changed 7 years ago by Owen Arnold

  • Description modified (diff)

comment:6 Changed 6 years ago by Keith Brown

  • Status changed from assigned to inprogress

Refs #9167 new option added to options

The live data options dialog is in the process of being converted to a general purpouse options dialog

Changeset: 0bc5e088074bba3e7eb1a9f24bf8445ab970f805

comment:7 Changed 6 years ago by Keith Brown

Refs #9167 Options dialog updated

Teh live data options is now a general options dialog, and now contains a checkbox which will be wired to the functionality that loads workspaces from the ADS.

Changeset: a6ddbf34bea1cde7bf42abc7d8f1b9a0116ff1e9

comment:8 Changed 6 years ago by Keith Brown

Refs #9167 options now persist in settings

The new option has been wired into the Qsettings and is saved between uses.

The option isn't actually used yet.

Changeset: 9d1328303ca94293b1b781ea7c79d587ae4dc22e

comment:9 Changed 6 years ago by Keith Brown

Refs #9167 Minor style edit

In another ticket i'd added a new method, but ti didn't have apropriate spacing or a docstring, just fixed that quickly

Changeset: 9ed5442d77635d41356004cfafdbc89e471da2aa

comment:10 Changed 6 years ago by Keith Brown

Refs #9167 ADS get option wired up

The option to enable or disable fetching of names form the ADS is now wired up.

Changeset: d8297839fd02dcf17fc1fbd8a66fa23d9f71ea13

comment:11 Changed 6 years ago by Keith Brown

Refs #9167 Fix syntax errors

I've forgotten to change an import after repurpousing the live data options.

I'd also not capitalised a False.

Changeset: ff8c42455b90b96db3a3ccca457808984c32104b

comment:12 Changed 6 years ago by Keith Brown

Refs #9167 Tweak to functionality

After changing the options the list will be repopulated if the ADS otpion has changed.

This is so that the change will take effect immeadiately

Changeset: 86b1288801079c07abbe251ca7e7c8a8851d9412

comment:13 Changed 6 years ago by Keith Brown

Refs #9167 Transfer process changed

The way runs are transferred is now much mroe efficient.

First a regex will check if the text being transferred starts with one or more numbers followed by a colon. If so it uses the numbers. If it's not a simple case of extracting numbers, it'll then check the ADS and look at the sample logs. If it's not in the ADS it'll try and load it first

The functionality that refreshes the runs list after the ADS fetch option is changed has also been removed after a word with Owen,

Changeset: aa8fd94ed87b3c008bd7745aa9b225b03159ad3e

comment:14 Changed 6 years ago by Keith Brown

Refs #9167 Changed wording of ADS fetch option

Apparently users won't know what the "ADS" is, so i've changed the wording to "workspaces from mantid"

Changeset: 8c791cd00602818d43d3be7c5be6886cc8ee4289

comment:15 Changed 6 years ago by Keith Brown

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

To Tester:

Load some Workspaces into mantid (any workspaces) and load the ISIS reflectometry Gui. In previous versions the workspaces would appear in the Runs list to the side, by default that functionality is switched off now.

Go Options -> Refl Gui Options.. and check "Load workspaces from mantid into the Runs list" hit OK. Now close and reload the gui and the workspaces should appear in the list.

The way runs are transferred to the table has been improved.

Search for an existing RB number to get some runs into the list. Highlight a load of the runs that appeared from the search and hit the '=>' button. The transfer should be instant as it should no longer be loading anything.

Also check:

  • Workspaces in the ADS aren't loaded again, their transfer should be instant as it should only be looking at the sample logs.
  • Group Workspaces have their name transferred rather than a number.
  • Transferred runs that don't exist in the ADS and don't start with numbers then a colon, will be loaded and the run number extracted. This will only work if the file is in your managed user directories. I'd suggest deleting an item from the ADS while the interface is running to test this.
  • It will throw if it's unable to find a run number sample log on files already loaded in or being loaded in, or if it can't load a file in.

comment:16 Changed 6 years ago by Owen Arnold

  • Status changed from verify to verifying
  • Tester set to Owen Arnold

comment:17 Changed 6 years ago by Owen Arnold

  • Status changed from verifying to closed

Merge remote-tracking branch 'origin/feature/9167_Refl_Transfer_run_numbers'

Full changeset: 8efd0928d722644b271c72e1254a13d0481ea2e2

comment:18 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 10010

Note: See TracTickets for help on using tickets.