Refactor save_image into save_image_impl finally done!!

Signed-off-by: Omar Shrit <omar@shrit.me>

src/mlpack/core/data/CMakeLists.txt

@@ -25,6 +25,7 @@ set(SOURCES
   save.hpp
   save_impl.hpp
   save_image.hpp
+  save_image_impl.hpp
   split_data.hpp
   string_algorithms.hpp
   imputer.hpp

src/mlpack/core/data/load_image.hpp

@@ -23,7 +23,7 @@
 #define STB_IMAGE_IMPLEMENTATION
 #include <stb_image.h>
 
-#endif
+#endif // HAS_STB
 
 namespace mlpack {
 namespace data {

src/mlpack/core/data/load_image_impl.hpp

@@ -2,7 +2,7 @@
  * @file core/data/load_image_impl.hpp
  * @author Mehul Kumar Nirala
  *
- * An image loading utility implementation.
+ * An image loading utility implementation via STB.
  *
  * mlpack is free software; you may redistribute it and/or modify it under the
  * terms of the 3-clause BSD license.  You should have received a copy of the
@@ -188,7 +188,7 @@ inline bool LoadImage(const std::string& /* filename */,
 
 #endif
 
-}
-}
+} // namespace data
+} // namespace mlpack
 
 #endif

src/mlpack/core/data/load_impl.hpp

@@ -16,13 +16,12 @@
 // In case it hasn't already been included.
 #include "load.hpp"
 
-#include <exception>
 #include <algorithm>
+#include <exception>
 #include <mlpack/core/util/timers.hpp>
 
 #include "extension.hpp"
 #include "detect_file_type.hpp"
-
 #include "string_algorithms.hpp"
 
 namespace mlpack {

src/mlpack/core/data/save.hpp

@@ -14,6 +14,7 @@
 #ifndef MLPACK_CORE_DATA_SAVE_HPP
 #define MLPACK_CORE_DATA_SAVE_HPP
 
+#include <mlpack/prereqs.hpp>
 #include <mlpack/core/util/log.hpp>
 #include <mlpack/core/arma_extend/arma_extend.hpp> // Includes Armadillo.
 #include <string>
@@ -21,6 +22,7 @@
 #include "format.hpp"
 #include "image_info.hpp"
 #include "detect_file_type.hpp"
+#include "save_image.hpp"
 
 namespace mlpack {
 namespace data /** Functions to load and save matrices. */ {
@@ -130,49 +132,10 @@ bool Save(const std::string& filename,
           const bool fatal = false,
           format f = format::autodetect);
 
-/**
- * Save the image file from the given matrix.
- *
- * @param filename Name of the image file.
- * @param matrix Matrix to save the image from.
- * @param info An object of ImageInfo class.
- * @param fatal If an error should be reported as fatal (default false).
- * @return Boolean value indicating success or failure of load.
- */
-template<typename eT>
-bool Save(const std::string& filename,
-          arma::Mat<eT>& matrix,
-          ImageInfo& info,
-          const bool fatal = false);
-
-/**
- * Save the image file from the given matrix.
- *
- * @param files A vector consisting of filenames.
- * @param matrix Matrix to save the image from.
- * @param info An object of ImageInfo class.
- * @param fatal If an error should be reported as fatal (default false).
- * @return Boolean value indicating success or failure of load.
- */
-template<typename eT>
-bool Save(const std::vector<std::string>& files,
-          arma::Mat<eT>& matrix,
-          ImageInfo& info,
-          const bool fatal = false);
-
-/**
- * Helper function to save files.  Implementation in save_image.hpp.
- */
-inline bool SaveImage(const std::string& filename,
-                      arma::Mat<unsigned char>& image,
-                      ImageInfo& info,
-                      const bool fatal = false);
-
 } // namespace data
 } // namespace mlpack
 
 // Include implementation.
 #include "save_impl.hpp"
-#include "save_image.hpp"
 
 #endif

src/mlpack/core/data/save_image.hpp

@@ -1,8 +1,8 @@
 /**
- * @file core/data/save_image.hpp
- * @author Mehul Kumar Nirala
+ * @file core/data/save_image_impl.hpp
+ * @author Ryan Curtin
  *
- * Implementation of image saving functionality via STB.
+ * Implementation of save functionality.
  *
  * mlpack is free software; you may redistribute it and/or modify it under the
  * terms of the 3-clause BSD license.  You should have received a copy of the
@@ -12,138 +12,61 @@
 #ifndef MLPACK_CORE_DATA_SAVE_IMAGE_HPP
 #define MLPACK_CORE_DATA_SAVE_IMAGE_HPP
 
-#include "save.hpp"
+#include "image_info.hpp"
 
 #ifdef HAS_STB
 
-// Include STB functions.  Note that we include the implementations, too, and
-// all functions will be marked as static.
 #define STB_IMAGE_WRITE_STATIC
 #define STB_IMAGE_WRITE_IMPLEMENTATION
 #include <stb_image_write.h>
 
+#endif // HAS_STB
+
 namespace mlpack {
 namespace data {
 
+/**
+ * Save the image file from the given matrix.
+ *
+ * @param filename Name of the image file.
+ * @param matrix Matrix to save the image from.
+ * @param info An object of ImageInfo class.
+ * @param fatal If an error should be reported as fatal (default false).
+ * @return Boolean value indicating success or failure of load.
+ */
+template<typename eT>
+bool Save(const std::string& filename,
+          arma::Mat<eT>& matrix,
+          ImageInfo& info,
+          const bool fatal = false);
+
+/**
+ * Save the image file from the given matrix.
+ *
+ * @param files A vector consisting of filenames.
+ * @param matrix Matrix to save the image from.
+ * @param info An object of ImageInfo class.
+ * @param fatal If an error should be reported as fatal (default false).
+ * @return Boolean value indicating success or failure of load.
+ */
+template<typename eT>
+bool Save(const std::vector<std::string>& files,
+          arma::Mat<eT>& matrix,
+          ImageInfo& info,
+          const bool fatal = false);
+
+/**
+ * Helper function to save files.  Implementation in save_image.hpp.
+ */
 inline bool SaveImage(const std::string& filename,
                       arma::Mat<unsigned char>& image,
                       ImageInfo& info,
-                      const bool fatal)
-{
-  // Check to see if the file type is supported.
-  if (!ImageFormatSupported(filename, true))
-  {
-    std::ostringstream oss;
-    oss << "Save(): file type " << Extension(filename) << " not supported.\n";
-    oss << "Currently image saving supports ";
-    for (auto extension : SaveFileTypes())
-      oss << ", " << extension;
-    oss << "." << std::endl;
+                      const bool fatal = false);
 
-    if (fatal)
-    {
-      Log::Fatal << oss.str();
-    }
-    else
-    {
-      Log::Warn << oss.str();
-    }
+} //namespace data
+} //namespace mlpack
 
