From 0bb3166bbe71ffc4b121fe27274326b1e31381e8 Mon Sep 17 00:00:00 2001 From: Jan Kotanski Date: Tue, 29 Apr 2025 09:16:46 +0200 Subject: [PATCH 1/6] add utf8 attribute tests and relax blosc tests --- .../attribute_variable_string_io.cpp | 30 +++++++++++++++++++ test/filter/external_filter_test.cpp | 5 ++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/test/attribute/attribute_variable_string_io.cpp b/test/attribute/attribute_variable_string_io.cpp index 312489bfe2..eaab06373e 100644 --- a/test/attribute/attribute_variable_string_io.cpp +++ b/test/attribute/attribute_variable_string_io.cpp @@ -45,7 +45,37 @@ SCENARIO("variable string attribute IO") { auto simple_space = dataspace::Simple{{6}}; auto scalar_space = dataspace::Scalar(); auto string_type = datatype::create(); + auto utf8_type = datatype::create(); + utf8_type.encoding(datatype::CharacterEncoding::UTF8); + GIVEN("a utf8 scalar attribute") { + auto space = dataspace::Scalar(); + auto attr = root_group.attributes.create("scalar", utf8_type, space); + AND_GIVEN("a string of arbitrary length") { + std::string write = "hello"; + THEN("we can write the string to the attribute") { + REQUIRE_NOTHROW(attr.write(write)); + std::string read; + AND_THEN("read the attribute using the default datatype") { + REQUIRE_NOTHROW(attr.read(read)); + REQUIRE(write == read); + } + AND_THEN("read the attribute using the attributes datatype") { + REQUIRE_NOTHROW(attr.read(read, attr.datatype())); + REQUIRE(write == read); + } + } + } + THEN("we can write a const char string to the attribute") { + REQUIRE_NOTHROW(attr.write("A short notice")); + AND_THEN("read it again") { + std::string expect = "A short notice"; + std::string read; + REQUIRE_NOTHROW(attr.read(read)); + REQUIRE_THAT(expect, Catch::Matchers::Equals(read)); + } + } + } GIVEN("a scalar attribute") { auto space = dataspace::Scalar(); auto attr = root_group.attributes.create("scalar", string_type, space); diff --git a/test/filter/external_filter_test.cpp b/test/filter/external_filter_test.cpp index e4b995e9ff..9d799b52b7 100644 --- a/test/filter/external_filter_test.cpp +++ b/test/filter/external_filter_test.cpp @@ -184,8 +184,9 @@ SCENARIO("External filter Blosc LZ4") { REQUIRE(flags[0] == filter::Availability::Mandatory); REQUIRE_THAT(filters[0].cd_values(), Equals(params)); REQUIRE(filters[0].id() == static_cast(FILTER_BLOSC)); - REQUIRE(filters[0].name() == - "HDF5 blosc filter; see http://www.hdfgroup.org/services/contributions.html"); + if (filters[0].name() != "blosc") + REQUIRE(filters[0].name() == + "HDF5 blosc filter; see http://www.hdfgroup.org/services/contributions.html"); } } } From ec608a20d3c5360c5ac4b511c74d6c8e878fc0a8 Mon Sep 17 00:00:00 2001 From: Jan Kotanski Date: Tue, 29 Apr 2025 09:20:13 +0200 Subject: [PATCH 2/6] update actions --- .github/workflows/cmake-build.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/cmake-build.yml b/.github/workflows/cmake-build.yml index cb0df69cd0..e5d1107ed4 100644 --- a/.github/workflows/cmake-build.yml +++ b/.github/workflows/cmake-build.yml @@ -32,7 +32,7 @@ jobs: # image: ${{ matrix.image }} # options: '--user=root' steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set environment variables run: | bash .github/workflows/set_env_vars.sh \ @@ -58,7 +58,7 @@ jobs: conan lock create conanfile.py ${CONAN_ARGS} --lockfile-packages --lockfile-out base.lock conan lock create conanfile.py ${CONAN_ARGS} --build missing - name: cache conan dependencies - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.conan/data key: conan-${{ matrix.profile }}-${{ hashfiles('base.lock') }}-${{ hashFiles('conan.lock') }} @@ -105,7 +105,7 @@ jobs: ] runs-on: windows-2022 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Add MSVC to PATH uses: ilammy/msvc-dev-cmd@v1 - name: Set environment variables @@ -126,7 +126,7 @@ jobs: conan lock create conanfile.py ${CONAN_ARGS} --lockfile-packages --lockfile-out base.lock conan lock create conanfile.py ${CONAN_ARGS} --build missing - name: cache conan dependencies - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.conan/data key: conan-vs2022-${{ hashfiles('base.lock') }}-${{ hashFiles('conan.lock') }} @@ -173,8 +173,8 @@ jobs: ] runs-on: macos-13 steps: - - uses: actions/checkout@v2 - - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v4 with: python-version: '3.x' - name: Set environment variables @@ -206,7 +206,7 @@ jobs: conan lock create conanfile.py ${CONAN_ARGS} --lockfile-packages --lockfile-out base.lock conan lock create conanfile.py ${CONAN_ARGS} --build missing - name: cache conan dependencies - uses: actions/cache@v2 + uses: actions/cache@v4 with: path: ~/.conan/data key: conan-apple-clang12-${{ hashfiles('base.lock') }}-${{ hashFiles('conan.lock') }} @@ -262,7 +262,7 @@ jobs: container: image: debian:bookworm steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Set environment variables run: | bash .github/workflows/set_env_vars.sh \ From 5ac52481503b8a5f1ab548d6de4e29ec44dcf6bb Mon Sep 17 00:00:00 2001 From: Jan Kotanski Date: Tue, 29 Apr 2025 09:22:13 +0200 Subject: [PATCH 3/6] update ubuntu runner --- .github/workflows/cmake-build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cmake-build.yml b/.github/workflows/cmake-build.yml index e5d1107ed4..c3499ec3d8 100644 --- a/.github/workflows/cmake-build.yml +++ b/.github/workflows/cmake-build.yml @@ -258,7 +258,7 @@ jobs: mpi, serial ] - runs-on: ubuntu-20.04 + runs-on: ubuntu-24.04 container: image: debian:bookworm steps: From 9729a83e376a16001d8b29ae50aca1b86509e8e1 Mon Sep 17 00:00:00 2001 From: Jan Kotanski Date: Tue, 29 Apr 2025 11:30:41 +0200 Subject: [PATCH 4/6] add uft8 dataset tests --- .../attribute_variable_string_io.cpp | 2 +- test/node/dataset_variable_string_io.cpp | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/test/attribute/attribute_variable_string_io.cpp b/test/attribute/attribute_variable_string_io.cpp index eaab06373e..d5a21b3c16 100644 --- a/test/attribute/attribute_variable_string_io.cpp +++ b/test/attribute/attribute_variable_string_io.cpp @@ -50,7 +50,7 @@ SCENARIO("variable string attribute IO") { GIVEN("a utf8 scalar attribute") { auto space = dataspace::Scalar(); - auto attr = root_group.attributes.create("scalar", utf8_type, space); + auto attr = root_group.attributes.create("utf8_scalar", utf8_type, space); AND_GIVEN("a string of arbitrary length") { std::string write = "hello"; THEN("we can write the string to the attribute") { diff --git a/test/node/dataset_variable_string_io.cpp b/test/node/dataset_variable_string_io.cpp index 80dd33181c..1c2383203f 100644 --- a/test/node/dataset_variable_string_io.cpp +++ b/test/node/dataset_variable_string_io.cpp @@ -39,8 +39,31 @@ SCENARIO("testing variable length string IO") { auto string_type = hdf5::datatype::create(); hdf5::dataspace::Scalar scalar_space; hdf5::dataspace::Simple simple_space({7}); + auto utf8_type = datatype::create(); + utf8_type.encoding(datatype::CharacterEncoding::UTF8); hdf5::property::DatasetTransferList dtpl; + GIVEN("a scalar dataset") { + node::Dataset dataset(f.root(), "utf_scalar", utf8_type, scalar_space); + THEN("we can write a single string value to it") { + std::string value = "hello"; + dataset.write(value); + AND_THEN("read it back") { + std::string readback; + dataset.read(readback); + REQUIRE(readback == value); + } + } + THEN("we can write a string from a char pointer") { + dataset.write("this is a test"); + AND_THEN("read this back") { + std::string readback; + dataset.read(readback); + REQUIRE(readback == "this is a test"); + } + } + } + GIVEN("a scalar dataset") { node::Dataset dataset(f.root(), "scalar", string_type, scalar_space); THEN("we can write a single string value to it") { From ab01a291f5499e1e1070e82db1111014d3159fcd Mon Sep 17 00:00:00 2001 From: Jan Kotanski Date: Tue, 29 Apr 2025 14:07:27 +0200 Subject: [PATCH 5/6] add fix for writing utf8 strings --- src/h5cpp/attribute/attribute.hpp | 13 ++++++++++--- src/h5cpp/node/dataset.hpp | 23 +++++++++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/h5cpp/attribute/attribute.hpp b/src/h5cpp/attribute/attribute.hpp index ab47bf8bcc..1b5a5e92b3 100644 --- a/src/h5cpp/attribute/attribute.hpp +++ b/src/h5cpp/attribute/attribute.hpp @@ -350,9 +350,16 @@ void Attribute::write(const T &data,const datatype::Datatype &mem_type) const template void Attribute::write(const T &data) const { - hdf5::datatype::DatatypeHolder mem_type_holder; - - write(data,mem_type_holder.get()); + auto file_type = datatype(); + if(file_type.get_class() == datatype::Class::String) + { + write(data,file_type); + } + else + { + hdf5::datatype::DatatypeHolder mem_type_holder; + write(data,mem_type_holder.get()); + } } template diff --git a/src/h5cpp/node/dataset.hpp b/src/h5cpp/node/dataset.hpp index e691f42326..0403d2c1bf 100644 --- a/src/h5cpp/node/dataset.hpp +++ b/src/h5cpp/node/dataset.hpp @@ -987,17 +987,32 @@ std::uint32_t Dataset::read_chunk(T &data, template void Dataset::write(const T &data,const property::DatasetTransferList &dtpl) { - hdf5::datatype::DatatypeHolder mem_type_holder; hdf5::dataspace::DataspaceHolder mem_space_holder(space_pool); - write_reshape(data, mem_type_holder.get(data), mem_space_holder.get(data), dtpl); + if(file_type_.get_class() == datatype::Class::String) + { + write_reshape(data, file_type_, mem_space_holder.get(data), dtpl); + } + else + { + hdf5::datatype::DatatypeHolder mem_type_holder; + write_reshape(data, mem_type_holder.get(data), mem_space_holder.get(data), dtpl); + } } template void Dataset::write(const T &data,const property::DatasetTransferList &dtpl) const { - hdf5::datatype::DatatypeHolder mem_type_holder; hdf5::dataspace::DataspaceHolder mem_space_holder; - write_reshape(data, mem_type_holder.get(data), mem_space_holder.get(data), dtpl); + if(file_type_.get_class() == datatype::Class::String) + { + hdf5::datatype::DatatypeHolder mem_type_holder; + write_reshape(data, file_type_, mem_space_holder.get(data), dtpl); + } + else + { + hdf5::datatype::DatatypeHolder mem_type_holder; + write_reshape(data, mem_type_holder.get(data), mem_space_holder.get(data), dtpl); + } } template From e498e3260c5d8be8abba6db5beadee56bb3d34d0 Mon Sep 17 00:00:00 2001 From: Jan Kotanski Date: Tue, 29 Apr 2025 20:18:30 +0200 Subject: [PATCH 6/6] clean the patch --- src/h5cpp/node/dataset.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/h5cpp/node/dataset.hpp b/src/h5cpp/node/dataset.hpp index 0403d2c1bf..fcf8eb9307 100644 --- a/src/h5cpp/node/dataset.hpp +++ b/src/h5cpp/node/dataset.hpp @@ -1005,7 +1005,6 @@ void Dataset::write(const T &data,const property::DatasetTransferList &dtpl) con hdf5::dataspace::DataspaceHolder mem_space_holder; if(file_type_.get_class() == datatype::Class::String) { - hdf5::datatype::DatatypeHolder mem_type_holder; write_reshape(data, file_type_, mem_space_holder.get(data), dtpl); } else