diff --git a/doc/Doxyfile b/doc/Doxyfile index 6ebc393ec..d0cf7efb1 100644 --- a/doc/Doxyfile +++ b/doc/Doxyfile @@ -866,6 +866,7 @@ WARN_LOGFILE = INPUT = @CMAKE_CURRENT_SOURCE_DIR@/../include \ @CMAKE_CURRENT_SOURCE_DIR@/installation.md \ + @CMAKE_CURRENT_SOURCE_DIR@/migration_guide.md \ @CMAKE_CURRENT_SOURCE_DIR@/developer_guide.md \ @CMAKE_CURRENT_SOURCE_DIR@/../CHANGELOG.md \ @CMAKE_CURRENT_SOURCE_DIR@/../README.md diff --git a/doc/migration_guide.md b/doc/migration_guide.md new file mode 100644 index 000000000..e85002b15 --- /dev/null +++ b/doc/migration_guide.md @@ -0,0 +1,16 @@ +# Migration Guide +A collection of tips for migrating away from deprecated features. + +## Deprecation of `FixedLenStringArray`. +The issue with `FixedLenStringArray` is that it is unable to avoid copies. +Essentially, this class acts as a means to create a copy of the data in a +format suitable for writing fixed-length strings. Additionally, the class acts +as a tag for HighFive to overload on. The support of `std::string` in HighFive +has improved considerable. Since 2.8.0 we can write/read `std::string` to fixed +or variable length HDF5 strings. + +Therefore, this class serves no purpose anymore. Any occurrence of it can be +replaced with an `std::vector` (for example). + +If desired one can silence warnings by replacing `FixedLenStringArray` with +`deprecated::FixedLenStringArray`. diff --git a/include/highfive/H5DataType.hpp b/include/highfive/H5DataType.hpp index 0d596965f..b15f62165 100644 --- a/include/highfive/H5DataType.hpp +++ b/include/highfive/H5DataType.hpp @@ -342,6 +342,7 @@ template DataType create_and_check_datatype(); +namespace deprecated { /// /// \brief A structure representing a set of fixed-length strings /// @@ -460,6 +461,11 @@ class FixedLenStringArray { private: vector_t datavec; }; +} // namespace deprecated + +template +using FixedLenStringArray H5_DEPRECATED_USING("Use 'std::vector'.") = + deprecated::FixedLenStringArray; } // namespace HighFive diff --git a/include/highfive/bits/H5DataType_misc.hpp b/include/highfive/bits/H5DataType_misc.hpp index e29c99b0e..619e51e71 100644 --- a/include/highfive/bits/H5DataType_misc.hpp +++ b/include/highfive/bits/H5DataType_misc.hpp @@ -207,7 +207,7 @@ class AtomicType: public DataType { }; template -class AtomicType>: public DataType { +class AtomicType>: public DataType { public: inline AtomicType() : DataType(create_string(StrLen)) {} @@ -239,8 +239,7 @@ AtomicType::AtomicType() { } -// class FixedLenStringArray - +namespace deprecated { template inline FixedLenStringArray::FixedLenStringArray(const char array[][N], std::size_t length) { datavec.resize(length); @@ -283,6 +282,7 @@ template inline std::string FixedLenStringArray::getString(std::size_t i) const { return std::string(datavec[i].data()); } +} // namespace deprecated // Internal // Reference mapping diff --git a/include/highfive/bits/H5Inspector_misc.hpp b/include/highfive/bits/H5Inspector_misc.hpp index 8cbb11c37..7ae90d84f 100644 --- a/include/highfive/bits/H5Inspector_misc.hpp +++ b/include/highfive/bits/H5Inspector_misc.hpp @@ -289,10 +289,10 @@ struct inspector: type_helper { }; template -struct inspector> { - using type = FixedLenStringArray; +struct inspector> { + using type = deprecated::FixedLenStringArray; using value_type = char*; - using base_type = FixedLenStringArray; + using base_type = deprecated::FixedLenStringArray; using hdf5_type = char; static constexpr size_t ndim = 1; diff --git a/include/highfive/bits/H5Node_traits.hpp b/include/highfive/bits/H5Node_traits.hpp index 05e923a3b..6f4a93ce6 100644 --- a/include/highfive/bits/H5Node_traits.hpp +++ b/include/highfive/bits/H5Node_traits.hpp @@ -79,8 +79,9 @@ class NodeTraits { template + H5_DEPRECATED("Use 'std::vector'.") DataSet createDataSet(const std::string& dataset_name, - const FixedLenStringArray& data, + const deprecated::FixedLenStringArray& data, const DataSetCreateProps& createProps = DataSetCreateProps::Default(), const DataSetAccessProps& accessProps = DataSetAccessProps::Default(), bool parents = true); diff --git a/include/highfive/bits/H5Node_traits_misc.hpp b/include/highfive/bits/H5Node_traits_misc.hpp index 2db2b2aab..a98600598 100644 --- a/include/highfive/bits/H5Node_traits_misc.hpp +++ b/include/highfive/bits/H5Node_traits_misc.hpp @@ -83,7 +83,7 @@ inline DataSet NodeTraits::createDataSet(const std::string& dataset_na template template inline DataSet NodeTraits::createDataSet(const std::string& dataset_name, - const FixedLenStringArray& data, + const deprecated::FixedLenStringArray& data, const DataSetCreateProps& createProps, const DataSetAccessProps& accessProps, bool parents) { diff --git a/include/highfive/bits/H5Utils.hpp b/include/highfive/bits/H5Utils.hpp index 2d9d24f88..b3f039e20 100644 --- a/include/highfive/bits/H5Utils.hpp +++ b/include/highfive/bits/H5Utils.hpp @@ -25,9 +25,11 @@ namespace HighFive { +namespace deprecated { // If ever used, recognize dimensions of FixedLenStringArray template class FixedLenStringArray; +} // namespace deprecated namespace details { // converter function for hsize_t -> size_t when hsize_t != size_t diff --git a/include/highfive/bits/H5_definitions.hpp b/include/highfive/bits/H5_definitions.hpp index 746723c88..ad4b95af2 100644 --- a/include/highfive/bits/H5_definitions.hpp +++ b/include/highfive/bits/H5_definitions.hpp @@ -5,10 +5,17 @@ #elif defined(_MSC_VER) #define H5_DEPRECATED(msg) __declspec(deprecated(#msg)) #else -#pragma message("WARNING: Compiler doesnt support deprecation") +#pragma message("WARNING: Compiler doesn't support deprecation") #define H5_DEPRECATED(msg) #endif +#if defined(__GNUC__) || defined(__clang__) +#define H5_DEPRECATED_USING(msg) H5_DEPRECATED((msg)) +#else +#pragma message("WARNING: Compiler doesn't support deprecating using statements.") +#define H5_DEPRECATED_USING(msg) +#endif + // Forward declarations @@ -38,8 +45,10 @@ class AtomicType; template class AnnotateTraits; +namespace deprecated { template class FixedLenStringArray; +} template class NodeTraits; diff --git a/src/examples/read_write_fixedlen_string.cpp b/src/examples/read_write_fixedlen_string.cpp deleted file mode 100644 index 60589637e..000000000 --- a/src/examples/read_write_fixedlen_string.cpp +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright (c), 2020, Blue Brain Project - * - * Distributed under the Boost Software License, Version 1.0. - * (See accompanying file LICENSE_1_0.txt or copy at - * http://www.boost.org/LICENSE_1_0.txt) - * - */ -#include -#include - -#include - -using namespace HighFive; - -// This examples shows how compile time constant strings work. -// -// Note, that as of version 2.8.0., writing `std::string` as fixed-length -// strings there's a simpler API. -int main() { - // Create a new file using the default property lists. - File file("create_dataset_string_example.h5", File::Truncate); - const char strings_fixed[][16] = {"abcabcabcabcabc", "123123123123123"}; - - // create a dataset ready to contains strings of the size of the vector - file.createDataSet("ds1", DataSpace(2)).write(strings_fixed); - - // Without specific type info this will create an int8 dataset - file.createDataSet("ds2", strings_fixed); - - // Now test the new interface type - FixedLenStringArray<10> arr{"0000000", "1111111"}; - auto ds = file.createDataSet("ds3", arr); - - // Read back truncating to 4 chars - FixedLenStringArray<4> array_back; - ds.read(array_back); - std::cout << "First item is '" << array_back[0] << "'\n" - << "Second item is '" << array_back[1] << "'\n"; - - return 0; -} diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index b8943067f..2f01bdd81 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -52,3 +52,5 @@ if(HIGHFIVE_TEST_SINGLE_INCLUDES) target_link_libraries("tests_include_${CLASS_NAME}" HighFive HighFiveWarnings) endforeach() endif() + +add_subdirectory(deprecated) diff --git a/tests/unit/deprecated/CMakeLists.txt b/tests/unit/deprecated/CMakeLists.txt new file mode 100644 index 000000000..5e515374b --- /dev/null +++ b/tests/unit/deprecated/CMakeLists.txt @@ -0,0 +1,10 @@ +foreach(test_name test_fixed_len_string_array) + add_executable(${test_name} "${test_name}.cpp") + + target_link_libraries(${test_name} HighFive HighFiveWarnings Catch2::Catch2WithMain) + catch_discover_tests(${test_name}) + + if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU") + target_compile_options(${test_name} PRIVATE -Wno-deprecated-declarations) + endif() +endforeach() diff --git a/tests/unit/deprecated/test_fixed_len_string_array.cpp b/tests/unit/deprecated/test_fixed_len_string_array.cpp new file mode 100644 index 000000000..1d0c33aaa --- /dev/null +++ b/tests/unit/deprecated/test_fixed_len_string_array.cpp @@ -0,0 +1,172 @@ +#include + +#include +#include "../tests_high_five.hpp" + +namespace HighFive { + +TEST_CASE("HighFiveFixedLenStringArray") { + const std::string file_name("fixed_len_string_array.h5"); + + // Create a new file using the default property lists. + File file(file_name, File::ReadWrite | File::Create | File::Truncate); + + { // Dedicated FixedLenStringArray (now deprecated). + FixedLenStringArray<10> arr{"0000000", "1111111"}; + + // More API: test inserting something + arr.push_back("2222"); + auto ds = file.createDataSet("ds7", arr); // Short syntax ok + + // Recover truncating + FixedLenStringArray<4> array_back; + ds.read(array_back); + CHECK(array_back.size() == 3); + CHECK(array_back[0] == std::string("000")); + CHECK(array_back[1] == std::string("111")); + CHECK(array_back[2] == std::string("222")); + CHECK(array_back.getString(1) == "111"); + CHECK(array_back.front() == std::string("000")); + CHECK(array_back.back() == std::string("222")); + CHECK(array_back.data() == std::string("000")); + array_back.data()[0] = 'x'; + CHECK(array_back.data() == std::string("x00")); + + for (auto& raw_elem: array_back) { + raw_elem[1] = 'y'; + } + CHECK(array_back.getString(1) == "1y1"); + for (auto iter = array_back.cbegin(); iter != array_back.cend(); ++iter) { + CHECK((*iter)[1] == 'y'); + } + } +} + +template +static void check_fixed_len_string_array_contents(const FixedLenStringArray& array, + const std::vector& expected) { + REQUIRE(array.size() == expected.size()); + + for (size_t i = 0; i < array.size(); ++i) { + CHECK(array[i] == expected[i]); + } +} + + +TEST_CASE("HighFiveFixedLenStringArrayStructure") { + using fixed_array_t = FixedLenStringArray<10>; + // increment the characters of a string written in a std::array + auto increment_string = [](const fixed_array_t::value_type arr) { + fixed_array_t::value_type output(arr); + for (auto& c: output) { + if (c == 0) { + break; + } + ++c; + } + return output; + }; + + SECTION("create from std::vector (onpoint)") { + auto expected = std::vector{"000", "111"}; + auto actual = FixedLenStringArray<4>(expected); + check_fixed_len_string_array_contents(actual, expected); + } + + SECTION("create from std::vector (oversized)") { + auto expected = std::vector{"000", "111"}; + auto actual = FixedLenStringArray<8>(expected); + check_fixed_len_string_array_contents(actual, expected); + } + + SECTION("create from pointers (onpoint)") { + auto expected = std::vector{"000", "111"}; + auto actual = FixedLenStringArray<4>(expected.data(), expected.data() + expected.size()); + check_fixed_len_string_array_contents(actual, expected); + } + + SECTION("create from pointers (oversized)") { + auto expected = std::vector{"000", "111"}; + auto actual = FixedLenStringArray<8>(expected.data(), expected.data() + expected.size()); + check_fixed_len_string_array_contents(actual, expected); + } + + + SECTION("create from std::initializer_list (onpoint)") { + auto expected = std::vector{"000", "111"}; + auto actual = FixedLenStringArray<4>{"000", "111"}; + check_fixed_len_string_array_contents(actual, expected); + } + + SECTION("create from std::initializer_list (oversized)") { + auto expected = std::vector{"000", "111"}; + auto actual = FixedLenStringArray<8>{"000", "111"}; + check_fixed_len_string_array_contents(actual, expected); + } + + // manipulate FixedLenStringArray with std::copy + SECTION("compatible with std::copy") { + const fixed_array_t arr1{"0000000", "1111111"}; + fixed_array_t arr2{"0000000", "1111111"}; + std::copy(arr1.begin(), arr1.end(), std::back_inserter(arr2)); + CHECK(arr2.size() == 4); + } + + SECTION("compatible with std::transform") { + fixed_array_t arr; + { + const fixed_array_t arr1{"0000000", "1111111"}; + std::transform(arr1.begin(), arr1.end(), std::back_inserter(arr), increment_string); + } + CHECK(arr.size() == 2); + CHECK(arr[0] == std::string("1111111")); + CHECK(arr[1] == std::string("2222222")); + } + + SECTION("compatible with std::transform (reverse iterator)") { + fixed_array_t arr; + { + const fixed_array_t arr1{"0000000", "1111111"}; + std::copy(arr1.rbegin(), arr1.rend(), std::back_inserter(arr)); + } + CHECK(arr.size() == 2); + CHECK(arr[0] == std::string("1111111")); + CHECK(arr[1] == std::string("0000000")); + } + + SECTION("compatible with std::remove_copy_if") { + fixed_array_t arr2; + { + const fixed_array_t arr1{"0000000", "1111111"}; + std::remove_copy_if(arr1.begin(), + arr1.end(), + std::back_inserter(arr2), + [](const fixed_array_t::value_type& s) { + return std::strncmp(s.data(), "1111111", 7) == 0; + }); + } + CHECK(arr2.size() == 1); + CHECK(arr2[0] == std::string("0000000")); + } +} + +TEST_CASE("HighFiveFixedLenStringArrayAttribute") { + const std::string file_name("fixed_array_attr.h5"); + // Create a new file using the default property lists. + { + File file(file_name, File::ReadWrite | File::Create | File::Truncate); + FixedLenStringArray<10> arr{"Hello", "world"}; + file.createAttribute("str", arr); + } + // Re-read it + { + File file(file_name); + FixedLenStringArray<8> arr; // notice the output strings can be smaller + file.getAttribute("str").read(arr); + CHECK(arr.size() == 2); + CHECK(arr[0] == std::string("Hello")); + CHECK(arr[1] == std::string("world")); + } +} + +} // namespace HighFive diff --git a/tests/unit/tests_high_five_base.cpp b/tests/unit/tests_high_five_base.cpp index 163535b55..fefdcdd55 100644 --- a/tests/unit/tests_high_five_base.cpp +++ b/tests/unit/tests_high_five_base.cpp @@ -2405,36 +2405,6 @@ TEST_CASE("HighFiveFixedString") { file.createDataSet("ds6", DataSpace(1)).write(buffer); } - { // Dedicated FixedLenStringArray - FixedLenStringArray<10> arr{"0000000", "1111111"}; - - // More API: test inserting something - arr.push_back("2222"); - auto ds = file.createDataSet("ds7", arr); // Short syntax ok - - // Recover truncating - FixedLenStringArray<4> array_back; - ds.read(array_back); - CHECK(array_back.size() == 3); - CHECK(array_back[0] == std::string("000")); - CHECK(array_back[1] == std::string("111")); - CHECK(array_back[2] == std::string("222")); - CHECK(array_back.getString(1) == "111"); - CHECK(array_back.front() == std::string("000")); - CHECK(array_back.back() == std::string("222")); - CHECK(array_back.data() == std::string("000")); - array_back.data()[0] = 'x'; - CHECK(array_back.data() == std::string("x00")); - - for (auto& raw_elem: array_back) { - raw_elem[1] = 'y'; - } - CHECK(array_back.getString(1) == "1y1"); - for (auto iter = array_back.cbegin(); iter != array_back.cend(); ++iter) { - CHECK((*iter)[1] == 'y'); - } - } - { // Direct way of writing `std::string` as a fixed length // HDF5 string. @@ -2492,132 +2462,6 @@ TEST_CASE("HighFiveFixedString") { } } -template -static void check_fixed_len_string_array_contents(const FixedLenStringArray& array, - const std::vector& expected) { - REQUIRE(array.size() == expected.size()); - - for (size_t i = 0; i < array.size(); ++i) { - CHECK(array[i] == expected[i]); - } -} - -TEST_CASE("HighFiveFixedLenStringArrayStructure") { - using fixed_array_t = FixedLenStringArray<10>; - // increment the characters of a string written in a std::array - auto increment_string = [](const fixed_array_t::value_type arr) { - fixed_array_t::value_type output(arr); - for (auto& c: output) { - if (c == 0) { - break; - } - ++c; - } - return output; - }; - - SECTION("create from std::vector (onpoint)") { - auto expected = std::vector{"000", "111"}; - auto actual = FixedLenStringArray<4>(expected); - check_fixed_len_string_array_contents(actual, expected); - } - - SECTION("create from std::vector (oversized)") { - auto expected = std::vector{"000", "111"}; - auto actual = FixedLenStringArray<8>(expected); - check_fixed_len_string_array_contents(actual, expected); - } - - SECTION("create from pointers (onpoint)") { - auto expected = std::vector{"000", "111"}; - auto actual = FixedLenStringArray<4>(expected.data(), expected.data() + expected.size()); - check_fixed_len_string_array_contents(actual, expected); - } - - SECTION("create from pointers (oversized)") { - auto expected = std::vector{"000", "111"}; - auto actual = FixedLenStringArray<8>(expected.data(), expected.data() + expected.size()); - check_fixed_len_string_array_contents(actual, expected); - } - - - SECTION("create from std::initializer_list (onpoint)") { - auto expected = std::vector{"000", "111"}; - auto actual = FixedLenStringArray<4>{"000", "111"}; - check_fixed_len_string_array_contents(actual, expected); - } - - SECTION("create from std::initializer_list (oversized)") { - auto expected = std::vector{"000", "111"}; - auto actual = FixedLenStringArray<8>{"000", "111"}; - check_fixed_len_string_array_contents(actual, expected); - } - - // manipulate FixedLenStringArray with std::copy - SECTION("compatible with std::copy") { - const fixed_array_t arr1{"0000000", "1111111"}; - fixed_array_t arr2{"0000000", "1111111"}; - std::copy(arr1.begin(), arr1.end(), std::back_inserter(arr2)); - CHECK(arr2.size() == 4); - } - - SECTION("compatible with std::transform") { - fixed_array_t arr; - { - const fixed_array_t arr1{"0000000", "1111111"}; - std::transform(arr1.begin(), arr1.end(), std::back_inserter(arr), increment_string); - } - CHECK(arr.size() == 2); - CHECK(arr[0] == std::string("1111111")); - CHECK(arr[1] == std::string("2222222")); - } - - SECTION("compatible with std::transform (reverse iterator)") { - fixed_array_t arr; - { - const fixed_array_t arr1{"0000000", "1111111"}; - std::copy(arr1.rbegin(), arr1.rend(), std::back_inserter(arr)); - } - CHECK(arr.size() == 2); - CHECK(arr[0] == std::string("1111111")); - CHECK(arr[1] == std::string("0000000")); - } - - SECTION("compatible with std::remove_copy_if") { - fixed_array_t arr2; - { - const fixed_array_t arr1{"0000000", "1111111"}; - std::remove_copy_if(arr1.begin(), - arr1.end(), - std::back_inserter(arr2), - [](const fixed_array_t::value_type& s) { - return std::strncmp(s.data(), "1111111", 7) == 0; - }); - } - CHECK(arr2.size() == 1); - CHECK(arr2[0] == std::string("0000000")); - } -} - -TEST_CASE("HighFiveFixedLenStringArrayAttribute") { - const std::string file_name("fixed_array_attr.h5"); - // Create a new file using the default property lists. - { - File file(file_name, File::ReadWrite | File::Create | File::Truncate); - FixedLenStringArray<10> arr{"Hello", "world"}; - file.createAttribute("str", arr); - } - // Re-read it - { - File file(file_name); - FixedLenStringArray<8> arr; // notice the output strings can be smaller - file.getAttribute("str").read(arr); - CHECK(arr.size() == 2); - CHECK(arr[0] == std::string("Hello")); - CHECK(arr[1] == std::string("world")); - } -} - TEST_CASE("HighFiveReference") { const std::string file_name("h5_ref_test.h5"); const std::string dataset1_name("dset1");