-    return false;
-  }
-
-  // Ensure the shape of the matrix is correct.
-  if (image.n_cols > 1)
-  {
-    Log::Warn << "Save(): given input image matrix contains more than 1 image."
-        << std::endl;
-    Log::Warn << "Only the first image will be saved!" << std::endl;
-  }
-
-  if (info.Width() * info.Height() * info.Channels() != image.n_elem)
-  {
-    Log::Fatal << "data::Save(): The given image dimensions do not match the "
-        << "dimensions of the matrix to be saved!" << std::endl;
-  }
-
-  bool status = false;
-  unsigned char* imageMem = image.memptr();
-
-  if ("png" == Extension(filename))
-  {
-    status = stbi_write_png(filename.c_str(), info.Width(), info.Height(),
-        info.Channels(), imageMem, info.Width() * info.Channels());
-  }
-  else if ("bmp" == Extension(filename))
-  {
-    status = stbi_write_bmp(filename.c_str(), info.Width(), info.Height(),
-        info.Channels(), imageMem);
-  }
-  else if ("tga" == Extension(filename))
-  {
-    status = stbi_write_tga(filename.c_str(), info.Width(), info.Height(),
-        info.Channels(), imageMem);
-  }
-  else if ("hdr" == Extension(filename))
-  {
-    // We'll have to convert to float...
-    arma::fmat tmpImage = arma::conv_to<arma::fmat>::from(image);
-    status = stbi_write_hdr(filename.c_str(), info.Width(), info.Height(),
-        info.Channels(), tmpImage.memptr());
-  }
-  else if ("jpg" == Extension(filename))
-  {
-    status = stbi_write_jpg(filename.c_str(), info.Width(), info.Height(),
-        info.Channels(), imageMem, info.Quality());
-  }
-
-  if (!status)
-  {
-    if (fatal)
-    {
-      Log::Fatal << "Save(): error saving image to '" << filename << "'."
-          << std::endl;
-    }
-    else
-    {
-      Log::Warn << "Save(): error saving image to '" << filename << "'."
-          << std::endl;
-    }
-  }
-
-  return status;
-}
-
-} // namespace data
-} // namespace mlpack
-
-#else
-
-namespace mlpack {
-namespace data {
-
-inline bool SaveImage(const std::string& /* filename */,
-                      arma::Mat<unsigned char>& /* image */,
-                      ImageInfo& /* info */,
-                      const bool fatal)
-{
-  if (fatal)
-  {
-    Log::Fatal << "Save(): mlpack was not compiled with STB support, so images "
-        << "cannot be saved!" << std::endl;
-  }
-  else
-  {
-    Log::Warn << "Save(): mlpack was not compiled with STB support, so images "
-        << "cannot be saved!" << std::endl;
-  }
-
-  return false;
-}
-
-} // namespace data
-} // namespace mlpack
-
-#endif
+// Include implementation of Save() for images.
+#include "save_image_impl.hpp"
 
 #endif

