Merge branch 'master' into 1uc/rework-cmake

README.md

@@ -1,3 +1,8 @@
+*Note:* In preparation of `v3` of HighFive, we've started merging breaking
+changes into the main branch. More information and opportunity to comment can
+be found at:
+https://github.com/BlueBrain/HighFive/issues/864
+
 # HighFive - HDF5 header-only C++ Library
 
 [![Doxygen -> gh-pages](https://github.com/BlueBrain/HighFive/workflows/gh-pages/badge.svg?branch=master)](https://BlueBrain.github.io/HighFive/actions/workflows/gh-pages.yml?query=branch%3Amaster)

doc/migration_guide.md

@@ -14,3 +14,108 @@ replaced with an `std::vector<std::string>` (for example).
 
 If desired one can silence warnings by replacing `FixedLenStringArray` with
 `deprecated::FixedLenStringArray`.
+
+
+## Deprecation of `read(T*, ...)`.
+A "raw read" is when the user allocates sufficient bytes and provides HighFive
+with the pointer to the first byte. "Regular reads" take a detour via the
+inspector and might resize the container, etc.
+
+The issue is that HighFive `v2` had the following two overloads:
+```
+template<class T>
+DataSet::read(T& x, /* skipped */);
+
+template<class T>
+DataSet::read(T* x, /* skipped */);
+```
+and the analogous for `Attribute`.
+
+The issue is that the second overload will also match things like `T**` and
+`T[][]`. For example the following code used the removed overload:
+```
+double x[2][3];
+dset.read(x);
+```
+which is fine because is a contiguous sequence of doubles. It's equivalent to
+following `v3` code:
+```
+double x[2][3];
+dset.read_raw((double*) x);
+```
+
+### Accidental Raw Read
+We consider the example above to be accidentally using a raw read, when it
+could be performing a regular read. We suggest to not change the above, i.e.
+```
+double x[2][3];
+dset.read(x);
+```
+continues to be correct in `v3` and can check that the dimensions match. The
+inspector recognizes `double[2][3]` as a contiguous array of doubles.
+Therefore, it'll use the shallow-copy buffer and avoid the any additional
+allocations or copies.
+
+### Intentional Raw Read
+When genuinely performing a "raw read", one must replace `read` with
+`read_raw`. For example:
+
+```
+double x = malloc(2*3 * sizeof(double));
+dset.read_raw(x);
+```
+is correct in `v3`.
+
+## Reworked CMake
+In `v3` we completely rewrote the CMake code of HighFive. Since HighFive is a
+header only library, it needs to perform two tasks:
+
+1. Copy the sources during installation.
+2. Export a target that sets `-I ${HIGHFIVE_DIR}` and links with HDF5.
+
+We've removed all flags for optional dependencies, such as
+`-DHIGHFIVE_USE_BOOST`. Instead user that want to read/write into/from
+optionally supported containers, include a header with the corresponding name
+and make sure to adjust their CMake code to link with the dependency.
+
+The C++ code should have:
+```
+#include <highfive/boost.hpp>
+
+// Code the reads or write `boost::multi_array`.
+```
+and the CMake code would have
+```
+add_executable(app)
+
+# These lines might work, but depend on how exactly the user intends to use
+# Boost. They are not specific to HighFive, but previously added automatically
+# (and sometimes correctly) by HighFive.
+find_package(Boost)
+target_link_libraries(add PUBLIC boost::boost)
+
+# For HighFive there's two options for adding `-I ${HIGHFIVE_DIR}` and the
+# flags for HDF5.
+#
+# Option 1: HighFive is install (systemwide) as a regular library:
+find_package(HighFive)
+target_link_libraries(app PUBLIC HighFive::HighFive)
+
+# Option 2: HighFive is vendored as part of the project:
+add_subdirectory(third_party/HighFive)
+target_link_libraries(app PUBLIC HighFive::HighFive)
+```
+
+There are extensive examples of project integration in `tests/cmake_integration`,
+including how those projects in turn can be included in other projects. If these
+examples don't help, please feel free to open an Issue.
+
+## Type change `DataSpace::DataSpaceType`.
+We've converted the `enum` `DataSpace::DataSpaceType` to an `enum class`. We've
+added static `constexpr` members `dataspace_null` and `dataspace_scalar` to
+`DataSpace`. This minimizes the risk of breaking user code.
+
+Note that objects of type `DataSpace::DataSpaceType` will no longer silently
+convert to an integer. Including the two constants
+`DataSpace::dataspace_{scalar,null}`.
+

include/highfive/H5Attribute.hpp

@@ -132,7 +132,7 @@ class Attribute: public Object, public PathTraits<Attribute> {
     /// \endcode
     /// \since 2.2.2
     template <typename T>
-    void read(T* array, const DataType& mem_datatype) const;
+    void read_raw(T* array, const DataType& mem_datatype) const;
 
     /// \brief Read the attribute into a buffer.
     /// Behaves like Attribute::read(T*, const DataType&) const but
@@ -154,7 +154,7 @@ class Attribute: public Object, public PathTraits<Attribute> {
     /// \endcode
     /// \since 2.2.2
     template <typename T>
-    void read(T* array) const;
+    void read_raw(T* array) const;
 
     /// \brief Write the value into the Attribute.
     ///

include/highfive/bits/H5Attribute_misc.hpp

@@ -83,7 +83,7 @@ inline void Attribute::read(T& array) const {
     }
 
     auto r = details::data_converter::get_reader<T>(dims, array, file_datatype);
-    read(r.getPointer(), buffer_info.data_type);
+    read_raw(r.getPointer(), buffer_info.data_type);
     // re-arrange results
     r.unserialize(array);
 
@@ -102,19 +102,19 @@ inline void Attribute::read(T& array) const {
 }
 
 template <typename T>
-inline void Attribute::read(T* array, const DataType& mem_datatype) const {
+inline void Attribute::read_raw(T* array, const DataType& mem_datatype) const {
     static_assert(!std::is_const<T>::value,
                   "read() requires a non-const structure to read data into");
 
     detail::h5a_read(getId(), mem_datatype.getId(), static_cast<void*>(array));
 }
 
 template <typename T>
-inline void Attribute::read(T* array) const {
+inline void Attribute::read_raw(T* array) const {
     using element_type = typename details::inspector<T>::base_type;
     const DataType& mem_datatype = create_and_check_datatype<element_type>();
 
-    read(array, mem_datatype);
+    read_raw(array, mem_datatype);
 }
 
 template <typename T>

include/highfive/bits/H5Slice_traits.hpp

@@ -303,36 +303,6 @@ class SliceTraits {
     template <typename T>
     void read(T& array, const DataTransferProps& xfer_props = DataTransferProps()) const;
 
-    ///
-    /// Read the entire dataset into a raw buffer
-    ///
-    /// \deprecated Use `read_raw` instead.
-    ///
-    /// No dimensionality checks will be performed, it is the user's
-    /// responsibility to ensure that the right amount of space has been
-    /// allocated.
-    /// \param array: A buffer containing enough space for the data
-    /// \param dtype: The datatype of elements of the in memory buffer.
-    /// \param xfer_props: Data Transfer properties
-    template <typename T>
-    void read(T* array,
-              const DataType& dtype,
-              const DataTransferProps& xfer_props = DataTransferProps()) const;
-
-    ///
-    /// Read the entire dataset into a raw buffer
-    ///
-    /// \deprecated Use `read_raw` instead.
-    ///
-    /// Same as `read(T*, const DataType&, const DataTransferProps&)`. However,
-    /// this overload deduces the HDF5 datatype of the element of `array` from
-    /// `T`. Note, that the file datatype is already fixed.
-    ///
-    /// \param array: A buffer containing enough space for the data
-    /// \param xfer_props: Data Transfer properties
-    template <typename T>
-    void read(T* array, const DataTransferProps& xfer_props = DataTransferProps()) const;
-
     ///
     /// Read the entire dataset into a raw buffer
     ///

include/highfive/bits/H5Slice_traits_misc.hpp

@@ -207,20 +207,6 @@ inline void SliceTraits<Derivate>::read(T& array, const DataTransferProps& xfer_
     }
 }
 
-template <typename Derivate>
-template <typename T>
-inline void SliceTraits<Derivate>::read(T* array,
-                                        const DataType& mem_datatype,
-                                        const DataTransferProps& xfer_props) const {
-    read_raw(array, mem_datatype, xfer_props);
-}
-
-template <typename Derivate>
-template <typename T>
-inline void SliceTraits<Derivate>::read(T* array, const DataTransferProps& xfer_props) const {
-    read_raw(array, xfer_props);
-}
-
 
 template <typename Derivate>
 template <typename T>

include/highfive/boost.hpp

@@ -4,8 +4,6 @@
 #include "H5Exception.hpp"
 
 #include <boost/multi_array.hpp>
-// starting Boost 1.64, serialization header must come before ublas
-#include <boost/serialization/vector.hpp>
 #include <boost/numeric/ublas/matrix.hpp>
 
 namespace HighFive {

include/highfive/h5easy_bits/H5Easy_Eigen.hpp

@@ -98,7 +98,7 @@ struct io_impl<T, typename std::enable_if<std::is_base_of<Eigen::DenseBase<T>, T
         DataSet dataset = file.getDataSet(path);
         std::vector<typename T::Index> dims = shape(file, path, dataset, T::RowsAtCompileTime);
         T data(dims[0], dims[1]);
-        dataset.read(data.data());
+        dataset.read_raw(data.data());
         if (data.IsVectorAtCompileTime || data.IsRowMajor) {
             return data;
         }
@@ -130,7 +130,7 @@ struct io_impl<T, typename std::enable_if<std::is_base_of<Eigen::DenseBase<T>, T
         DataSpace dataspace = attribute.getSpace();
         std::vector<typename T::Index> dims = shape(file, path, dataspace, T::RowsAtCompileTime);
         T data(dims[0], dims[1]);
-        attribute.read(data.data());
+        attribute.read_raw(data.data());
         if (data.IsVectorAtCompileTime || data.IsRowMajor) {
             return data;
         }

include/highfive/h5easy_bits/H5Easy_opencv.hpp

@@ -61,7 +61,7 @@ struct io_impl<T, typename std::enable_if<is_opencv<T>::value>::type> {
         DataSet dataset = file.getDataSet(path);
         std::vector<int> dims = shape(file, path, dataset.getDimensions());
         T data(dims[0], dims[1]);
-        dataset.read(reinterpret_cast<value_type*>(data.data));
+        dataset.read_raw(reinterpret_cast<value_type*>(data.data));
         return data;
     }
 
@@ -89,7 +89,7 @@ struct io_impl<T, typename std::enable_if<is_opencv<T>::value>::type> {
         DataSpace dataspace = attribute.getSpace();
         std::vector<int> dims = shape(file, path, dataspace.getDimensions());
         T data(dims[0], dims[1]);
-        attribute.read(reinterpret_cast<value_type*>(data.data));
+        attribute.read_raw(reinterpret_cast<value_type*>(data.data));
         return data;
     }
 };

include/highfive/h5easy_bits/H5Easy_xtensor.hpp

@@ -44,7 +44,7 @@ struct io_impl<T, typename std::enable_if<xt::is_xexpression<T>::value>::type> {
         DataSet dataset = file.getDataSet(path);
         std::vector<size_t> dims = dataset.getDimensions();
         T data = T::from_shape(dims);
-        dataset.read(data.data());
+        dataset.read_raw(data.data());
         return data;
     }
 
@@ -73,7 +73,7 @@ struct io_impl<T, typename std::enable_if<xt::is_xexpression<T>::value>::type> {
         DataSpace dataspace = attribute.getSpace();
         std::vector<size_t> dims = dataspace.getDimensions();
         T data = T::from_shape(dims);
-        attribute.read(data.data());
+        attribute.read_raw(data.data());
         return data;
     }
 };

tests/unit/tests_high_five_base.cpp

@@ -2122,7 +2122,7 @@ TEST_CASE("DirectWriteBool") {
     SECTION("WriteReadCycleAttribute") {
         auto attr = file.createAttribute("attr", dataspace, datatype);
         attr.write_raw(expected);
-        attr.read(actual);
+        attr.read_raw(actual);
 
         for (size_t i = 0; i < n; ++i) {
             REQUIRE(expected[i] == actual[i]);
@@ -2132,7 +2132,7 @@ TEST_CASE("DirectWriteBool") {
     SECTION("WriteReadCycleDataSet") {
         auto dset = file.createAttribute("dset", dataspace, datatype);
         dset.write_raw(expected);
-        dset.read(actual);
+        dset.read_raw(actual);
 
         for (size_t i = 0; i < n; ++i) {
             REQUIRE(expected[i] == actual[i]);
@@ -2442,7 +2442,7 @@ TEST_CASE("HighFiveFixedString") {
 #if HIGHFIVE_CXX_STD >= 17
         {
             auto expected = std::string(value.size(), '-');
-            ds.read(expected.data(), datatype);
+            ds.read_raw(expected.data(), datatype);
 
             REQUIRE(expected == value);
         }