Ticket #8034 (closed: fixed)
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: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: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