-
Notifications
You must be signed in to change notification settings - Fork 95
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
Introduce classes for ProjData indices #1273
Conversation
3129c05
to
1e1a89c
Compare
temp zenodo problems. Also, Appveyor didn't run, as last commit was only for the release notes. I'll need to push something else. |
A lot of work to be done here, but that has to be after the next release.
I agree with the change and it looks good. I dont have time to fully review it. Are there any additional tests that should be added? |
I guess so. However, the old functions are now mostly implemented in terms of the new ones, so we have the tests for free. There will be a few untested, but we will find out once we apply this to TOF. I wanted to have this PR on 5.2, such that we can tell people to use the new interface, and it will work for the next STIR version as well (while the old functions won't work for TOF). Maybe I didn't make the latter very clear in the release notes.... |
@danieldeidda this should be ready now, aside from some tiny edits to the release notes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
- fixes to text on SegmentIndices etc - flag that we will require C++ 14 in the next major version
This PR introduces
SegmentIndices
,ViewgramIndices
andSinogramIndices
(and derivesBin
fromViewgramIndices
) and add constructors and methods to use these, as opposed to listing indices separately. This will allow for more concise code, but also prepares for TOF (and energy/layers etc).Using these new methods is left for the TOF branch.
DataSymmetriesForViewSegmentNumbers
etc have not been modified yet.Fixes #1263