Skip to content

Avoid reading h5 imagesets as having a single image, when a format class is not available.#825

Merged
jbeilstenedmands merged 5 commits intomainfrom
fix_import_xds_imagesets
Jun 3, 2025
Merged

Avoid reading h5 imagesets as having a single image, when a format class is not available.#825
jbeilstenedmands merged 5 commits intomainfrom
fix_import_xds_imagesets

Conversation

@jbeilstenedmands
Copy link
Contributor

Attempt to fix an issue described in dials/dials#2944

I will try to create a minimal reproducer of the issue, but my basic understanding of the underlying cause is that these parts of the code should not always be defaulting to use Format when the true format class is not known (as we can't pass an image range to set up the 'Reader' correctly). That is appropriate for a e.g. cbf template but not data from a master.h5. Rather we need a FormatMultiImage in these cases. This kind of pattern seems to happen in other functions in this file, and I guess it is not a common code path for imageset creation as we usually know the format.

The dxtbx tests and dials command line tests all seem to pass still.

@jbeilstenedmands jbeilstenedmands requested a review from dagewa May 29, 2025 12:53
Copy link
Member

@dagewa dagewa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original function is a bit messy - it has specific behaviour according to whether the string "master" is in the template, which is surely fragile, and implies this special case is only intended for *_master.h5-style DECTRIS data. There are other image stack types, like MRC files, and I suppose for those other stack types we'd also want to create FormatMultiImage.

However, I think fixing all that would require rather more invasive changes. I'm satisfied that this is a reasonable fix for dials/dials#2944, and it seems that errors like that are rare in practice.

@jbeilstenedmands jbeilstenedmands merged commit da699ca into main Jun 3, 2025
13 checks passed
@jbeilstenedmands jbeilstenedmands deleted the fix_import_xds_imagesets branch June 3, 2025 10:46
aaronfinke pushed a commit to aaronfinke/dxtbx that referenced this pull request Oct 7, 2025
…ass is not available. (cctbx#825)

In order to make sure the xds.to_imageset function returns an imageset of the correct length for h5 image files.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants