Repository: mila-udem/fuel
Pull request #151 (2015-06-04 15:57:36)

Refactored HDF5 dataset
I renamed it to `PytablesDataset`, refactored and added some documentation.

dmitriy-serdyuk: CC @vdumoulin 
vdumoulin: I'll have a look after lunch, thanks!
bartvm: What's the use case for this class right now? If possible, we should probably just stick with `h5py`, because this duplicates a lot of functionality, but I know there's some more advanced features that are only in PyTables...
dmitriy-serdyuk: We have our datasets in pytables and I suspect that there can be people who still use this class. Also it is much more simple and flexible, it may be a good starting point to create an interface to one's own pytables dataset.
dmitriy-serdyuk: Why the coverage decreased so much?
bartvm: Seems like your test isn't being run: https://coveralls.io/builds/2730193/source?filename=fuel%2Fdatasets%2Fhdf5.py
vdumoulin: I think we should leave it in, since as @dmitriy-serdyuk said, some people are using it. When we decided that we would be focusing on `H5PYDataset`, we also decided to leave other options to people, even if those options weren't as polished or maintained.

Regarding the name change, is it sufficient to announce it on the mailing list?
bartvm: Yeah, an e-mail to the mailing list is enough.
vdumoulin: There's some weird stuff going on: Travis says the tests pass, but looking at the logs I see errors.
dmitriy-serdyuk: I cannot open Travis. I think they have problems again.
bartvm: #152
bartvm: Rebasing on current master should fix Travis's build status. It seems like there are other tests broken though.

Patch:
diff --git a/fuel/datasets/hdf5.py b/fuel/datasets/hdf5.py
index b9788ce..7beefec 100644
--- a/fuel/datasets/hdf5.py
+++ b/fuel/datasets/hdf5.py
@@ -11,26 +11,33 @@ from fuel.utils import do_not_pickle_attributes
 
 
 @do_not_pickle_attributes('nodes', 'h5file')
-class Hdf5Dataset(Dataset):
-    """An HDF5 dataset.
+class PytablesDataset(Dataset):
+    """A pytables dataset.
+
+    An HDF5 Dataset which was created with pytables. The dataset should
+    have the following structure: `/<data_node>/paths/to/sources`. In
+    order to have train/validation/test split you may want to open
+    several datasets with different data nodes or source paths. It is
+    also possible to use start and stop arguments to split your dataset.
 
     Parameters
     ----------
     sources : tuple of strings
-        Sources which the dataset returns
+        Sources which the dataset returns.
     start : int
-        Start index
+        Start index. Optional, by default is 0.
     stop : int
-        Stop index
+        Stop index. Optional, if is not provided, will be set to the
+        number of rows of the first source.
     data_node : str
-        Parent data node in HDF5 file
+        Parent data node in HDF5 file, all path are relative to this node.
     sources_in_file : tuple of strings
         Names of nodes in HDF5 file which contain sources. Should the same
         length as `sources`.
         Optional, if not set will be equal to `sources`.
 
     """
-    def __init__(self, sources, start, stop, path, data_node='Data',
+    def __init__(self, path, sources, start=0, stop=None, data_node='Data',
                  sources_in_file=None):
         if sources_in_file is None:
             sources_in_file = sources
@@ -40,22 +47,26 @@ class Hdf5Dataset(Dataset):
         self.data_node = data_node
         self.start = start
         self.stop = stop
-        self.num_examples = self.stop - self.start
         self.nodes = None
-        self.open_file(self.path)
-        super(Hdf5Dataset, self).__init__(self.provides_sources)
+        self.open_file(path)
+        super(PytablesDataset, self).__init__(self.provides_sources)
 
     def open_file(self, path):
         self.h5file = tables.open_file(path, mode="r")
-        node = self.h5file.getNode('/', self.data_node)
+        node = self.h5file.get_node('/', self.data_node)
 
         self.nodes = [getattr(node, source) for source in self.sources_in_file]
+        if self.stop is None:
+            self.stop = self.nodes[0].nrows
+        self.num_examples = self.stop - self.start
 
     def load(self):
         self.open_file(self.path)
 
-    def close(self):
+    def close_file(self):
         self.h5file.close()
+        del self._h5file
+        del self._nodes
 
     def get_data(self, state=None, request=None):
         """ Returns data from HDF5 dataset.
@@ -63,16 +74,15 @@ class Hdf5Dataset(Dataset):
         .. note:: The best performance if `request` is a slice.
 
         """
-        if self.start:
-            if isinstance(request, slice):
-                request = slice(request.start + self.start,
-                                request.stop + self.start, request.step)
-                data = [node[request] for node in self.nodes]
-            elif isinstance(request, list):
-                request = [index + self.start for index in request]
-                data = [node[request, ...] for node in self.nodes]
-            else:
-                raise ValueError
+        if isinstance(request, slice):
+            request = slice(request.start + self.start,
+                            request.stop + self.start, request.step)
+            data = [node[request] for node in self.nodes]
+        elif isinstance(request, list):
+            request = [index + self.start for index in request]
+            data = [node[request, ...] for node in self.nodes]
+        else:
+            raise ValueError
         return data
 
 


Test patch:
diff --git a/tests/test_hdf5.py b/tests/test_hdf5.py
index df1fa5f..c5bd11f 100644
--- a/tests/test_hdf5.py
+++ b/tests/test_hdf5.py
@@ -6,11 +6,11 @@ import numpy
 from numpy.testing import assert_equal, assert_raises
 from six.moves import range, cPickle
 
-from fuel.datasets.hdf5 import Hdf5Dataset, H5PYDataset
+from fuel.datasets.hdf5 import PytablesDataset, H5PYDataset
 from fuel.streams import DataStream
 
 
-class TestHdf5Dataset(object):
+class TestPytablesDataset(object):
     def setUp(self):
         num_rows = 500
         filters = tables.Filters(complib='blosc', complevel=5)
@@ -24,12 +24,18 @@ class TestHdf5Dataset(object):
             y[i] = i
         h5file.flush()
         h5file.close()
-        self.dataset = Hdf5Dataset(['y'], 20, 500, 'tmp.h5')
+        self.dataset = PytablesDataset('tmp.h5', ('y',), 20, 500)
+        self.dataset_default = PytablesDataset('tmp.h5', ('y',))
 
     def tearDown(self):
-        self.dataset.close()
+        self.dataset.close_file()
+        self.dataset_default.close_file()
         os.remove('tmp.h5')
 
+    def test_dataset_default(self):
+        assert self.dataset_default.start == 0
+        assert self.dataset_default.stop == 500
+
     def test_get_data_slice_request(self):
         assert_equal(self.dataset.get_data(request=slice(0, 10))[0],
                      numpy.arange(20, 30).reshape(10, 1))