demo: fix example plugin

[japie1235813]

• Replace old multiplexer with MultiplexerDataProvider and make related changes.
• Add Description column

[AnimeshSinha1309]

I tested this, it works great for me, thanks and that solve #5355.

[nfelt]

Thanks for the fixes!

[nfelt]

If there is no actual data for this run and tag, the result will be an empty dictionary, but then on the next line data[0] will fail. It's also not actually the right type - to match read_tensors(), that type should be a list.

I'd just change it to .get(tag, []) and then add an explicit check, like

if not data:
    raise werkzeug.exceptions.BadRequest("Invalid run or tag")
event_data = [data[0].numpy.item().decode("utf-8")]

The except KeyError below only worked with multiplexer.Tensors(), it won't work with the code you have now since .get() on a dict won't actually raise a KeyError.

[japie1235813]

Would data[0] be caught in the except by replacing KeyError with IndexError? (I ran a quick python terminal and looks like it does)
However I prefer add if not data but then I think the try...except could just be removed (or is there anything I miss?)

[nfelt]

Yes, I wasn't totally clear but with the suggested if not data code, you don't need the try...except any more.

FWIW, just to clarify my earlier comment a little - the except KeyError in the previous code was there in order to handle the case where multiplexer.Tensors() is called with a run/tag combination that has no data, since in that case the API would raise a KeyError. In the new code, the equivalent path that produces the list of Events (which is read_tensors().get(run, {}).get(tag, [])) won't raise any exception at all, even if the run and tag don't exist, because each .get() will just return the default value, so you get an empty list rather than an exception. The new code will still raise an exception eventually - but it's the next step where we call data[0] that would raise the exception here, which is a little different. That said, you're right that even if it's coming from a different step in the lookup process, we could preserve the try...except and just catch IndexError instead of KeyError. Up to you which you prefer.

Update: this will probably change anyway since we most likely don't want to do data[0] after all, see other comment.

[japie1235813]

Thanks for the clarification. I remove the try...except and use the condition to raise the exception. Also made some related changes.

[nfelt]

Yes, I wasn't totally clear but with the suggested if not data code, you don't need the try...except any more.

FWIW, just to clarify my earlier comment a little - the except KeyError in the previous code was there in order to handle the case where multiplexer.Tensors() is called with a run/tag combination that has no data, since in that case the API would raise a KeyError. In the new code, the equivalent path that produces the list of Events (which is read_tensors().get(run, {}).get(tag, [])) won't raise any exception at all, even if the run and tag don't exist, because each .get() will just return the default value, so you get an empty list rather than an exception. The new code will still raise an exception eventually - but it's the next step where we call data[0] that would raise the exception here, which is a little different. That said, you're right that even if it's coming from a different step in the lookup process, we could preserve the try...except and just catch IndexError instead of KeyError. Up to you which you prefer.

Update: this will probably change anyway since we most likely don't want to do data[0] after all, see other comment.

[nfelt]

Actually, rather than what I said earlier, in order to handle multiple events (for the case where we allow downsample to be >1, i.e. we might return multiple steps' worth of data for a given time series), we should process all the elements of data - something like:

event_data = [datum.numpy.item().decode("utf-8") for datum in data]

[japie1235813]

Thanks for the example code. I've applied and checked the result. It makes more sense to me now (screenshot updated in the pr description above). Please have a second look. Thanks!

[nfelt]

🎉 Thanks again for updating this!