Minor attribute fix

changelog.org

@@ -1,3 +1,10 @@
+* v0.6.3
+- make ~[]~ for attributes taking a string only for the data type kind
+  to check if the key exist, raise if not.
+  Previously if the attribute hadn't been opened before, the call
+  would fail even if it exists in the file
+- make ~withAttr~ check if the attribute exists in the file
+  first. Previously would fail if it hadn't been opened before
 * v0.6.2
 - fix a small memory leak related to ~cstrings~ not being freed
 - fix the ~-d:DEBUG_HDF5~ option for debug information

src/nimhdf5/attributes.nim

@@ -6,7 +6,7 @@
 
 import typeinfo
 import tables
 import strutils, sequtils
 
 import hdf5_wrapper, h5util, H5nimtypes, datatypes, dataspaces, util
 
@@ -101,28 +101,6 @@
   # add to this attribute object
   h5attr.attr_tab[name] = attr
 
-proc readAttributeInfo(h5attr: H5Attributes, key: string) =
-  ## reads all information about the attribute `key` from the H5 file
-  ## NOTE: this does ``not`` read the value of that attribute!
-  var attr = newH5Attr()
-  attr.attr_id = openAttribute(h5attr, key)
-  attr.opened = true
-  readAttributeInfo(h5attr, attr, key)
-
-proc read_all_attributes*(h5attr: H5Attributes) =
-  ## proc to read all attributes of the parent from file and store the names
-  ## and attribute ids in `h5attr`.
-  ## NOTE: If possible try to avoid using this proc! However, if you must, make
-  ## sure to close all attributes after usage, otherwise memory leaks might happen.
-  # first get how many objects there are
-  h5attr.num_attrs = h5attr.getNumAttrs
-  for i in 0 ..< h5attr.num_attrs:
-    var attr = newH5Attr()
-    attr.attr_id = openAttrByIdx(h5attr, i)
-    attr.opened = true
-    let name = getAttrName(attr.attr_id)
-    readAttributeInfo(h5attr, attr, name)
-
 proc existsAttribute*(parent: ParentID, name: string): bool =
   ## simply check if the given attribute name corresponds to an attribute
   ## of the given object
@@ -147,6 +125,38 @@
   ## field of a group or dataset
   result = attr.parent_id.existsAttribute(key)
 
+proc readAttributeInfo*(h5attr: H5Attributes, key: string, closeAttribute = false) =
+  ## Checks if the given attribute `key` exists in the set of attributes
+  ## and raises if not.
+  ## If it exists, it reads the attribute information, making it available
+  ## in the `attr_tab`.
+  ##
+  ## NOTE: this does ``not`` read the value of that attribute!
+  let attr_exists = key in h5attr
+  if attr_exists:
+    var attr = newH5Attr()
+    attr.attr_id = openAttribute(h5attr, key)
+    attr.opened = true
+    readAttributeInfo(h5attr, attr, key)
+    if closeAttribute: # close again
+      attr.close()
+  else:
+    raise newException(KeyError, "No attribute `$#` exists in object `$#`" % [key, h5attr.parent_name])
+
+proc read_all_attributes*(h5attr: H5Attributes) =
+  ## proc to read all attributes of the parent from file and store the names
+  ## and attribute ids in `h5attr`.
+  ## NOTE: If possible try to avoid using this proc! However, if you must, make
+  ## sure to close all attributes after usage, otherwise memory leaks might happen.
+  # first get how many objects there are
+  h5attr.num_attrs = h5attr.getNumAttrs
+  for i in 0 ..< h5attr.num_attrs:
+    var attr = newH5Attr()
+    attr.attr_id = openAttrByIdx(h5attr, i)
+    attr.opened = true
+    let name = getAttrName(attr.attr_id)
+    readAttributeInfo(h5attr, attr, name)
+
 proc deleteAttribute*(h5id: ParentID, name: string): bool =
   ## deletes the given attribute `name` on the object defined by
   ## the H5 id `h5id`
