Ticket #3700 (assigned)

Opened 9 years ago

Last modified 5 years ago

There is too much logic in the Instrument Window code

Reported by: Russell Taylor Owned by: Roman Tolchenov
Priority: major Milestone: Backlog
Component: MantidPlot Keywords:
Cc: Blocked By:
Blocking: Tester:

Description (last modified by Russell Taylor) (diff)

The entities that should be creating or modifying workspaces are Algorithms, not a GUI layer. As such, the code in the Instrument Window classes that does this needs to be moved into new or existing algorithms.

Particular culprits are:

  • InstrumentWindowMaskTab::createMaskWorkspace
  • InstrumentWindowMaskTab::saveMaskToWorkspace
  • InstrumentWindowPickTab::addPeak

Related to this, in InstrumentWindow.cpp, the are 2 different methods employed of running algorithms. One is to emit an 'execMantidAlgorithm' signal that is connected to a MantidUI method, the other is to do it directly via an IAlgorithm handle obtained from the FrameworkManager. This should be made consistent and optimal (e.g. algorithms should run in a separate thread), bearing in mind the possibility that the InstrumentWindow may in the future be extracted from within the MantidPlot executable itself.

Thirdly, the MaskDetectors and GroupDetectors algorithms can both take a list of detector ids, so why does the instrument window (InstrumentWindow::groupDetectors & InstrumentWindow::maskDetectors) go to the trouble of converting such a list to workspace indices prior to calling the algorithms.

Change History

comment:1 Changed 9 years ago by Russell Taylor

  • Description modified (diff)

comment:2 Changed 9 years ago by Russell Taylor

  • Owner set to Roman Tolchenov
  • Status changed from new to assigned
  • Milestone changed from Iteration 30 to Iteration 31

comment:3 Changed 9 years ago by Nick Draper

  • Milestone changed from Iteration 32 to Iteration 33

Moved to iteration 33 at iteration 32 code freeze

comment:4 Changed 9 years ago by Stuart Campbell

refs #5043 & #3700. Corrected to return mask and inverse.

Changed the confusing text labels on the GUI. Removed some of the logic code in the GUI code in favour of using existing algorithms.

Changeset: 6d4bcf8e8c3563b8482763ed788b9c68c1326e85

comment:5 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.1 to Release 2.2

Moved at end of release 2.1

comment:6 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.2 to Release 2.3

Moved at the end of release 2.2

comment:7 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.3 to Release 2.4

Moved to milestone 2.4

comment:8 Changed 8 years ago by Nick Draper

  • Milestone changed from Release 2.4 to Release 2.5

Moved at the code freeze for release 2.4

comment:9 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.5 to Release 2.6

Moved to r2.6 at the end of r2.5

comment:10 Changed 7 years ago by Nick Draper

  • Status changed from assigned to new

comment:11 Changed 7 years ago by Nick Draper

  • Milestone changed from Release 2.6 to Backlog

Moved to backlog at the code freeze for R2.6

comment:12 Changed 7 years ago by Nick Draper

  • Status changed from new to assigned

Bulk move to assigned at the introduction of the triage step

comment:13 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue ​4547

Note: See TracTickets for help on using tickets.