Ticket #8034 (closed: fixed)

Opened 7 years ago

Last modified 5 years ago

CombineMulti to use Stitch1D

Reported by: Owen Arnold Owned by: Owen Arnold
Priority: major Milestone: Release 3.0
Component: Reflectometry Keywords:
Cc: Blocked By:
Blocking: Tester: Jay Rainey

Description

Rip out the existing script based implementation of combine2 in CombineMulti and instead use the new Stitch1D algorithm. This is an important step towards better maintainability and consistency on the Reflectometry side of Mantid.

There are system tests for combineMulti, so these can be used as a guide for a correct implementation.

Change History

comment:1 Changed 7 years ago by Owen Arnold

  • Status changed from new to inprogress

comment:2 Changed 7 years ago by Owen Arnold

  • Milestone changed from Backlog to Release 3.0

comment:3 Changed 7 years ago by Owen Arnold

refs #8034. Directly to Stitch1D algorithm.

Simply wire-in the Stitch1D algorithm. Get rid of the existing messy implementations. We now get System test coverage for Stitch1D, and conversely get Unit test coverage for aspects of combineMulti via Stitch1D.

Changeset: e7a9dd21c503a50c3976d2137602f86e734b77c2

comment:4 Changed 7 years ago by Owen Arnold

At this stage the ISIS Refl GUI is still working, so I haven't introduced any undesired behaviour into the GUI.

comment:5 Changed 7 years ago by Owen Arnold

refs #8034. More removal of code.

Take out unnecessary block of code and imports. Everything still works fine.

Changeset: 081d64804817108a23efc26a9c2baca874e720c1

comment:6 Changed 7 years ago by Owen Arnold

Tester:

  • Code should be much simpler
  • System tests for ReflectometryQuickCombineMulti should pass on all platforms (still)
  • As a thorough check the reflectometry gui should still works the same as described in the tests for this ticket #7372

This is the last set of changes I plan to do to combineMulti. Moving forward, an algorithmic approach to combining multiple workspaces would be better. It would also allow me to actually introduce a new Algorithm API, rather than the string-based one we have to live with on combineMulti at present.

comment:7 Changed 7 years ago by Owen Arnold

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

comment:8 Changed 7 years ago by Owen Arnold

Last edited 7 years ago by Owen Arnold (previous) (diff)

comment:9 Changed 7 years ago by Jay Rainey

  • Status changed from verify to verifying
  • Tester set to Jay Rainey

comment:10 Changed 7 years ago by Jay Rainey

  • Status changed from verifying to closed
  • Code is now more compact.
  • No problems found on Jenkins, and I manually ran the ReflectometryQuickCombineMulti system test on my machine and it passed (Ubuntu 12.04).
  • I followed the steps in #7372 and the GUI still functions as expected.

comment:11 Changed 7 years ago by Jay Rainey

Merge remote-tracking branch 'origin/feature/8034_combinemulti_stitch1d'

Full changeset: 1b8a91e1938e1078f12fc2fdd858e86976f80438

comment:12 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 8879

Note: See TracTickets for help on using tickets.