Warn on detector components falling back to raw data#712
Conversation
philsmt
left a comment
philsmt
left a comment
There was a problem hiding this comment.
I would probably opt for a more aggressive break with backwards compatibility here in light on the upcoming removal of legacy sources, but I also don't mind the compromise.
LGTM.
|
|
||
| if (raw is False) and any(cls._data_is_raw(data, s) for s in source_to_modno): | ||
| raise SourceNameError(f'{detector_name}/CORR/...') | ||
| if any(cls._data_is_raw(data, s) for s in source_to_modno): |
There was a problem hiding this comment.
n.b. I was curious how this method is implemented and realized its check for image.data dtype can fail if photonization is used. I will open a PR for that.
There was a problem hiding this comment.
There's already an override for AGIPD for precisely that region. I think that's the only detector where we have photonisation so far? Of course, if we're changing that, we should update this. The obvious thing would be to check for one of the extra keys like image.mask, but it's possible to select that out of the data before making the component.
There was a problem hiding this comment.
I was thinking of an early
if '/CORR/' in source:
return False
|
I'm figuring we can probably remove the fallback in another cycle or two. It is pretty easy to adapt with |
2 participants
When instantiating a detector component, if
raw=is not specified, it looks for corrected data first and then falls back to raw data if no corrected data is found. This was a compromise between having the defaults give people corrected data, and keeping existing code withopen_run(..., data='raw')working the same way.However, this fallback is an occasional source of confusion - if correction fails, or you open a run before the corrected files exist, or if code that works with recent (renamed sources) data is run on older data, you're suddenly trying to use raw data and getting garbage results. I'd like to remove the fallback and change the default to
raw=Falsein time, but for now this adds a warning if the fallback occurs, pushing people towards specifyingraw=explcitly.We added the raw parameter and changed the default behaviour in
open_run()- to combine raw & proc - in January 2025.cc @turkot @philsmt