Add support for selecting a TOA range in detector plots #304
Conversation
| if (toa_range := self._toa_range()) is None: | ||
| return data | ||
| low, high = toa_range | ||
| return data.bins.assign_coords(toa=data.bins.data).bins['toa', low:high] |
There was a problem hiding this comment.
I need some explanation here 😄
- What is
data? (What shape shape and what fields is it expected to have?) - Why do we need to assign coords?
- Why is
toa=data.bins.data?
| options=[ | ||
| {'label': 'Filter by time-of-arrival [μs]', 'value': 'enabled'} | ||
| ], | ||
| value=[], |
There was a problem hiding this comment.
(minor) checklist is slightly odd for what seems to be a "toggle" on/off, maybe consider https://dash.plotly.com/dash-daq/booleanswitch
There was a problem hiding this comment.
That appears to be from a different library (dash_daq)?
| sc.scalar(value['low'], unit=value['unit']).to(unit='ns'), | ||
| sc.scalar(value['high'], unit=value['unit']).to(unit='ns'), |
There was a problem hiding this comment.
Will the unit always be 'ns', or should we instead just store floats and use the units of data.bins.coords['toa'] in apply_toa_range below?
There was a problem hiding this comment.
The bounds come via a message from Kafka. It is essential to send to unit with this (or you will waste hours debugging). Not sure what is the benefit from your suggestion, recreating the low and high variables in every single call below? It is just making it a bit slower?
| dcc.Checklist( | ||
| id='toa-checkbox', | ||
| options=[ | ||
| {'label': 'Filter by time-of-arrival [μs]', 'value': 'enabled'} |
There was a problem hiding this comment.
It seems it's microseconds here and nanoseconds above?
There was a problem hiding this comment.
The text in the html.Label is 'Time-of-arrival range (us)', and the unit label in the square brackets is Filter by time-of-arrival [μs]?
Unless this is not the same time as in the _convert_toa_range function?
There was a problem hiding this comment.
Not sure I understand, us is just another spelling of microsecond?
There was a problem hiding this comment.
What I meant was that on L149-150 in detector_data_handler.py you seem to have converted them to nanoseconds?
By 'above' I meant further up the page, not in the same file, sorry.
There was a problem hiding this comment.
What is wrong with converting microseconds to nanoseconds? The dashboard uses microseconds, since nanoseconds are a bit unwieldy for a user to think about.
Fixes #305.