Skip to content

Fix failure to ignore darks/flats when getting the other from separate file#683

Merged
yousefmoazzam merged 1 commit intomainfrom
imgda-650
Feb 11, 2026
Merged

Fix failure to ignore darks/flats when getting the other from separate file#683
yousefmoazzam merged 1 commit intomainfrom
imgda-650

Conversation

@yousefmoazzam
Copy link
Collaborator

@yousefmoazzam yousefmoazzam commented Feb 10, 2026

Fixes IMGDA-650

An alternative to #656, as a precursor to the refactoring of the darks/flats config class.

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have made corresponding changes to the documentation

@dkazanc
Copy link
Collaborator

dkazanc commented Feb 11, 2026

thanks @yousefmoazzam , looks good. So in this instance _empty_aux_array from dataset.py is not called as you create a dummy array of zeroes or ones directly here? I'm trying to understand in what situation _empty_aux_array is triggered anyway?

@yousefmoazzam
Copy link
Collaborator Author

yousefmoazzam commented Feb 11, 2026

I've not looked into _empty_aux_array(), I actually accidentally stumbled upon this way to fix the issue during refactoring when I was explicitly enumerating each possible case in terms of code paths.

From a cursory look at where the function is mentioned https://github.com/search?q=repo%3ADiamondLightSource%2Fhttomo%20_empty_aux_array(&type=code, it looks like it gets called only when the darks/flats property is accessed but their value is None.

So I'd guess that, somehow, in the other PR to fix this, the darks/flats were None for some reason - and so changing _empty_aux_data() was a viable way to fix it.

(As I was already doing refactoring that was on top of this, I decided to split this out to make it clear where the bug fix was, rather than having the bug fix be buried within the subsequent refactoring).

@dkazanc
Copy link
Collaborator

dkazanc commented Feb 11, 2026

Thanks! Yes, I was curious about it just because there was this bug which might need fixing if any other case refers to _empty_aux_array() and that array is getting build. For that change all looks fine, thanks for the separate PR on this.

@yousefmoazzam yousefmoazzam merged commit 8af931c into main Feb 11, 2026
6 of 7 checks passed
@yousefmoazzam yousefmoazzam deleted the imgda-650 branch February 11, 2026 12:25
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.

2 participants