Make Imageset slices consistently index from 0

Without this, slices sometimes needed to be indexed by their original
image index, even inside the slice.

The previous workaround in 67aab2 (#485) for negative offset was
reacting to the erroneous behaviour introduced by including batch
offsets in imagesequence slicing.

newsfragments/633.bugfix

@@ -0,0 +1 @@
+Slicing of imageset objects is now consistently 0-based, including for the sliced data accessor. Previously, the data accessor had to be accessed with the original index offsets.

src/dxtbx/imageset.py

@@ -300,29 +300,12 @@ def __getitem__(self, item):
         if not isinstance(item, slice):
             return self.get_corrected_data(item)
         else:
-            offset = self.get_scan().get_batch_offset()
             if item.step is not None:
                 raise IndexError("Sequences must be sequential")
 
-            # nasty workaround for https://github.com/dials/dials/issues/1153
-            # slices with -1 in them are meaningful :-/ so grab the original
-            # constructor arguments of the slice object.
-            # item.start and item.stop may have been compromised at this point.
-            if offset < 0:
-                start, stop, step = item.__reduce__()[1]
-                if start is None:
-                    start = 0
-                else:
-                    start -= offset
-                if stop is None:
-                    stop = len(self)
-                else:
-                    stop -= offset
-            else:
-                start = item.start or 0
-                stop = item.stop or (len(self) + offset)
-                start -= offset
-                stop -= offset
+            start = item.start or 0
+            stop = item.stop or len(self)
+
             if self.data().has_single_file_reader():
                 reader = self.reader().copy(self.reader().paths(), stop - start)
             else:

src/dxtbx/model/experiment_list.py

@@ -625,8 +625,7 @@ def from_sequence_and_crystal(imageset, crystal, load_models=True):
             # if imagesequence is still images, make one experiment for each
             # all referencing into the same image set
             if imageset.get_scan().is_still():
-                start, end = imageset.get_scan().get_array_range()
-                for j in range(start, end):
+                for j in range(len(imageset)):
                     subset = imageset[j : j + 1]
                     experiments.append(
                         Experiment(

tests/test_imageset.py

@@ -387,6 +387,11 @@ def tst_get_item(self, sequence):
         with pytest.raises(RuntimeError):
             _ = sequence2[5]
 
+        # Check data access matches expected images from the slice
+        panel_data1 = sequence[3][0]
+        panel_data2 = sequence2[0][0]
+        assert panel_data1.all_eq(panel_data2)
+
         assert len(sequence2) == 4
         assert_can_get_detectorbase(sequence2, range(0, 4), 5)
         self.tst_get_models(sequence2, range(0, 4), 5)
@@ -396,11 +401,12 @@ def tst_get_item(self, sequence):
         with pytest.raises(IndexError):
             _ = sequence[3:7:2]
 
+        # Batch offset should not affect slicing of imagesequence
         # Simulate a scan starting from image 0
         sequence_ = copy.deepcopy(sequence)
         sequence_.get_scan().set_batch_offset(-1)
         sequence3 = sequence_[3:7]
-        assert sequence3.get_array_range() == (4, 8)
+        assert sequence3.get_array_range() == (3, 7)
 
     @staticmethod
     def tst_paths(sequence, filenames1):