Conversation
nvaytet
left a comment
There was a problem hiding this comment.
Looks good 👍
Let's see what the CI says once we release essreduce.
|
Changes look ok, but there seems to be something up with the |
…the values from the LUT
| proton_current=proton_current, | ||
| graph={ | ||
| **graph, | ||
| "wavelength": lambda wavelength_from_mcstas: wavelength_from_mcstas, |
|
|
||
| return batch.compute(target) | ||
| result = batch.compute(target) | ||
| return result if runs_is_mapping else list(result.values()) |
There was a problem hiding this comment.
Do we want to do this, or do we always want to return a mapping? (which is easier to work with I think?)
We could above just convert runs to a dict if it is not a mapping, and leave the rest un-touched?
There was a problem hiding this comment.
The type hints of the function says that it accepts either a sequence or a mapping, but currently if you pass a sequence it errors.
There was a problem hiding this comment.
I think in some cases it can be more convenient to just pass a sequence and get a sequence back 🤷
Co-authored-by: Neil Vaytet <39047984+nvaytet@users.noreply.github.com>
Needs a new release of essreduce before being merged.