src/mlpack/core/data/save_image_impl.hpp

@@ -0,0 +1,195 @@
+/**
+ * @file core/data/save_image_impl.hpp
+ * @author Mehul Kumar Nirala
+ *
+ * Implementation of image saving functionality via STB.
+ *
+ * mlpack is free software; you may redistribute it and/or modify it under the
+ * terms of the 3-clause BSD license.  You should have received a copy of the
+ * 3-clause BSD license along with mlpack.  If not, see
+ * http://www.opensource.org/licenses/BSD-3-Clause for more information.
+ */
+#ifndef MLPACK_CORE_DATA_SAVE_IMAGE_IMPL_HPP
+#define MLPACK_CORE_DATA_SAVE_IMAGE_IMPL_HPP
+
+// In case it hasn't been included yet.
+#include "save_image.hpp"
+#include "image_info.hpp"
+
+namespace mlpack {
+namespace data {
+
+/**
+ * Save the given image to the given filename.
+ *
+ * @param filename Filename to save to.
+ * @param matrix Matrix containing image to be saved.
+ * @param info Information about the image (width/height/channels/etc.).
+ * @param fatal Whether an exception should be thrown on save failure.
+ */
+template<typename eT>
+bool Save(const std::string& filename,
+          arma::Mat<eT>& matrix,
+          ImageInfo& info,
+          const bool fatal)
+{
+  arma::Mat<unsigned char> tmpMatrix =
+      arma::conv_to<arma::Mat<unsigned char>>::from(matrix);
+
+  return SaveImage(filename, tmpMatrix, info, fatal);
+}
+
+// Image saving API for multiple files.
+template<typename eT>
+bool Save(const std::vector<std::string>& files,
+          arma::Mat<eT>& matrix,
+          ImageInfo& info,
+          const bool fatal)
+{
+  if (files.size() == 0)
+  {
+    if (fatal)
+    {
+      Log::Fatal << "Save(): vector of image files is empty; nothing to save."
+          << std::endl;
+    }
+    else
+    {
+      Log::Warn << "Save(): vector of image files is empty; nothing to save."
+          << std::endl;
+    }
+
+    return false;
+  }
+
+  arma::Mat<unsigned char> img;
+  bool status = true;
+
+  for (size_t i = 0; i < files.size() ; ++i)
+  {
+    arma::Mat<eT> colImg(matrix.colptr(i), matrix.n_rows, 1,
+        false, true);
+    status &= Save(files[i], colImg, info, fatal);
+  }
+
+  return status;
+}
+
+#ifdef HAS_STB
+
+inline bool SaveImage(const std::string& filename,
+                      arma::Mat<unsigned char>& image,
+                      ImageInfo& info,
+                      const bool fatal)
+{
+  // Check to see if the file type is supported.
+  if (!ImageFormatSupported(filename, true))
+  {
+    std::ostringstream oss;
+    oss << "Save(): file type " << Extension(filename) << " not supported.\n";
+    oss << "Currently image saving supports ";
+    for (auto extension : SaveFileTypes())
+      oss << ", " << extension;
+    oss << "." << std::endl;
+
+    if (fatal)
+    {
+      Log::Fatal << oss.str();
+    }
+    else
+    {
+      Log::Warn << oss.str();
+    }
+
+    return false;
+  }
+
+  // Ensure the shape of the matrix is correct.
+  if (image.n_cols > 1)
+  {
+    Log::Warn << "Save(): given input image matrix contains more than 1 image."
+        << std::endl;
+    Log::Warn << "Only the first image will be saved!" << std::endl;
+  }
+
+  if (info.Width() * info.Height() * info.Channels() != image.n_elem)
+  {
+    Log::Fatal << "data::Save(): The given image dimensions do not match the "
+        << "dimensions of the matrix to be saved!" << std::endl;
+  }
+
+  bool status = false;
+  unsigned char* imageMem = image.memptr();
+
+  if ("png" == Extension(filename))
+  {
+    status = stbi_write_png(filename.c_str(), info.Width(), info.Height(),
+        info.Channels(), imageMem, info.Width() * info.Channels());
+  }
+  else if ("bmp" == Extension(filename))
+  {
+    status = stbi_write_bmp(filename.c_str(), info.Width(), info.Height(),
+        info.Channels(), imageMem);
+  }
+  else if ("tga" == Extension(filename))
+  {
+    status = stbi_write_tga(filename.c_str(), info.Width(), info.Height(),
+        info.Channels(), imageMem);
+  }
+  else if ("hdr" == Extension(filename))
+  {
+    // We'll have to convert to float...
+    arma::fmat tmpImage = arma::conv_to<arma::fmat>::from(image);
+    status = stbi_write_hdr(filename.c_str(), info.Width(), info.Height(),
+        info.Channels(), tmpImage.memptr());
+  }
+  else if ("jpg" == Extension(filename))
+  {
+    status = stbi_write_jpg(filename.c_str(), info.Width(), info.Height(),
+        info.Channels(), imageMem, info.Quality());
+  }
+
+  if (!status)
+  {
+    if (fatal)
+    {
+      Log::Fatal << "Save(): error saving image to '" << filename << "'."
+          << std::endl;
+    }
+    else
+    {
+      Log::Warn << "Save(): error saving image to '" << filename << "'."
+          << std::endl;
+    }
+  }
+
+  return status;
+}
+
+#else // HAS_STB
+
+inline bool SaveImage(const std::string& /* filename */,
+                      arma::Mat<unsigned char>& /* image */,
+                      ImageInfo& /* info */,
+                      const bool fatal)
+{
+  if (fatal)
+  {
+    Log::Fatal << "Save(): mlpack was not compiled with STB support, so images "
+        << "cannot be saved!" << std::endl;
+  }
+  else
+  {
+    Log::Warn << "Save(): mlpack was not compiled with STB support, so images "
+        << "cannot be saved!" << std::endl;
+  }
+
+  return false;
+}
+
+#endif
+
+} // namespace data
+} // namespace mlpack
+
+#endif

src/mlpack/core/data/save_impl.hpp

@@ -16,10 +16,6 @@
 #include "save.hpp"
 #include "extension.hpp"
 
-#include <cereal/archives/xml.hpp>
-#include <cereal/archives/json.hpp>
-#include <cereal/archives/binary.hpp>
-
 namespace mlpack {
 namespace data {
 
@@ -344,62 +340,6 @@ bool Save(const std::string& filename,
   }
 }
 
-/**
- * Save the given image to the given filename.
- *
- * @param filename Filename to save to.
- * @param matrix Matrix containing image to be saved.
- * @param info Information about the image (width/height/channels/etc.).
- * @param fatal Whether an exception should be thrown on save failure.
- */
-template<typename eT>
-bool Save(const std::string& filename,
-          arma::Mat<eT>& matrix,
-          ImageInfo& info,
-          const bool fatal)
-{
-  arma::Mat<unsigned char> tmpMatrix =
-      arma::conv_to<arma::Mat<unsigned char>>::from(matrix);
-
-  return SaveImage(filename, tmpMatrix, info, fatal);
-}
-
-// Image saving API for multiple files.
-template<typename eT>
-bool Save(const std::vector<std::string>& files,
-          arma::Mat<eT>& matrix,
-          ImageInfo& info,
-          const bool fatal)
-{
-  if (files.size() == 0)
-  {
-    if (fatal)
-    {
-      Log::Fatal << "Save(): vector of image files is empty; nothing to save."
-          << std::endl;
-    }
-    else
-    {
-      Log::Warn << "Save(): vector of image files is empty; nothing to save."
-          << std::endl;
-    }
-
-    return false;
-  }
-
-  arma::Mat<unsigned char> img;
-  bool status = true;
-
-  for (size_t i = 0; i < files.size() ; ++i)
-  {
-    arma::Mat<eT> colImg(matrix.colptr(i), matrix.n_rows, 1,
-        false, true);
-    status &= Save(files[i], colImg, info, fatal);
-  }
-
-  return status;
-}
-
 } // namespace data
 } // namespace mlpack