Ticket #7386 (closed: fixed)
Safe getDetectorPosition on Peak object
| Reported by: | Owen Arnold | Owned by: | Owen Arnold |
|---|---|---|---|
| Priority: | major | Milestone: | Release 2.6 |
| Component: | Framework | Keywords: | |
| Cc: | reuterma@… | Blocked By: | |
| Blocking: | #7325 | Tester: | Martyn Gigg |
Description
getDetectorPosition on a peak doesn't check whether the detector is null first. As a result, we're seeing a failure on PeaksOnSurface (#7325). The remedy is to add an additional method, which will check whether the detector is null and then throw something that can be caught, rather than getting a segfault. I'm going to leave the existing getDetectorPosition in place, because I don't want to adversely affect performance in other areas.
Change History
comment:2 Changed 7 years ago by Owen Arnold
Tester: Code a code review of these changes is the best way to test. Also check that the 2 new unit tests run. There's no good way to access this functionality at present, at least until #7325 is implemented.
comment:3 Changed 7 years ago by Owen Arnold
refs #7386. Fix mock ipeak class.
Changeset: b2d1be3955432ae22dc3c604ed30a69b67af86be
comment:5 Changed 7 years ago by Owen Arnold
- Status changed from accepted to verify
- Resolution set to fixed
comment:6 Changed 7 years ago by Martyn Gigg
- Status changed from verify to verifying
- Tester set to Martyn Gigg
comment:7 Changed 7 years ago by Martyn Gigg
- Status changed from verifying to reopened
- Resolution fixed deleted
I think it would be best if the default getDetectorPosition did the check and was safe and then we have a separate getDetectorPositionNoCheck if it is necessary for performance. That way people have to opt out of being safe.
It looks like it is only used in PeaksInRegion at the moment.
comment:9 Changed 7 years ago by Owen Arnold
refs #7386. Rename functions.
Changeset: 597dc38deecd6207bd4bdbf02a5529008a058cbf
comment:10 Changed 7 years ago by Owen Arnold
- Status changed from accepted to verify
- Resolution set to fixed
comment:12 Changed 7 years ago by Martyn Gigg
- Status changed from verifying to closed
Looks good now.
comment:13 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 8232
refs #7386. Safe detector position method.
Changeset: bf7c1cd17db187ba08a9974d5f1774f992590445