Refactor HDF5 attribute handling and improve error reporting

Author: kojix2

lib/hdf5/attribute.rb

@@ -3,45 +3,51 @@ class Attribute
     def initialize(dataset_id, attr_name)
       @dataset_id = dataset_id
       @attr_name = attr_name
-      @attr_id = FFI.H5Aopen(@dataset_id, @attr_name, 0)
-      raise 'Failed to open attribute' if @attr_id < 0
+      @attr_id = HDF5::FFI.H5Aopen(@dataset_id, @attr_name, HDF5::DEFAULT_PROPERTY_LIST)
+      raise HDF5::Error, 'Failed to open attribute' if @attr_id < 0
     end
 
     def read
-      type_id = FFI.H5Aget_type(@attr_id)
-      space_id = FFI.H5Aget_space(@attr_id)
+      type_id = HDF5::FFI.H5Aget_type(@attr_id)
+      space_id = HDF5::FFI.H5Aget_space(@attr_id)
 
-      size = FFI.H5Sget_simple_extent_npoints(space_id)
+      size = HDF5::FFI.H5Sget_simple_extent_npoints(space_id)
 
       buffer = \
-        case FFI.H5Tget_class(type_id)
+        case HDF5::FFI.H5Tget_class(type_id)
         when :H5T_INTEGER
           ::FFI::MemoryPointer.new(:int, size)
         when :H5T_FLOAT
           ::FFI::MemoryPointer.new(:double, size)
         when :H5T_STRING
           ::FFI::MemoryPointer.new(:pointer, size)
         else
-          raise 'Unsupported data type'
+          raise HDF5::Error, 'Unsupported data type'
         end
 
-      status = FFI.H5Aread(@attr_id, type_id, buffer)
-      raise 'Failed to read attribute' if status < 0
+      status = HDF5::FFI.H5Aread(@attr_id, type_id, buffer)
+      raise HDF5::Error, 'Failed to read attribute' if status < 0
 
-      case FFI.H5Tget_class(type_id)
+      case HDF5::FFI.H5Tget_class(type_id)
       when :H5T_INTEGER
         buffer.read_array_of_int(size)
       when :H5T_FLOAT
         buffer.read_array_of_double(size)
       when :H5T_STRING
         buffer.read_pointer.read_string
       else
-        raise 'Unsupported data type'
+        raise HDF5::Error, 'Unsupported data type'
       end
+    ensure
+      HDF5::FFI.H5Tclose(type_id) if type_id && type_id >= 0
+      HDF5::FFI.H5Sclose(space_id) if space_id && space_id >= 0
     end
 
     def close
-      FFI.H5Aclose(@attr_id)
+      return if @attr_id.nil?
+
+      HDF5::FFI.H5Aclose(@attr_id)
+      @attr_id = nil
     end
   end
 

lib/hdf5/group.rb

@@ -66,7 +66,7 @@ def list_datasets
        else
          HDF5::FFI.H5Literate2(@group_id, :H5_INDEX_NAME, :H5_ITER_NATIVE, nil, callback, nil)
        end).negative? &&
-        raise('Failed to list datasets')
+        raise(HDF5::Error, 'Failed to list datasets')
 
       datasets
     end

test/hdf5_test.rb

@@ -90,6 +90,56 @@ class HDF5Test < Test::Unit::TestCase
     end
   end
 
+  test 'dataset attrs reads integer attribute' do
+    Dir.mktmpdir do |dir|
+      path = File.join(dir, 'attribute.h5')
+
+      HDF5::File.create(path) do |file|
+        dataset = file.create_dataset('values', [1, 2, 3])
+        dataset_id = dataset.instance_variable_get(:@dataset_id)
+
+        dims = FFI::MemoryPointer.new(:ulong_long, 1)
+        dims.write_array_of_ulong_long([1])
+        attr_space_id = HDF5::FFI.H5Screate_simple(1, dims, nil)
+        attr_id = HDF5::FFI.H5Acreate2(
+          dataset_id,
+          'scale',
+          HDF5::FFI.H5T_NATIVE_INT,
+          attr_space_id,
+          HDF5::DEFAULT_PROPERTY_LIST,
+          HDF5::DEFAULT_PROPERTY_LIST
+        )
+        value = FFI::MemoryPointer.new(:int, 1)
+        value.write_int(42)
+        status = HDF5::FFI.H5Awrite(attr_id, HDF5::FFI.H5T_NATIVE_INT, value)
+        assert_equal(0, status)
+      ensure
+        HDF5::FFI.H5Aclose(attr_id) if attr_id && attr_id >= 0
+        HDF5::FFI.H5Sclose(attr_space_id) if attr_space_id && attr_space_id >= 0
+        dataset.close if dataset
+      end
+
+      HDF5::File.open(path) do |file|
+        assert_equal([42], file['values'].attrs['scale'])
+      end
+    end
+  end
+
+  test 'dataset attrs raises HDF5 error for missing attribute' do
+    Dir.mktmpdir do |dir|
+      path = File.join(dir, 'missing-attribute.h5')
+
+      HDF5::File.create(path) do |file|
+        dataset = file.create_dataset('values', [1, 2, 3])
+        assert_raise(HDF5::Error) do
+          dataset.attrs['does-not-exist']
+        end
+      ensure
+        dataset.close if dataset
+      end
+    end
+  end
+
   test 'raises HDF5 error for missing file' do
     assert_raise(HDF5::Error) do
       HDF5::File.open('/tmp/does-not-exist-ruby-hdf5.h5')