h2oai / wave

Realtime Web Apps and Dashboards for Python and R
https://wave.h2o.ai
Apache License 2.0
3.9k stars 323 forks source link

feat: Format dates inside plot tooltips properly. #2149

Closed mturoci closed 9 months ago

mturoci commented 9 months ago

@marek-mihok - can you make a code review and a thorough QA to check if everything works as expected?

marek-mihok commented 9 months ago

LGTM @mturoci! I went through all of the plot examples and also tested different scenarios (e.g. updating the data) and everything is working fine. The code also makes sense. There's only one thing I've noticed: After your change the date is also formatted in examples with only the year specified, e.g. plot_line.py.

This PR:

image

Current master:

image

However, I think your implementation makes sense (to format tooltip date if x_scale === 'time') so I would only suggest removing x_scale='time' prop from these .py examples. WDYT?

mturoci commented 9 months ago

I would only suggest removing x_scale='time' prop from these .py examples. WDYT?

I don't think it's necessary since people may stumble into this scenario regardless of its presence in examples. I would say this is expected since scale='time'. If anyone wants a plain number in the tooltip, they should go with the default scale instead.

marek-mihok commented 9 months ago

If anyone wants a plain number in the tooltip, they should go with the default scale instead

@mturoci agreed. Then it's good to go.