Issue 666: fix for writing utf8 strings (#669)

jkotan · web-flow · commit 3afccf2894e3 · 2025-04-30T08:02:58.000+02:00
* add utf8 attribute tests and relax blosc tests

* update actions

* update ubuntu runner

* add uft8 dataset tests

* add fix for writing utf8 strings

* clean the patch
diff --git 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') }}
@@ -258,11 +258,11 @@ jobs:
           mpi,
           serial
         ]
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     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 \
diff --git 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<typename T>
 void Attribute::write(const T &data) const
 {
-  hdf5::datatype::DatatypeHolder mem_type_holder;
-
-  write(data,mem_type_holder.get<T>());
+  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<T>());
+  }
 }
 
 template<typename T>
diff --git a/src/h5cpp/node/dataset.hpp b/src/h5cpp/node/dataset.hpp
@@ -987,17 +987,31 @@ std::uint32_t Dataset::read_chunk(T &data,
 template<typename T>
 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<typename T>
 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)
+    {
+      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<typename T>
diff --git 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<std::string>();
+  auto utf8_type = datatype::create<std::string>();
+  utf8_type.encoding(datatype::CharacterEncoding::UTF8);
 
+  GIVEN("a utf8 scalar attribute") {
+    auto space = dataspace::Scalar();
+    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") {
+        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
@@ -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<int>(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");
           }
         }
       }
diff --git 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<std::string>();
   hdf5::dataspace::Scalar scalar_space;
   hdf5::dataspace::Simple simple_space({7});
+  auto utf8_type = datatype::create<std::string>();
+  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") {

Original file line number	Diff line number	Diff line change
`@@ -184,8 +184,9 @@ SCENARIO("External filter Blosc LZ4") {`
`184`	`184`	`REQUIRE(flags[0] == filter::Availability::Mandatory);`
`185`	`185`	`REQUIRE_THAT(filters[0].cd_values(), Equals(params));`
`186`	`186`	`REQUIRE(filters[0].id() == static_cast<int>(FILTER_BLOSC));`
`187`		`- REQUIRE(filters[0].name() ==`
`188`		`- "HDF5 blosc filter; see http://www.hdfgroup.org/services/contributions.html");`
	`187`	`+ if (filters[0].name() != "blosc")`
	`188`	`+ REQUIRE(filters[0].name() ==`
	`189`	`+ "HDF5 blosc filter; see http://www.hdfgroup.org/services/contributions.html");`
`189`	`190`	`}`
`190`	`191`	`}`
`191`	`192`	`}`