Repository: NeurodataWithoutBorders/pynwb

Issue:
Bug: unittest test_standard_iterator_unbuffered seems buggy
Subtle bug, but here it goes:

On test_standard_iterator_unbuffered (tests/unit/form_tests/utils_test/test_core_DataChunkIterator), line 57 and 59, we have
dci = DataChunkIterator(data=range(10), buffer_size=1)
self.assertTupleEqual(dci.max_shape, (None,))

I claim it should read:
self.assertTupleEqual(dci.max_shape, (10,))

This is because in the constructor of DataChunkIterator we have:

```        if self.max_shape is None:
            # If the self.data object identifies it shape then use it
            if hasattr(self.data,  "shape"):
                self.max_shape = self.data.shape
                # Avoid the special case of scalar values by making them into a 1D numpy array
                if len(self.max_shape) == 0:
                    self.data = np.asarray([self.data, ])
                    self.max_shape = self.data.shape
                    self.__data_iter = iter(self.data)
            # Try to get an accurate idea of max_shape for other Python datastructures if possible.
            # Don't just callget_shape for a generator as that would potentially trigger loading of all the data
            elif isinstance(self.data, list) or isinstance(self.data, tuple):
                self.max_shape = ShapeValidator.get_data_shape(self.data)```
```

Initially, max_shape is None will be true, and data will not have attribut.  So the flow control will pass to line ```elif isinstance(self.data, list) or isinstance(self.data, tuple):```

However, this will not evaluate to true in python3, because data is type "range" (in python 2, it is type list, so the line will evaluate True, and max_shape would be set to 10).  

In python3 max_shape will not be set, so the test will pass, but I think this is incorrect behavior for the test.  I think ```elif isinstance(self.data, list) or isinstance(self.data, tuple):``` should be replaced by ```elif isinstance(self.data, (list, tuple, range)):``` and tests or other code should be adjusted accordingly.  This is a short job, but one prone to mistakes, and since it involves rewriting test behavior, I vote for @ajtritt to make the modifications if he agrees.


Discussion:

Patch:
diff --git a/src/form/data_utils.py b/src/form/data_utils.py
index 7e0f16f8..6b249dbc 100644
--- a/src/form/data_utils.py
+++ b/src/form/data_utils.py
@@ -98,7 +98,7 @@ class DataChunkIterator(AbstractDataChunkIterator):
             # Try to get an accurate idea of max_shape for other Python datastructures if possible.
             # Don't just callget_shape for a generator as that would potentially trigger loading of all the data
             elif isinstance(self.data, list) or isinstance(self.data, tuple):
-                self.max_shape = ShapeValidator.get_data_shape(self.data)
+                self.max_shape = ShapeValidator.get_data_shape(self.data, strict_no_data_load=True)
 
         # If we have a data iterator, then read the first chunk
         if self.__data_iter is not None: # and(self.max_shape is None or self.dtype is None):
@@ -108,13 +108,16 @@ class DataChunkIterator(AbstractDataChunkIterator):
         if self.max_shape is None and self.__next_chunk.data is not None:
             data_shape = ShapeValidator.get_data_shape(self.__next_chunk.data)
             self.max_shape = list(data_shape)
-            self.max_shape[0] = None
+            try:
+                self.max_shape[0] = len(self.data)  # We use self.data here because self.__data_iter does not allow len
+            except TypeError:
+                self.max_shape[0] = None
             self.max_shape = tuple(self.max_shape)
 
         # Determine the type of the data if possible
         if self.__next_chunk.data is not None:
             self.dtype = self.__next_chunk.data.dtype
-            self.__first_chunk_shape = self.__next_chunk.data.shape
+            self.__first_chunk_shape = ShapeValidator.get_data_shape(self.__next_chunk.data)
 
     def __iter__(self):
         """Return the iterator object"""


Test patch:
diff --git a/tests/unit/form_tests/utils_test/test_core_DataChunkIterator.py b/tests/unit/form_tests/utils_test/test_core_DataChunkIterator.py
index b57867c1..58d86c3f 100644
--- a/tests/unit/form_tests/utils_test/test_core_DataChunkIterator.py
+++ b/tests/unit/form_tests/utils_test/test_core_DataChunkIterator.py
@@ -56,22 +56,22 @@ class DataChunkIteratorTests(unittest.TestCase):
     def test_standard_iterator_unbuffered(self):
         dci = DataChunkIterator(data=range(10), buffer_size=1)
         self.assertEqual(dci.dtype, np.dtype(int))
-        self.assertTupleEqual(dci.max_shape, (None,))
-        self.assertTupleEqual(dci.recommended_data_shape(), (1,)) # Test before and after iteration
+        self.assertTupleEqual(dci.max_shape, (10,))
+        self.assertTupleEqual(dci.recommended_data_shape(), (10,)) # Test before and after iteration
         count = 0
         for chunk in dci:
             self.assertEqual(chunk.data.shape[0], 1)
             count+=1
         self.assertEqual(count, 10)
-        self.assertTupleEqual(dci.recommended_data_shape(), (1,)) # Test before and after iteration
+        self.assertTupleEqual(dci.recommended_data_shape(), (10,)) # Test before and after iteration
         self.assertIsNone(dci.recommended_chunk_shape())
 
     def test_standard_iterator_unmatched_buffersized(self):
         dci = DataChunkIterator(data=range(10), buffer_size=3)
         self.assertEquals(dci.dtype, np.dtype(int))
-        self.assertTupleEqual(dci.max_shape, (None,))
+        self.assertTupleEqual(dci.max_shape, (10,))
         self.assertIsNone(dci.recommended_chunk_shape())
-        self.assertTupleEqual(dci.recommended_data_shape(), (3,)) # Test before and after iteration
+        self.assertTupleEqual(dci.recommended_data_shape(), (10,)) # Test before and after iteration
         count = 0
         for chunk in dci:
             if count < 3:
@@ -79,7 +79,7 @@ class DataChunkIteratorTests(unittest.TestCase):
             else:
                 self.assertEqual(chunk.data.shape[0], 1)
             count +=1
-        self.assertTupleEqual(dci.recommended_data_shape(), (3,)) # Test before and after iteration
+        self.assertTupleEqual(dci.recommended_data_shape(), (10,)) # Test before and after iteration
         self.assertEqual(count, 4)
 
     def test_multidimensional_list(self):