[essimaging] Publish reduced scitiff from Odin bragg edge workflow#641
Conversation
…to compute TofDetector, add generic TofDetector and TofMonitor types but they are not inserted in wf
|
Needs scipp/scitiff#56 |
| x=normed.coords['x_pixel_offset'], | ||
| y=normed.coords['y_pixel_offset'], | ||
| t=sc.midpoints(normed.coords['tof']), |
There was a problem hiding this comment.
I think we should keep the coordinate names as they are, no...?
There was a problem hiding this comment.
I just assumed that people reading the images expect to have x/y as coordinates instead of our strange 'pixel_offset' coords?
There was a problem hiding this comment.
Also, we rename the dims below, so it sort of makes sense that the coords have the same name as the dims? But maybe I misunderstood the Scitiff data structure?
There was a problem hiding this comment.
The dimension names are limited so that we make sure of the order of those dimensions.
It's because, imaging processing SW such as ImageJ often assumes certain order of dimensions.
Coordinate names should stay informative so that people know what they actually mean on the other hand.
However, it doesn't matter too much since they will have units.... I guess...?
Like, t dimension can have either tof or wavelength but unit will tell you which is which.
It'll be more problematic if they have same units like tof and event_time_offset, but this is less likely to happen...?
About the x/y_pixel_offset. I think it is important to keep them as they are. It'll be less confusing even.
For example, x_pixel_offset doesn't represent its actual x-position of pixel in a certain coordinate.
x_pixel_offset is clear that it's only referring to the detector plane.
YooSunYoung
left a comment
There was a problem hiding this comment.
Looks good to me except for the minor comment about the coordinate names in the output file.
We add a unit test that publishes a normalized image as a scitiff file.
This is to be picked up by analysis team for further tests.
The would like to have
tofas the 3rd dimension, so we add a provider to compute it.