Skip to content

Commit

Permalink
Make Imageset slices consistently index from 0
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dagewa authored Jul 26, 2023
1 parent e9b88d2 commit 69169ff
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 23 deletions.
1 change: 1 addition & 0 deletions newsfragments/633.bugfix
Original file line number Diff line number Diff line change
@@ -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.
23 changes: 3 additions & 20 deletions src/dxtbx/imageset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
3 changes: 1 addition & 2 deletions src/dxtbx/model/experiment_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
8 changes: 7 additions & 1 deletion tests/test_imageset.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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):
Expand Down

0 comments on commit 69169ff

Please sign in to comment.