@@ -498,17 +508,11 @@
   ## throws:
   ##   KeyError: In case the key does not exist as an attribute
   # TODO: check err values!
-  let attr_exists = name in h5attr
-  var err: herr_t
-  if attr_exists:
-    # in case of existence, read the data and return
-    h5attr.readAttributeInfo(name)
-    let attr = h5attr.attr_tab[name]
-    result = attr.readAttribute(dtype)
-    # close attribute again after reading
-    h5attr.attr_tab[name].close()
-  else:
-    raise newException(KeyError, "No attribute `$#` exists in object `$#`" % [name, h5attr.parent_name])
+  h5attr.readAttributeInfo(name) # raises KeyError if it does not exist
+  let attr = h5attr.attr_tab[name]
+  result = attr.readAttribute(dtype)
+  # close attribute again after reading
+  h5attr.attr_tab[name].close()
 
 proc `[]`*[T](h5attr: H5Attributes, name: string, dtype: typedesc[T]): T =
   # convenience access to read_attribute
@@ -517,7 +521,8 @@
 proc `[]`*(h5attr: H5Attributes, name: string): DtypeKind =
   # accessing H5Attributes by string simply returns the datatype of the stored
   # attribute as an AnyKind value
-  h5attr.attr_tab[name].dtypeAnyKind
+  h5attr.readAttributeInfo(name, closeAttribute = true) # raises KeyError if it does not exist
+  result = h5attr.attr_tab[name].dtypeAnyKind
 
 proc `[]`*[T](attr: H5Attr, dtype: typedesc[T]): T =
   # convenience access to readAttribute for the actual attribute

src/nimhdf5/datatypes.nim

@@ -320,8 +320,8 @@
 
 proc close*(h5id: H5Id | H5IdObj, msg = "")
 proc `=copy`*(target: var H5IdObj, source: H5IdObj) {.error: "H5Id identifiers cannot be copied.".}
 proc `=destroy`*(h5id: var H5IdObj) = # {.error: "`=destroy` of a raw `H5Id` is not a valid operation.".}
   h5id.close()
   `=destroy`(h5id.kind)
   `=destroy`(h5id.id)
 
@@ -565,7 +565,8 @@
         $(attr.attr_id.id) & "!")
     withDebug:
       echo "Closed attribute with status ", err
-    attr.opened = false
+  # close state regardless of object, to make sure we don't leave it open accidentally
+  attr.opened = false
 
 proc close*(attr: H5Attr) = attr[].close()
 
@@ -628,9 +629,9 @@
 proc close*(dset: H5DataSet) = dset[].close()
 
 when (NimMajor, NimMinor, NimPatch) >= (1, 6, 0):
   proc `=destroy`*(attr: var H5AttrObj) {.raises: [HDF5LibraryError].} =
     ## Closes the given attribute.
     attr.close()
     for name, field in fieldPairs(attr):
       when typeof(field).distinctBase() isnot H5Id:
         `=destroy`(field)
@@ -638,21 +639,21 @@
         if cast[pointer](field) != nil:
           `=destroy`(field)
 
   proc `=destroy`*(attrs: var H5AttributesObj) =
     ## Closes all attributes that are stored in this `H5Attributes` table.
     if attrs.attr_tab != nil:
       `=destroy`(attrs.attr_tab)
     for name, field in fieldPairs(attrs):
       when typeof(field).distinctBase() isnot H5Id:
         if name.normalize != "attrtab":
           `=destroy`(field)
       else:
         if cast[pointer](field) != nil:
           `=destroy`(field)
 
   proc `=destroy`*(dset: var H5DataSetObj) {.raises: [HDF5LibraryError].} =
     ## Closes the dataset and resets all references to nil.
     dset.close() # closes its dataspace etc. as well
     dset.opened = false
     if dset.attrs != nil:
       `=destroy`(dset.attrs)
