From a97167711bdd18bbaf73b1075105b8156cc6d059 Mon Sep 17 00:00:00 2001
From: Kris Thielemans <k.thielemans@ucl.ac.uk>
Date: Fri, 20 Dec 2024 00:29:45 +0000
Subject: [PATCH 1/2] make 2 deprecated members private

- ProjDataInfoBlocksOnCylindricalNoArcCorr defines 2 deprecated (see #105)
functions that are only used in tests, so I made them private.
- removed unused file

WARNING: this in principle breaks backwards compatibility.
---
 documentation/release_6.3.htm                 |  8 ++++-
 ...ProjDataInfoBlocksOnCylindricalNoArcCorr.h |  7 ++++
 ...ojDataInfoBlocksOnCylindricalNoArcCorr.inl | 32 -------------------
 3 files changed, 14 insertions(+), 33 deletions(-)
 delete mode 100644 src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.inl

diff --git a/documentation/release_6.3.htm b/documentation/release_6.3.htm
index dc6cbad024..47894a9454 100644
--- a/documentation/release_6.3.htm
+++ b/documentation/release_6.3.htm
@@ -98,7 +98,13 @@ <h3>New functionality</h3>
 </li>
 
 <h3>Changed functionality</h3>
-
+<ul>
+  <li>
+    Made 2 (deprecated) members of <code>ProjDataInfoBlocksOnCylindricalNoArcCorr</code> <code>private</code>
+    <br>
+    <a href="https://github.com/UCL/STIR/pull/1556">PR #1556</a>
+  </li>
+</ul>
 
 <h3>Bug fixes</h3>
 <ul>
diff --git a/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h b/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h
index f3f953ee8b..608f0cc800 100644
--- a/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h
+++ b/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h
@@ -33,6 +33,9 @@
 START_NAMESPACE_STIR
 
 class Succeeded;
+class ProjDataInfoTests;
+class BlocksTests;
+
 /*!
   \ingroup projdata
   \brief Projection data info for data from a scanner with discrete dtectors organised by blocks
@@ -98,6 +101,7 @@ class ProjDataInfoBlocksOnCylindricalNoArcCorr : public ProjDataInfoGenericNoArc
 
   std::string parameter_info() const override;
 
+private:
   //! \name set of obsolete functions to go between bins<->LORs (will disappear!)
   //@{
   Succeeded find_scanner_coordinates_given_cartesian_coordinates(int& det1,
@@ -111,6 +115,9 @@ class ProjDataInfoBlocksOnCylindricalNoArcCorr : public ProjDataInfoGenericNoArc
                                                          const CartesianCoordinate3D<float>& coord_1,
                                                          const CartesianCoordinate3D<float>& coord_2) const;
   //@}
+  // give test classes access to the private members
+  friend class ProjDataInfoTests;
+  friend class BlocksTests;
 
 private:
   bool blindly_equals(const root_type* const) const override;
diff --git a/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.inl b/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.inl
deleted file mode 100644
index a324911268..0000000000
--- a/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.inl
+++ /dev/null
@@ -1,32 +0,0 @@
-
-/*
-    Copyright (C) 2000- 2005, Hammersmith Imanet Ltd
-    Copyright (C) 2017, ETH Zurich, Institute of Particle Physics and Astrophysics
-    Copyright (C) 2021, University of Leeds
-    This file is part of STIR.
-
-    SPDX-License-Identifier: Apache-2.0
-
-    See STIR/LICENSE.txt for details
-*/
-
-/*!
-  \file
-  \ingroup projdata
-
-  \brief Implementation of inline functions of class
-        stir::ProjDataInfoBlocksOnCylindricalNoArcCorr
-
-  \author Kris Thielemans
-  \author Parisa Khateri
-  \author Viet Ahn Dao
-
-*/
-#include "stir/Bin.h"
-#include "stir/Succeeded.h"
-#include "stir/LORCoordinates.h"
-#include <math.h>
-
-START_NAMESPACE_STIR
-
-END_NAMESPACE_STIR

From 041f6debf99e8dca7b0361c17a6ca1c2dc9bb416 Mon Sep 17 00:00:00 2001
From: Kris Thielemans <k.thielemans@ucl.ac.uk>
Date: Fri, 20 Dec 2024 00:55:20 +0000
Subject: [PATCH 2/2] clean-up some unnecessary #includes and code

ProjDataInfoBlocksOnCylindrical is identical to ProjDataInfoGeneric
and should not be used any more.

I removed some dynamic checks on ProjDataInfoBlocksOnCylindrical
and use the fall-back to generic. Note however that this code was
never used, as currently the ProjDataInfoCylindrical case will
always be used.
---
 documentation/release_6.3.htm                 |  1 +
 src/IO/interfile.cxx                          | 69 +++----------------
 ...ojDataInfoBlocksOnCylindricalNoArcCorr.cxx |  1 +
 src/buildblock/VoxelsOnCartesianGrid.cxx      | 21 +-----
 ...ProjDataInfoBlocksOnCylindricalNoArcCorr.h | 40 ++---------
 5 files changed, 18 insertions(+), 114 deletions(-)

diff --git a/documentation/release_6.3.htm b/documentation/release_6.3.htm
index 47894a9454..0d34037d6c 100644
--- a/documentation/release_6.3.htm
+++ b/documentation/release_6.3.htm
@@ -101,6 +101,7 @@ <h3>Changed functionality</h3>
 <ul>
   <li>
     Made 2 (deprecated) members of <code>ProjDataInfoBlocksOnCylindricalNoArcCorr</code> <code>private</code>
+    and do some clean-up of which files to include.
     <br>
     <a href="https://github.com/UCL/STIR/pull/1556">PR #1556</a>
   </li>
diff --git a/src/IO/interfile.cxx b/src/IO/interfile.cxx
index e7565fb549..e47ffa0eb8 100644
--- a/src/IO/interfile.cxx
+++ b/src/IO/interfile.cxx
@@ -1334,12 +1334,11 @@ write_basic_interfile_PDFS_header(const string& header_file_name, const string&
   else
     {
       // !author Parisa Khateri
-      const shared_ptr<const ProjDataInfoBlocksOnCylindrical> proj_data_info_sptr
-          = dynamic_pointer_cast<const ProjDataInfoBlocksOnCylindrical>(pdfs.get_proj_data_info_sptr());
+      // generic (and hence BlocksOnCylindrical) scanner
+      const auto proj_data_info_sptr = dynamic_pointer_cast<const ProjDataInfoGeneric>(pdfs.get_proj_data_info_sptr());
 
       if (proj_data_info_sptr != NULL)
         {
-          // BlocksOncylindrical scanners
           output_header << "minimum ring difference per segment := ";
           {
             std::vector<int>::const_iterator seg = segment_sequence.begin();
@@ -1359,66 +1358,18 @@ write_basic_interfile_PDFS_header(const string& header_file_name, const string&
           }
 
           const Scanner& scanner = *proj_data_info_sptr->get_scanner_ptr();
-#if 0 // KT commented out. currently no get_ring_radius() anymore
-        if (fabs(proj_data_info_sptr->get_ring_radius()-
-        scanner.get_effective_ring_radius()) > .1)
-      warning("write_basic_interfile_PDFS_header: inconsistent effective ring radius:\n"
-         "\tproj_data_info has %g, scanner has %g.\n"
-         "\tThis really should not happen and signifies a bug.\n"
-         "\tYou will have a problem reading this data back in.",
-         proj_data_info_sptr->get_ring_radius(),
-         scanner.get_effective_ring_radius());
-#endif
-          if (fabs(proj_data_info_sptr->get_ring_spacing() - scanner.get_ring_spacing()) > .1)
-            warning("write_basic_interfile_PDFS_header: inconsistent ring spacing:\n"
-                    "\tproj_data_info has %g, scanner has %g.\n"
-                    "\tThis really should not happen and signifies a bug.\n"
-                    "\tYou will have a problem reading this data back in.",
-                    proj_data_info_sptr->get_ring_spacing(),
-                    scanner.get_ring_spacing());
 
           output_header << scanner.parameter_info();
 
-        }  // end of BlocksOnCylindrical scanner
-      else // generic scanner
+        } // end generic scanner
+      else if (!dynamic_pointer_cast<const ProjDataInfoSubsetByView>(pdfs.get_proj_data_info_sptr()))
         {
-          const shared_ptr<const ProjDataInfoGeneric> proj_data_info_sptr
-              = dynamic_pointer_cast<const ProjDataInfoGeneric>(pdfs.get_proj_data_info_sptr());
-
-          if (proj_data_info_sptr != NULL)
-            {
-              output_header << "minimum ring difference per segment := ";
-              {
-                std::vector<int>::const_iterator seg = segment_sequence.begin();
-                output_header << "{ " << proj_data_info_sptr->get_min_ring_difference(*seg);
-                for (seg++; seg != segment_sequence.end(); seg++)
-                  output_header << "," << proj_data_info_sptr->get_min_ring_difference(*seg);
-                output_header << "}\n";
-              }
-
-              output_header << "maximum ring difference per segment := ";
-              {
-                std::vector<int>::const_iterator seg = segment_sequence.begin();
-                output_header << "{ " << proj_data_info_sptr->get_max_ring_difference(*seg);
-                for (seg++; seg != segment_sequence.end(); seg++)
-                  output_header << "," << proj_data_info_sptr->get_max_ring_difference(*seg);
-                output_header << "}\n";
-              }
-
-              const Scanner& scanner = *proj_data_info_sptr->get_scanner_ptr();
-
-              output_header << scanner.parameter_info();
-
-            } // end generic scanner
-          else if (!dynamic_pointer_cast<const ProjDataInfoSubsetByView>(pdfs.get_proj_data_info_sptr()))
-            {
-              error("write_basic_interfile_PDFS_header: cannot write subset data yet. Sorry");
-            }
-          else
-            {
-              error("write_basic_interfile_PDFS_header: Error casting the projdata to one of its geometries: "
-                    "Cylindrical/BlocksOnCylindrical/Generic");
-            }
+          error("write_basic_interfile_PDFS_header: cannot write subset data yet. Sorry");
+        }
+      else
+        {
+          error("write_basic_interfile_PDFS_header: Error casting the projdata to one of its geometries: "
+                "Cylindrical/BlocksOnCylindrical/Generic");
         }
     }
 
diff --git a/src/buildblock/ProjDataInfoBlocksOnCylindricalNoArcCorr.cxx b/src/buildblock/ProjDataInfoBlocksOnCylindricalNoArcCorr.cxx
index 7e45fba125..531bff87a3 100644
--- a/src/buildblock/ProjDataInfoBlocksOnCylindricalNoArcCorr.cxx
+++ b/src/buildblock/ProjDataInfoBlocksOnCylindricalNoArcCorr.cxx
@@ -29,6 +29,7 @@
 #include "stir/LORCoordinates.h"
 #include "stir/round.h"
 #include "stir/DetectionPosition.h"
+#include "stir/DetectionPositionPair.h"
 #include "stir/error.h"
 #include <iostream>
 #include <fstream>
diff --git a/src/buildblock/VoxelsOnCartesianGrid.cxx b/src/buildblock/VoxelsOnCartesianGrid.cxx
index 2bfb2e54ec..e7a8bbfbd7 100644
--- a/src/buildblock/VoxelsOnCartesianGrid.cxx
+++ b/src/buildblock/VoxelsOnCartesianGrid.cxx
@@ -77,28 +77,9 @@ find_sampling_and_z_size(float& z_sampling, float& s_sampling, int& z_size, cons
                      ? proj_data_info_cyl_ptr->get_num_axial_poss(0)
                      : 2 * proj_data_info_cyl_ptr->get_num_axial_poss(0) - 1;
     }
-  else if (const ProjDataInfoBlocksOnCylindrical* proj_data_info_blk_ptr
-           = dynamic_cast<const ProjDataInfoBlocksOnCylindrical*>(proj_data_info_ptr))
-    {
-      // the case of BlocksOnCylindrical data
-
-      z_sampling = proj_data_info_blk_ptr->get_ring_spacing() / 2;
-
-      // for 'span>1' case, we take z_size = number of sinograms in segment 0
-      // for 'span==1' case, we take 2*num_rings-1
-
-      // first check if we have segment 0
-      assert(proj_data_info_blk_ptr->get_min_segment_num() <= 0);
-      assert(proj_data_info_blk_ptr->get_max_segment_num() >= 0);
-
-      if (z_size < 0)
-        z_size = proj_data_info_blk_ptr->get_max_ring_difference(0) > proj_data_info_blk_ptr->get_min_ring_difference(0)
-                     ? proj_data_info_blk_ptr->get_num_axial_poss(0)
-                     : 2 * proj_data_info_blk_ptr->get_num_axial_poss(0) - 1;
-    }
   else if (const ProjDataInfoGeneric* proj_data_info_gen_ptr = dynamic_cast<const ProjDataInfoGeneric*>(proj_data_info_ptr))
     {
-      // the case of Generic data
+      // the case of Generic (and therefore BlocksOnCylindrical) data
 
       z_sampling = proj_data_info_gen_ptr->get_ring_spacing() / 2;
 
diff --git a/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h b/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h
index 608f0cc800..bb79a0153f 100644
--- a/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h
+++ b/src/include/stir/ProjDataInfoBlocksOnCylindricalNoArcCorr.h
@@ -24,9 +24,6 @@
 #define __stir_ProjDataInfoBlocksOnCylindricalNoArcCorr_H__
 
 #include "stir/ProjDataInfoGenericNoArcCorr.h"
-#include "stir/ProjDataInfoBlocksOnCylindrical.h"
-#include "stir/GeometryBlocksOnCylindrical.h"
-#include "stir/DetectionPositionPair.h"
 #include "stir/VectorWithOffset.h"
 #include "stir/CartesianCoordinate3D.h"
 
@@ -40,39 +37,12 @@ class BlocksTests;
   \ingroup projdata
   \brief Projection data info for data from a scanner with discrete dtectors organised by blocks
 
-  This class also contains some functions specific for (static) full-ring PET
+  This class also contains 2 deprecated functions specific for (static) full-ring PET
   scanners. In this case, it is assumed that for 'raw' data (i.e. no mashing)
-  sinogram space is 'interleaved': 2 adjacent LOR_angles are
-  merged to 1 'view', while the corresponding bins are
-  interleaved:
-
-  \verbatim
-     before interleaving               after interleaving
-     a00     a01     a02 ...      view 0: a00 a10 a01 a11  ...
-         a10     a11     ...
-     a20     a21     a22 ...      view 1: a20 a30 a21 a31 ...
-         a30     a31     ...
-         \endverbatim
-  This (standard) interleaving is done because for 'odd' LOR_angles there
-  is no LOR which goes through the origin.
-
-
-  \par Interchanging the 2 detectors
-
-  When the ring difference = 0 (i.e. a 2D - or direct - sinogram),
-  interchanging the 2 detectors does not change the LOR. This is why
-  (in 2D) one gets away with a full sinogram size of
-  num_views * 2 * num_views, where the size of 'detector-space' is
-  twice as large.
-  However, in 3D, interchanging the detectors, also interchanges the
-  rings. One has 2 options:
-  - have 1 sinogram with twice as many views, together with the rings
-    as 'unordered pair' (i.e. ring_difference is always >0)
-  - have 2 sinograms of the same size as in 2D, together with the rings
-    as 'ordered pair' (i.e. ring_difference can be positive and negative).
-  In STIR, we use the second convention.
-
-  */
+  sinogram space is 'interleaved'. See documentation for ProjDataInfoCylindricalNoArcCorr.
+
+  \deprecated This class will be removed in v7.0.
+*/
 class ProjDataInfoBlocksOnCylindricalNoArcCorr : public ProjDataInfoGenericNoArcCorr
 {
 private: