Change prefix from PETAcquisitionData to STIRAcquisitionData.

evgueni-ovtchinnikov commented 1 year ago

Changes in this pull request

STIR now handles SPECT in addition to PET, so we should use prefix STIR for STIR data containers.

This was already in place for images - this pull request does the same for acquisition data.

For the sake of backward compatibility, the old nomenclature is preserved for the time being via typedef.

Testing performed

Related issues

Fixes #1126.

Checklist before requesting a review

[ ] I have performed a self-review of my code
[x] I have added docstrings/doxygen in line with the guidance in the developer guide
[ ] I have implemented unit tests that cover any new or modified functionality
[x] The code builds and runs on my machine
[x] CHANGES.md has been updated with any functionality change

Contribution Notes

Please read and adhere to the contribution guidelines.

Please tick the following:

[x] The content of this Pull Request (the Contribution) is intentionally submitted for inclusion in SIRF (the Work) under the terms and conditions of the Apache-2.0 License.

KrisThielemans commented 1 year ago

thanks! We will need to put this in version 4.0, as it breaks backwards compatibility.

@johannesmayer presumably this will affect #1007 ?

evgueni-ovtchinnikov commented 1 year ago

We will need to put this in version 4.0, as it breaks backwards compatibility.

Actually, this PR can be merged right away as we have

    ///
    ///  Backward compatibility - to be removed in SIRF 4
    ///
    typedef STIRAcquisitionData PETAcquisitionData;
    typedef STIRAcquisitionDataInFile PETAcquisitionDataInFile;
    typedef STIRAcquisitionDataInMemory PETAcquisitionDataInMemory;

(stir_data_containers.h, lines 772-777).

KrisThielemans commented 1 year ago

ah. perfect. Great idea!

can you put those typedefs between #ifdef SIRF_VERSION < 040000 or whatever it has to be? Also add a line to CHANGES.md. Feel free to merge then. thanks!

KrisThielemans commented 1 year ago

I notice the problem you had with version.h. I guess this means that we don't have access to the generated file Correct? I think this means we need to add a line after https://github.com/SyneRBI/SIRF/blob/cf384b18ef5f000f9509457e6cb928971660ae2f/CMakeLists.txt#L213 include_directories(${CMAKE_CURRENT_BINARY_DIR}/cmake/include If you think that's correct, please create an issue for it

SyneRBI / SIRF