@@ -665,7 +666,7 @@
           `=destroy`(field)
 
 
   proc `=destroy`*(grp: var H5GroupObj) {.raises: [HDF5LibraryError].} =
     ## Closes the group and resets all references to nil.
     if grp.file_ref != nil:
       `=destroy`(grp.file_ref) # only destroy the `ref` to the file!
@@ -676,7 +677,7 @@
     if grp.groups != nil:
       `=destroy`(grp.groups)
     if grp.attrs != nil:
       `=destroy`(grp.attrs)
     for name, field in fieldPairs(grp):
       when typeof(field).distinctBase() isnot H5Id:
         if name notin ["attrs", "groups", "datasets", "file_ref"]:

src/nimhdf5/hdf5_json.nim

@@ -20,7 +20,7 @@
     `=destroy`(buf.buf)
 else:
   proc `=destroy`(buf: var BufferWrapperObj) =
     buf.cleanup(buf.buf, buf.dtypeID, buf.dspaceID)
     `=destroy`(buf.buf)
 
 ## Code to read also datasets and groups as JSON:
@@ -204,6 +204,9 @@
   ## to get access to the data as Nim objects, but also when copying attributes.
   ## Copying itself can also be done by simply getting the size, reading into a buffer,
   ## copying data type and space and writing the same buffer to a new location.
+
+  # read attribute info so that `attr_tab` knows it
+  h5attr.readAttributeInfo(name) # raises KeyError if it does not exist
   let attrObj {.inject.} = h5attr.attr_tab[name]
   case attrObj.dtypeAnyKind
   of dkBool:
@@ -287,6 +290,8 @@
   else:
     let attr {.inject.} = readJson(attrObj)
     actions
+  # close the attribute again
+  h5attr.attr_tab[name].close()
 
 ## The following JSON related procs are placeholders. We might implement them fully to be
 ## able to write compound data at runtime baesd on JSON data.

tests/tattributes.nim

@@ -32,19 +32,20 @@ proc write_attrs(grp: var H5Group) =
   grp.attrs["ComplexNamedSeqTuple"] = NamedTupSeqComplexAttr
 
 proc assert_attrs(grp: var H5Group) =
-  template readAndCheck(arg, typ, exp): untyped =
+  template readAndCheck(arg, typ, exp, kind): untyped =
     let data = grp.attrs[arg, typ]
     echo "Read: ", data
     doAssert data == exp, "Mismatch, was = " & $data & ", but expected = " & $exp
-
-  readAndCheck("Time", string, TimeStr)
-  readAndCheck("Counter", int, Counter)
-  readAndCheck("Seq", seq[int], SeqAttr)
-  readAndCheck("SeqStr", seq[string], SeqStrAttr)
-  readAndCheck("Tuple", (float, int), TupAttr)
-  readAndCheck("NamedTuple", tuple[foo: float, bar: int], NamedTupAttr)
-  readAndCheck("ComplexNamedTuple", tuple[foo: string, bar: float], NamedTupStrAttr)
-  readAndCheck("ComplexNamedSeqTuple", seq[tuple[foo: string, bar: float]], NamedTupSeqComplexAttr)
+    doAssert grp.attrs[arg] == kind, "Mismatch, was = " & $grp.attrs[arg] & ", but expected = " & $kind
+
+  readAndCheck("Time", string, TimeStr, dkString)
+  readAndCheck("Counter", int, Counter, dkInt64)
+  readAndCheck("Seq", seq[int], SeqAttr, dkSequence)
+  readAndCheck("SeqStr", seq[string], SeqStrAttr, dkSequence)
+  readAndCheck("Tuple", (float, int), TupAttr, dkObject)
+  readAndCheck("NamedTuple", tuple[foo: float, bar: int], NamedTupAttr, dkObject)
+  readAndCheck("ComplexNamedTuple", tuple[foo: string, bar: float], NamedTupStrAttr, dkObject)
+  readAndCheck("ComplexNamedSeqTuple", seq[tuple[foo: string, bar: float]], NamedTupSeqComplexAttr, dkSequence)
 
   doAssert("Time" in grp.attrs)
   doAssert("NoTime" notin grp.attrs)