Merge pull request #30 from aldanor/feature/clone-copy

Make all high-level objects cloneable

Author: aldanor

CHANGELOG.md

@@ -22,6 +22,7 @@
 - Added basic support for reading and writing dataset slices.
 - When creating datasets, in-memory type layouts are normalized (converted to C repr).
 - Added `packed` option to `DatasetBuilder` (for creating packed HDF5 datasets).
+- All high-level objects now implement `Clone` (shallow copy, increases refcount).
 
 ### Changed
 
@@ -46,6 +47,7 @@
   user can user methods of the parent type without having to import any traits into scope
   (for instance, `File` dereferences into `Group`, which dereferences into `Location`,
   which dereferences into `Object`).
+- Dataspaces and property lists can now be copied via `.copy()` method (instead of `.clone()`).
 
 ### Fixed
 

hdf5-rs/src/dim.rs

@@ -4,8 +4,9 @@ use std::slice;
 pub type Ix = usize;
 
 /// A trait for the shape and index types.
-pub trait Dimension: Clone {
+pub trait Dimension {
     fn ndim(&self) -> usize;
+
     fn dims(&self) -> Vec<Ix>;
 
     fn size(&self) -> Ix {
@@ -22,6 +23,7 @@ impl<'a, T: Dimension> Dimension for &'a T {
     fn ndim(&self) -> usize {
         Dimension::ndim(*self)
     }
+
     fn dims(&self) -> Vec<Ix> {
         Dimension::dims(*self)
     }
@@ -31,6 +33,7 @@ impl Dimension for Vec<Ix> {
     fn ndim(&self) -> usize {
         self.len()
     }
+
     fn dims(&self) -> Vec<Ix> {
         self.clone()
     }
@@ -74,6 +77,7 @@ impl Dimension for Ix {
     fn ndim(&self) -> usize {
         1
     }
+
     fn dims(&self) -> Vec<Ix> {
         vec![*self]
     }

hdf5-rs/src/hl/container.rs

@@ -2,7 +2,7 @@ use std::fmt::{self, Debug};
 use std::mem;
 use std::ops::Deref;
 
-use ndarray::{Array, Array1, Array2, ArrayD, ArrayView, ArrayView1, Ix1, Ix2};
+use ndarray::{Array, Array1, Array2, ArrayD, ArrayView, ArrayView1};
 use ndarray::{SliceInfo, SliceOrIndex};
 
 use libhdf5_sys::h5a::{H5Aget_space, H5Aget_storage_size, H5Aget_type, H5Aread, H5Awrite};
@@ -386,6 +386,7 @@ impl<'a> Writer<'a> {
 }
 
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Container(Handle);
 
 impl ObjectClass for Container {

hdf5-rs/src/hl/dataset.rs

@@ -22,6 +22,7 @@ use crate::internal_prelude::*;
 
 /// Represents the HDF5 dataset object.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Dataset(Handle);
 
 impl ObjectClass for Dataset {
@@ -311,7 +312,7 @@ impl<T: H5Type> DatasetBuilder<T> {
 
                     let dims = match self.chunk {
                         Chunk::Manual(ref c) => c.clone(),
-                        _ => infer_chunk_size(shape.clone(), datatype.size()),
+                        _ => infer_chunk_size(&shape, datatype.size()),
                     };
 
                     ensure!(
@@ -399,7 +400,7 @@ impl<T: H5Type> DatasetBuilder<T> {
     }
 }
 
-fn infer_chunk_size<D: Dimension>(shape: D, typesize: usize) -> Vec<Ix> {
+fn infer_chunk_size<D: Dimension>(shape: &D, typesize: usize) -> Vec<Ix> {
     // This algorithm is borrowed from h5py, though the idea originally comes from PyTables.
 
     const CHUNK_BASE: f64 = (16 * 1024) as _;
@@ -452,27 +453,27 @@ pub mod tests {
 
     #[test]
     pub fn test_infer_chunk_size() {
-        assert_eq!(infer_chunk_size((), 1), vec![]);
-        assert_eq!(infer_chunk_size(0, 1), vec![1]);
-        assert_eq!(infer_chunk_size((1,), 1), vec![1]);
+        assert_eq!(infer_chunk_size(&(), 1), vec![]);
+        assert_eq!(infer_chunk_size(&0, 1), vec![1]);
+        assert_eq!(infer_chunk_size(&(1,), 1), vec![1]);
 
         // generated regression tests vs h5py implementation
-        assert_eq!(infer_chunk_size((65682868,), 1), vec![64144]);
-        assert_eq!(infer_chunk_size((56755037,), 2), vec![27713]);
-        assert_eq!(infer_chunk_size((56882283,), 4), vec![27775]);
-        assert_eq!(infer_chunk_size((21081789,), 8), vec![10294]);
-        assert_eq!(infer_chunk_size((5735, 6266), 1), vec![180, 392]);
-        assert_eq!(infer_chunk_size((467, 4427), 2), vec![30, 554]);
-        assert_eq!(infer_chunk_size((5579, 8323), 4), vec![88, 261]);
-        assert_eq!(infer_chunk_size((1686, 770), 8), vec![106, 49]);
-        assert_eq!(infer_chunk_size((344, 414, 294), 1), vec![22, 52, 37]);
-        assert_eq!(infer_chunk_size((386, 192, 444), 2), vec![25, 24, 56]);
-        assert_eq!(infer_chunk_size((277, 161, 460), 4), vec![18, 21, 58]);
-        assert_eq!(infer_chunk_size((314, 22, 253), 8), vec![40, 3, 32]);
-        assert_eq!(infer_chunk_size((89, 49, 91, 59), 1), vec![12, 13, 23, 15]);
-        assert_eq!(infer_chunk_size((42, 92, 60, 80), 2), vec![6, 12, 15, 20]);
-        assert_eq!(infer_chunk_size((15, 62, 62, 47), 4), vec![4, 16, 16, 12]);
-        assert_eq!(infer_chunk_size((62, 51, 55, 64), 8), vec![8, 7, 7, 16]);
+        assert_eq!(infer_chunk_size(&(65682868,), 1), vec![64144]);
+        assert_eq!(infer_chunk_size(&(56755037,), 2), vec![27713]);
+        assert_eq!(infer_chunk_size(&(56882283,), 4), vec![27775]);
+        assert_eq!(infer_chunk_size(&(21081789,), 8), vec![10294]);
+        assert_eq!(infer_chunk_size(&(5735, 6266), 1), vec![180, 392]);
+        assert_eq!(infer_chunk_size(&(467, 4427), 2), vec![30, 554]);
+        assert_eq!(infer_chunk_size(&(5579, 8323), 4), vec![88, 261]);
+        assert_eq!(infer_chunk_size(&(1686, 770), 8), vec![106, 49]);
+        assert_eq!(infer_chunk_size(&(344, 414, 294), 1), vec![22, 52, 37]);
+        assert_eq!(infer_chunk_size(&(386, 192, 444), 2), vec![25, 24, 56]);
+        assert_eq!(infer_chunk_size(&(277, 161, 460), 4), vec![18, 21, 58]);
+        assert_eq!(infer_chunk_size(&(314, 22, 253), 8), vec![40, 3, 32]);
+        assert_eq!(infer_chunk_size(&(89, 49, 91, 59), 1), vec![12, 13, 23, 15]);
+        assert_eq!(infer_chunk_size(&(42, 92, 60, 80), 2), vec![6, 12, 15, 20]);
+        assert_eq!(infer_chunk_size(&(15, 62, 62, 47), 4), vec![4, 16, 16, 12]);
+        assert_eq!(infer_chunk_size(&(62, 51, 55, 64), 8), vec![8, 7, 7, 16]);
     }
 
     #[test]

hdf5-rs/src/hl/datatype.rs

@@ -46,6 +46,7 @@ macro_rules! be_le {
 
 /// Represents the HDF5 datatype object.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Datatype(Handle);
 
 impl ObjectClass for Datatype {

hdf5-rs/src/hl/file.rs

@@ -18,6 +18,7 @@ use crate::internal_prelude::*;
 
 /// Represents the HDF5 file object.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct File(Handle);
 
 impl ObjectClass for File {

hdf5-rs/src/hl/group.rs

@@ -13,6 +13,7 @@ use crate::internal_prelude::*;
 
 /// Represents the HDF5 group object.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Group(Handle);
 
 impl ObjectClass for Group {
@@ -198,6 +199,25 @@ pub mod tests {
         })
     }
 
+    #[test]
+    pub fn test_clone() {
+        with_tmp_file(|file| {
+            file.create_group("a").unwrap();
+            let a = file.group("a").unwrap();
+            assert_eq!(a.name(), "/a");
+            assert_eq!(a.file().unwrap().id(), file.id());
+            assert_eq!(a.refcount(), 1);
+            let b = a.clone();
+            assert_eq!(b.name(), "/a");
+            assert_eq!(b.file().unwrap().id(), file.id());
+            assert_eq!(b.refcount(), 2);
+            assert_eq!(a.refcount(), 2);
+            drop(a);
+            assert_eq!(b.refcount(), 1);
+            assert!(b.is_valid());
+        })
+    }
+
     #[test]
     pub fn test_len() {
         with_tmp_file(|file| {

hdf5-rs/src/hl/location.rs

@@ -12,6 +12,7 @@ use crate::internal_prelude::*;
 
 /// Named location (file, group, dataset, named datatype).
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Location(Handle);
 
 impl ObjectClass for Location {

hdf5-rs/src/hl/object.rs

@@ -6,6 +6,7 @@ use crate::internal_prelude::*;
 
 /// Any HDF5 object that can be referenced through an identifier.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Object(Handle);
 
 impl ObjectClass for Object {

hdf5-rs/src/hl/plist.rs

@@ -13,6 +13,7 @@ pub mod file_create;
 
 /// Represents the HDF5 property list.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct PropertyList(Handle);
 
 impl ObjectClass for PropertyList {
@@ -46,13 +47,6 @@ impl Deref for PropertyList {
     }
 }
 
-impl Clone for PropertyList {
-    fn clone(&self) -> Self {
-        let id = h5call!(H5Pcopy(self.id())).unwrap_or(H5I_INVALID_HID);
-        Self::from_id(id).ok().unwrap_or_else(Self::invalid)
-    }
-}
-
 impl PartialEq for PropertyList {
     fn eq(&self, other: &Self) -> bool {
         h5call!(H5Pequal(self.id(), other.id())).unwrap_or(0) == 1
@@ -156,6 +150,11 @@ impl FromStr for PropertyListClass {
 }
 
 impl PropertyList {
+    /// Copies the property list.
+    pub fn copy(&self) -> Self {
+        Self::from_id(h5lock!(H5Pcopy(self.id()))).unwrap_or_else(|_| Self::invalid())
+    }
+
     /// Queries whether a property name exists in the property list.
     pub fn has(&self, property: &str) -> bool {
         to_cstring(property)
@@ -244,7 +243,7 @@ pub mod tests {
     pub fn test_clone() {
         let (fapl, _) = make_plists();
         assert!(fapl.is_valid());
-        let fapl_c = fapl.clone();
+        let fapl_c = fapl.copy();
         assert!(fapl.is_valid());
         assert!(fapl_c.is_valid());
         assert_eq!(fapl.refcount(), 1);

hdf5-rs/src/hl/plist/file_create.rs

@@ -28,6 +28,7 @@ use crate::internal_prelude::*;
 
 /// File creation properties.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct FileCreate(Handle);
 
 impl ObjectClass for FileCreate {
@@ -88,12 +89,6 @@ impl PartialEq for FileCreate {
 
 impl Eq for FileCreate {}
 
-impl Clone for FileCreate {
-    fn clone(&self) -> Self {
-        unsafe { self.deref().clone().cast() }
-    }
-}
-
 /// Size of the offsets and lengths used in a file.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
 pub struct SizeofInfo {
@@ -400,6 +395,10 @@ impl FileCreate {
         Self::from_id(h5try!(H5Pcreate(*H5P_FILE_CREATE)))
     }
 
+    pub fn copy(&self) -> Self {
+        unsafe { self.deref().copy().cast() }
+    }
+
     pub fn build() -> FileCreateBuilder {
         FileCreateBuilder::new()
     }

hdf5-rs/src/hl/space.rs

@@ -3,7 +3,7 @@ use std::fmt::{self, Debug};
 use std::ops::Deref;
 use std::ptr;
 
-use ndarray::{SliceInfo, SliceOrIndex};
+use ndarray::SliceOrIndex;
 
 use libhdf5_sys::h5s::{
     H5Scopy, H5Screate_simple, H5Sget_simple_extent_dims, H5Sget_simple_extent_ndims,
@@ -14,6 +14,7 @@ use crate::internal_prelude::*;
 
 /// Represents the HDF5 dataspace object.
 #[repr(transparent)]
+#[derive(Clone)]
 pub struct Dataspace(Handle);
 
 impl ObjectClass for Dataspace {
@@ -53,6 +54,11 @@ impl Deref for Dataspace {
 }
 
 impl Dataspace {
+    /// Copies the dataspace.
+    pub fn copy(&self) -> Self {
+        Self::from_id(h5lock!(H5Scopy(self.id()))).unwrap_or_else(|_| Self::invalid())
+    }
+
     /// Select a slice (known as a 'hyperslab' in HDF5 terminology) of the Dataspace.
     /// Returns the shape of array that is capable of holding the resulting slice.
     /// Useful when you want to read a subset of a dataset.
@@ -160,13 +166,6 @@ impl Dimension for Dataspace {
     }
 }
 
-impl Clone for Dataspace {
-    fn clone(&self) -> Self {
-        let id = h5call!(H5Scopy(self.id())).unwrap_or(H5I_INVALID_HID);
-        Self::from_id(id).ok().unwrap_or_else(Self::invalid)
-    }
-}
-
 #[cfg(test)]
 pub mod tests {
     use crate::internal_prelude::*;
@@ -214,7 +213,7 @@ pub mod tests {
 
         assert_err!(Dataspace::from_id(H5I_INVALID_HID), "Invalid dataspace id");
 
-        let dc = d.clone();
+        let dc = d.copy();
         assert!(dc.is_valid());
         assert_ne!(dc.id(), d.id());
         assert_eq!((d.ndim(), d.dims(), d.size()), (dc.ndim(), dc.dims(), dc.size()));