Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slice using batch offsets fix #2472

Merged
merged 2 commits into from
Jul 27, 2023
Merged

Slice using batch offsets fix #2472

merged 2 commits into from
Jul 27, 2023

Conversation

jbeilstenedmands
Copy link
Contributor

Missed update in #2411 as highlighted by xia2 test failures

@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #2472 (657c67b) into main (c706b5b) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 657c67b differs from pull request most recent head 81cc130. Consider uploading reports for the commit 81cc130 to get more accurate results

@@            Coverage Diff             @@
##             main    #2472      +/-   ##
==========================================
- Coverage   78.64%   78.64%   -0.01%     
==========================================
  Files         606      606              
  Lines       74245    74244       -1     
  Branches    10093    10093              
==========================================
- Hits        58393    58391       -2     
  Misses      13704    13704              
- Partials     2148     2149       +1     

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.

Thanks for finding and fixing this. I did not spot it before as I didn't run xia2 tests on the PR pair.

I have checked this and agree that your change is correct. As noted, it essentially reverts e58d8fa, one of the fixes-upon-fixes that I have been trying to disentangle in this whole process.

Imagesets are accessed using 0-based indices, and sliced thusly. The scan [ behaves in the same way, therefore calculating beg and end as 0-based and then using them for both slice operations is correct. To paraphrase the code, this is correct:

beg = sr[0] - 1 - arr_start
end = sr[1] - arr_start
exp.scan[beg:end]
exp.imageset = exp.imageset[beg:end]

@jbeilstenedmands jbeilstenedmands merged commit 661432b into main Jul 27, 2023
16 checks passed
@jbeilstenedmands jbeilstenedmands deleted the imageset_slice_fallout branch July 27, 2023 16:11
dagewa pushed a commit that referenced this pull request Jul 31, 2023
Don't slice using batch offsets
toastisme pushed a commit to toastisme/dials that referenced this pull request Aug 21, 2023
Don't slice using batch offsets
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.

None yet

3 participants