Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

There is too much logic in the Instrument Window code #4547

Closed
mantid-builder opened this issue Sep 12, 2011 · 2 comments
Closed

There is too much logic in the Instrument Window code #4547

mantid-builder opened this issue Sep 12, 2011 · 2 comments

Comments

@mantid-builder
Copy link
Collaborator

This issue was originally TRAC 3700

Original Reporter: Russell Taylor

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.

@mantid-builder
Copy link
Collaborator Author

@NickDraper (2012-01-09T09:43:01):
Moved to iteration 33 at iteration 32 code freeze


@stuartcampbell (2012-04-04T21:14:47):
refs http://trac.mantidproject.org/mantid/ticket/5043 & http://trac.mantidproject.org/mantid/ticket/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.
6d4bcf8


@NickDraper (2012-04-30T14:12:07):
Moved at end of release 2.1


@NickDraper (2012-08-10T12:43:26):
Moved at the end of release 2.2


@NickDraper (2012-10-28T11:38:40):
Moved to milestone 2.4


@NickDraper (2013-01-28T09:22:53):
Moved at the code freeze for release 2.4


@NickDraper (2013-04-29T09:49:33):
Moved to r2.6 at the end of r2.5


@NickDraper (2013-07-26T13:54:58):
Moved to backlog at the code freeze for R2.6


@NickDraper (2014-02-14T11:04:55):
Bulk move to assigned at the introduction of the triage step

@martyngigg
Copy link
Member

There have been significant refactors of this code since this was created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants