Merge pull request #1316 from SamFlt/python_build_improvements

[WIP] Python build improvements

CMakeLists.txt

@@ -269,39 +269,93 @@ endif()
 
 # --- Python Support ---
 if(NOT IOS)
-  include(cmake/VISPDetectPython.cmake)
+  # Make sure to refresh the python interpreter every time we rerun cmake
+  # If we don't do this, we may use an old or invalid python when installing the bindings
+  # that was cached by a previous attempt at building
+  if(CMAKE_VERSION VERSION_LESS "3.15.0")
+    set(PYTHON3_CACHE_LIST
+      PYTHON3INTERP_FOUND PYTHONINTERP_FOUND PYTHONLIBS_FOUND PYTHON_FOUND
+      PYTHON3_EXECUTABLE PYTHON_EXECUTABLE
+    )
+    foreach (_variableName ${PYTHON3_CACHE_LIST})
+      unset(${_variableName} CACHE)
+    endforeach()
+    include(cmake/VISPDetectPython.cmake)
+  else()
+    set(PYTHON3_CACHE_LIST
+        Python3_FOUND Python3_EXECUTABLE Python3_Interpreter_FOUND Python3_LIBRARIES
+        _Python3_EXECUTABLE _Python3_INCLUDE_DIR _Python3_INTERPRETER_PROPERTIES _Python3_LIBRARY_RELEASE
+    )
+    foreach (_variableName ${PYTHON3_CACHE_LIST})
+      unset(${_variableName} CACHE)
+    endforeach()
+    # Find strategy
+    set(Python3_FIND_REGISTRY LAST)
+    set(Python3_FIND_VIRTUALENV FIRST)
+    set(Python3_FIND_STRATEGY LOCATION)
+    find_package (Python3 COMPONENTS Interpreter Development)
+
+    # Alias variables to be consistent with previous detection method
+    set(PYTHON3_FOUND ${Python3_FOUND})
+    set(PYTHON3_EXECUTABLE ${Python3_EXECUTABLE})
+    set(PYTHON_DEFAULT_EXECUTABLE ${PYTHON3_EXECUTABLE})
+    set(PYTHON3INTERP_FOUND ${Python3_Interpreter_FOUND})
+    set(PYTHON3_VERSION_STRING ${Python3_VERSION})
+  endif()
 endif()
 
-if(CMAKE_VERSION VERSION_LESS "3.19.0")
+# --- Python Bindings requirements ---
+VP_OPTION(USE_PYBIND11       pybind11       QUIET    "Include pybind11 to create Python bindings" "" ON)
+
+# Minimum tool versions
+set(CMAKE_MINIMUM_VERSION_PYTHON_BINDINGS "3.19.0")
+set(PYTHON3_MINIMUM_VERSION_PYTHON_BINDINGS "3.7.0")
+if(CMAKE_VERSION VERSION_LESS ${CMAKE_MINIMUM_VERSION_PYTHON_BINDINGS})
   set(CMAKE_NOT_OK_FOR_BINDINGS TRUE)
+  message(STATUS "Required CMake version for Python bindings is ${CMAKE_MINIMUM_VERSION_PYTHON_BINDINGS},
+  but you have ${CMAKE_VERSION}.
+  Python bindings generation will be deactivated.
+  ")
 else()
   set(CMAKE_NOT_OK_FOR_BINDINGS FALSE)
 endif()
 
-if(CMAKE_NOT_OK_FOR_BINDINGS)
-  status("${CMAKE_NOT_OK_FOR_BINDINGS}")
-  status("CMake version required for Python bindings is 3.19.0, but you have ${CMAKE_VERSION}. Python bindings generation will be deactivated")
+if(PYTHON3_VERSION_STRING VERSION_LESS ${PYTHON3_MINIMUM_VERSION_PYTHON_BINDINGS})
+  set(PYTHON3_NOT_OK_FOR_BINDINGS TRUE)
+  message(STATUS "Required Python version for Python bindings is ${PYTHON3_MINIMUM_VERSION_PYTHON_BINDINGS},
+  but you have ${PYTHON3_VERSION_STRING}.
+  Python bindings generation will be deactivated.
+  ")
+else()
+  set(PYTHON3_NOT_OK_FOR_BINDINGS FALSE)
+endif()
+if(VISP_CXX_STANDARD LESS VISP_CXX_STANDARD_17)
+  set(CXX_STANDARD_NOT_OK_FOR_BINDINGS TRUE)
+  message(STATUS "Required C++ standard is C++17, but you have ${VISP_CXX_STANDARD}")
+else()
+  set(CXX_STANDARD_NOT_OK_FOR_BINDINGS FALSE)
 endif()
 
-# --- Python Bindings requirements ---
 
-# this avoids non-active conda from getting picked anyway on Windows
-#set(Python_FIND_REGISTRY LAST)
-# Use environment variable PATH to decide preference for Python
-#set(Python_FIND_VIRTUALENV FIRST)
-#set(Python_FIND_STRATEGY LOCATION)
-
-#find_package(Python 3.7 COMPONENTS Interpreter Development) # TODO: use visp function to find python?
-#if(Python_FOUND)
-#  set(VISP_PYTHON_BINDINGS_EXECUTABLE "${Python_EXECUTABLE}")
-#endif()
-#find_package(pybind11)
-VP_OPTION(USE_PYBIND11       pybind11       QUIET    "Include pybind11 to create Python bindings" "" ON)
 
-#if(pybind11_FOUND)
-#  set(VISP_PYBIND11_DIR "${pybind11_DIR}")
-#endif()
-#message("${pybind11_FOUND}")
+# Forbid system Python
+if(DEFINED ENV{VIRTUAL_ENV} OR DEFINED ENV{CONDA_PREFIX})
+  set(_pip_args)
+  set(VISP_PYTHON_IS_SYSTEM_WIDE FALSE)
+else()
+  # First solution: raise an error when cmake will call pip install
+  # set(_pip_args "--require-virtualenv") # If this is a system python, throw an error
+  if(PYTHON3_FOUND)
+    message(STATUS "The python version that you are using (${PYTHON3_EXECUTABLE}) is the system interpreter.
+    pip packages should not be installed system-wide!
+    Python bindings targets will be deactivated!
+    To reenable them, install conda or virtualenv,
+    delete the CMakeCache file then rerun cmake when inside the virtual environment.
+    ")
+    set(VISP_PYTHON_IS_SYSTEM_WIDE TRUE)
+  endif()
+endif()
+
 # ---
 
 
@@ -447,7 +501,7 @@ VP_OPTION(BUILD_ANDROID_EXAMPLES  "" "" "Build examples for Android platform"
 VP_OPTION(INSTALL_ANDROID_EXAMPLES "" "" "Install Android examples"  ""                  OFF IF ANDROID )
 
 # Build python bindings as an option
-VP_OPTION(BUILD_PYTHON_BINDINGS  "" "" "Build Python bindings" "" ON IF (PYTHON3INTERP_FOUND AND USE_PYBIND11 AND NOT CMAKE_NOT_OK_FOR_BINDINGS) )
+VP_OPTION(BUILD_PYTHON_BINDINGS  "" "" "Build Python bindings" "" ON IF (PYTHON3INTERP_FOUND AND USE_PYBIND11 AND NOT CMAKE_NOT_OK_FOR_BINDINGS AND NOT VISP_PYTHON_IS_SYSTEM_WIDE AND NOT PYTHON3_NOT_OK_FOR_BINDINGS AND NOT CXX_STANDARD_NOT_OK_FOR_BINDINGS) )
 VP_OPTION(BUILD_PYTHON_BINDINGS_DOC  "" "" "Build the documentation for the Python bindings" "" ON IF BUILD_PYTHON_BINDINGS )
 
 
@@ -1623,8 +1677,17 @@ if(BUILD_PYTHON_BINDINGS)
   status("    Package version:"        "${VISP_PYTHON_PACKAGE_VERSION}")
   status("    Wrapped modules:"        "${VISP_PYTHON_BOUND_MODULES}")
   status("    Generated input config:" "${VISP_PYTHON_GENERATED_CONFIG_FILE}")
+else()
+  status("    Requirements: ")
+  status("      Python version > ${PYTHON3_MINIMUM_VERSION_PYTHON_BINDINGS}:"     PYTHON3_FOUND AND NOT PYTHON3_NOT_OK_FOR_BINDINGS THEN "ok (ver ${PYTHON3_VERSION_STRING})" ELSE "python not found or too old (${PYTHON3_VERSION_STRING})")
+  status("      Python in Virtual environment or conda:"     VISP_PYTHON_IS_SYSTEM_WIDE THEN "failed" ELSE "ok")
+  status("      Pybind11 found:"     USE_PYBIND11 THEN "ok" ELSE "failed")
+  status("      CMake > ${CMAKE_MINIMUM_VERSION_PYTHON_BINDINGS}:"     CMAKE_NOT_OK_FOR_BINDINGS THEN "failed (${CMAKE_VERSION})" ELSE "ok (${CMAKE_VERSION})")
+  status("      C++ standard > ${VISP_CXX_STANDARD_17}:"     CXX_STANDARD_NOT_OK_FOR_BINDINGS THEN "failed (${VISP_CXX_STANDARD})" ELSE "ok (${VISP_CXX_STANDARD})")
+
 endif()
 
+
 # ============================ Options ===========================
 status("")
 status("  Build options: ")

modules/core/include/visp3/core/vpBSpline.h

@@ -57,7 +57,8 @@
   - k indicates which kth derivative is computed.
   - value is the numerical value of \f$ N_{i,p}^k(u) \f$.
 */
-typedef struct vpBasisFunction {
+typedef struct vpBasisFunction
+{
   unsigned int i;
   unsigned int p;
   double u;
@@ -104,8 +105,8 @@ typedef struct vpBasisFunction {
 
 class VISP_EXPORT vpBSpline
 {
-  public /*protected*/:
-  //! Vector wich contains the control points
+public /*protected*/:
+//! Vector wich contains the control points
   std::vector<vpImagePoint> controlPoints;
   //! Vector which contain the knots \f$ {u0, ..., um} \f$
   std::vector<double> knots;
@@ -214,24 +215,24 @@ class VISP_EXPORT vpBSpline
     }
   }
 
-  static unsigned int findSpan(double l_u, unsigned int l_p, std::vector<double> &l_knots);
-  unsigned int findSpan(double u);
+  static unsigned int findSpan(double l_u, unsigned int l_p, const std::vector<double> &l_knots);
+  unsigned int findSpan(double u) const;
 
   static vpBasisFunction *computeBasisFuns(double l_u, unsigned int l_i, unsigned int l_p,
-                                           std::vector<double> &l_knots);
-  vpBasisFunction *computeBasisFuns(double u);
+                                           const std::vector<double> &l_knots);
+  vpBasisFunction *computeBasisFuns(double u) const;
 
   static vpBasisFunction **computeDersBasisFuns(double l_u, unsigned int l_i, unsigned int l_p, unsigned int l_der,
-                                                std::vector<double> &l_knots);
-  vpBasisFunction **computeDersBasisFuns(double u, unsigned int der);
+                                                const std::vector<double> &l_knots);
+  vpBasisFunction **computeDersBasisFuns(double u, unsigned int der) const;
 
-  static vpImagePoint computeCurvePoint(double l_u, unsigned int l_i, unsigned int l_p, std::vector<double> &l_knots,
-                                        std::vector<vpImagePoint> &l_controlPoints);
-  vpImagePoint computeCurvePoint(double u);
+  static vpImagePoint computeCurvePoint(double l_u, unsigned int l_i, unsigned int l_p, const std::vector<double> &l_knots,
+                                        const std::vector<vpImagePoint> &l_controlPoints);
+  vpImagePoint computeCurvePoint(double u) const;
 
   static vpImagePoint *computeCurveDers(double l_u, unsigned int l_i, unsigned int l_p, unsigned int l_der,
-                                        std::vector<double> &l_knots, std::vector<vpImagePoint> &l_controlPoints);
-  vpImagePoint *computeCurveDers(double u, unsigned int der);
+                                        const std::vector<double> &l_knots, const std::vector<vpImagePoint> &l_controlPoints);
+  vpImagePoint *computeCurveDers(double u, unsigned int der) const;
 };
 
 #endif

modules/core/include/visp3/core/vpCannyEdgeDetection.h

@@ -43,7 +43,6 @@
 // 3rd parties include
 #ifdef VISP_HAVE_NLOHMANN_JSON
 #include <nlohmann/json.hpp>
-using json = nlohmann::json; //! json namespace shortcut
 #endif
 
 class VISP_EXPORT vpCannyEdgeDetection
@@ -213,7 +212,7 @@ class VISP_EXPORT vpCannyEdgeDetection
    * \param[in] j : The JSON object, resulting from the parsing of a JSON file.
    * \param[out] detector : The detector that will be initialized from the JSON data.
    */
-  friend inline void from_json(const json &j, vpCannyEdgeDetection &detector)
+  friend inline void from_json(const nlohmann::json &j, vpCannyEdgeDetection &detector)
   {
     std::string filteringAndGradientName = vpImageFilter::vpCannyFilteringAndGradientTypeToString(detector.m_filteringAndGradientType);
     filteringAndGradientName = j.value("filteringAndGradientType", filteringAndGradientName);
@@ -233,10 +232,10 @@ class VISP_EXPORT vpCannyEdgeDetection
    * \param[out] j : A JSON parser object.
    * \param[in] detector : The vpCannyEdgeDetection object that must be parsed into JSON format.
    */
-  friend inline void to_json(json &j, const vpCannyEdgeDetection &detector)
+  friend inline void to_json(nlohmann::json &j, const vpCannyEdgeDetection &detector)
   {
     std::string filteringAndGradientName = vpImageFilter::vpCannyFilteringAndGradientTypeToString(detector.m_filteringAndGradientType);
-    j = json {
+    j = nlohmann::json {
             {"filteringAndGradientType", filteringAndGradientName},
             {"gaussianSize", detector.m_gaussianKernelSize},
             {"gaussianStdev", detector.m_gaussianStdev},

modules/core/include/visp3/core/vpFrameGrabber.h

@@ -103,9 +103,9 @@ class VISP_EXPORT vpFrameGrabber
   /** @name Inherited functionalities from vpFramegrabber */
   //@{
   //! Return the number of rows in the image.
-  inline unsigned int getHeight() const { return height; }
+  unsigned int getHeight() const;
   //! Return the number of columns in the image.
-  inline unsigned int getWidth() const { return width; }
+  unsigned int getWidth() const;
   //@}
 
 public:

modules/core/include/visp3/core/vpImageTools.h

@@ -1133,7 +1133,7 @@ void vpImageTools::resize(const vpImage<Type> &I, vpImage<Type> &Ires, const vpI
       }
     }
   }
-  }
+}
 
 #if defined(VISP_HAVE_SIMDLIB)
 template <>
@@ -1511,24 +1511,24 @@ void vpImageTools::warpLinear(const vpImage<Type> &src, const vpMatrix &T, vpIma
             const float s = xi_ - x_;
 
             if (y_ < static_cast<int>(src.getHeight()) - 1 && x_ < static_cast<int>(src.getWidth()) - 1) {
-              const Type val00 = src[y_][x_];
-              const Type val01 = src[y_][x_ + 1];
-              const Type val10 = src[y_ + 1][x_];
-              const Type val11 = src[y_ + 1][x_ + 1];
+              const float val00 = static_cast<float>(src[y_][x_]);
+              const float val01 = static_cast<float>(src[y_][x_ + 1]);
+              const float val10 = static_cast<float>(src[y_ + 1][x_]);
+              const float val11 = static_cast<float>(src[y_ + 1][x_ + 1]);
               const float col0 = lerp(val00, val01, s);
               const float col1 = lerp(val10, val11, s);
               const float interp = lerp(col0, col1, t);
               dst[i][j] = vpMath::saturate<Type>(interp);
             }
             else if (y_ < static_cast<int>(src.getHeight()) - 1) {
-              const Type val00 = src[y_][x_];
-              const Type val10 = src[y_ + 1][x_];
+              const float val00 = static_cast<float>(src[y_][x_]);
+              const float val10 = static_cast<float>(src[y_ + 1][x_]);
               const float interp = lerp(val00, val10, t);
               dst[i][j] = vpMath::saturate<Type>(interp);
             }
             else if (x_ < static_cast<int>(src.getWidth()) - 1) {
-              const Type val00 = src[y_][x_];
-              const Type val01 = src[y_][x_ + 1];
+              const float val00 = static_cast<float>(src[y_][x_]);
+              const float val01 = static_cast<float>(src[y_][x_ + 1]);
               const float interp = lerp(val00, val01, s);
               dst[i][j] = vpMath::saturate<Type>(interp);
             }
@@ -1583,24 +1583,24 @@ void vpImageTools::warpLinear(const vpImage<Type> &src, const vpMatrix &T, vpIma
         double t = y - y_lower;
 
         if (y_lower < static_cast<int>(src.getHeight()) - 1 && x_lower < static_cast<int>(src.getWidth()) - 1) {
-          const Type val00 = src[y_lower][x_lower];
-          const Type val01 = src[y_lower][x_lower + 1];
-          const Type val10 = src[y_lower + 1][x_lower];
-          const Type val11 = src[y_lower + 1][x_lower + 1];
+          const double val00 = static_cast<double>(src[y_lower][x_lower]);
+          const double val01 = static_cast<double>(src[y_lower][x_lower + 1]);
+          const double val10 = static_cast<double>(src[y_lower + 1][x_lower]);
+          const double val11 = static_cast<double>(src[y_lower + 1][x_lower + 1]);
           const double col0 = lerp(val00, val01, s);
           const double col1 = lerp(val10, val11, s);
           const double interp = lerp(col0, col1, t);
           dst[i][j] = vpMath::saturate<Type>(interp);
         }
         else if (y_lower < static_cast<int>(src.getHeight()) - 1) {
-          const Type val00 = src[y_lower][x_lower];
-          const Type val10 = src[y_lower + 1][x_lower];
+          const double val00 = static_cast<double>(src[y_lower][x_lower]);
+          const double val10 = static_cast<double>(src[y_lower + 1][x_lower]);
           const double interp = lerp(val00, val10, t);
           dst[i][j] = vpMath::saturate<Type>(interp);
         }
         else if (x_lower < static_cast<int>(src.getWidth()) - 1) {
-          const Type val00 = src[y_lower][x_lower];
-          const Type val01 = src[y_lower][x_lower + 1];
+          const double val00 = static_cast<double>(src[y_lower][x_lower]);
+          const double val01 = static_cast<double>(src[y_lower][x_lower + 1]);
           const double interp = lerp(val00, val01, s);
           dst[i][j] = vpMath::saturate<Type>(interp);
         }

modules/core/include/visp3/core/vpMatrix.h

@@ -1060,7 +1060,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use inverseByCholeskyLapack() or inverseByCholesky().
    */
-  vpMatrix inverseByCholeskyGsl() const
+  vp_deprecated vpMatrix inverseByCholeskyGsl() const
   {
 #if defined(VISP_HAVE_LAPACK)
     return inverseByCholeskyLapack();
@@ -1072,7 +1072,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use inverseByQRLapack() or inverseByQR().
    */
-  vpMatrix inverseByQRGsl() const
+  vp_deprecated vpMatrix inverseByQRGsl() const
   {
 #if defined(VISP_HAVE_LAPACK)
     return inverseByQRLapack();
@@ -1084,7 +1084,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use pseudoInverseLapack() or pseudoInverse().
    */
-  vpMatrix pseudoInverseGsl(double svThreshold = 1e-6) const
+  vp_deprecated vpMatrix pseudoInverseGsl(double svThreshold = 1e-6) const
   {
 #if defined(VISP_HAVE_LAPACK)
     return pseudoInverseLapack(svThreshold);
@@ -1097,7 +1097,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use pseudoInverseLapack() or pseudoInverse().
    */
-  unsigned int pseudoInverseGsl(vpMatrix &Ap, double svThreshold = 1e-6) const
+  vp_deprecated unsigned int pseudoInverseGsl(vpMatrix &Ap, double svThreshold = 1e-6) const
   {
 #if defined(VISP_HAVE_LAPACK)
     return pseudoInverseLapack(Ap, svThreshold);
@@ -1111,7 +1111,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use pseudoInverseLapack() or pseudoInverse().
    */
-  unsigned int pseudoInverseGsl(vpMatrix &Ap, vpColVector &sv, double svThreshold = 1e-6) const
+  vp_deprecated unsigned int pseudoInverseGsl(vpMatrix &Ap, vpColVector &sv, double svThreshold = 1e-6) const
   {
 #if defined(VISP_HAVE_LAPACK)
     return pseudoInverseLapack(Ap, sv, svThreshold);
@@ -1126,7 +1126,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use pseudoInverseLapack() or pseudoInverse().
    */
-  unsigned int pseudoInverseGsl(vpMatrix &Ap, vpColVector &sv, double svThreshold, vpMatrix &imA, vpMatrix &imAt,
+  vp_deprecated unsigned int pseudoInverseGsl(vpMatrix &Ap, vpColVector &sv, double svThreshold, vpMatrix &imA, vpMatrix &imAt,
                                 vpMatrix &kerAt) const
   {
 #if defined(VISP_HAVE_LAPACK)
@@ -1145,7 +1145,7 @@ class VISP_EXPORT vpMatrix : public vpArray2D<double>
   /*!
      \deprecated You should rather use svdLapack() or svd().
    */
-  void svdGsl(vpColVector &w, vpMatrix &V)
+  vp_deprecated void svdGsl(vpColVector &w, vpMatrix &V)
   {
 #if defined(VISP_HAVE_LAPACK)
     svdLapack(w, V);