From be887f73803e15ca5140ecc2a5237510bbb81587 Mon Sep 17 00:00:00 2001 From: Joe Date: Fri, 10 May 2024 10:10:03 -0500 Subject: [PATCH] fix: Improve TimePreprocessor code (#455) Tests were failing on #453 due to some parsing issues This is similar to https://github.com/deephaven/deephaven-core/issues/5373 which I thought was fixed. Regardless, we shouldn't be using `to_j_instant` in this way due to poor performance. For now I think assuming the column is an `Instant` is fine. --- .../src/deephaven/plot/express/plots/bar.py | 4 ++-- .../plot/express/preprocess/TimePreprocessor.py | 12 +++++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/plugins/plotly-express/src/deephaven/plot/express/plots/bar.py b/plugins/plotly-express/src/deephaven/plot/express/plots/bar.py index 7e2b51369..e2bd97504 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/plots/bar.py +++ b/plugins/plotly-express/src/deephaven/plot/express/plots/bar.py @@ -274,9 +274,9 @@ def timeline( table: Table | None: (Default value = None) A table to pull data from. x_start: str | None: (Default value = None) - A column that contains starting x-axis values. + A column that contains starting x-axis values. Must be a `java.time.Instant` column. x_end: str | None: (Default value = None) - A column that contains ending x-axis values. + A column that contains ending x-axis values. Must be a `java.time.Instant` column. by: str | list[str] | None: (Default value = None) A column or list of columns that contain values to plot the figure traces by. All values or combination of values map to a unique design. The variable diff --git a/plugins/plotly-express/src/deephaven/plot/express/preprocess/TimePreprocessor.py b/plugins/plotly-express/src/deephaven/plot/express/preprocess/TimePreprocessor.py index 19d69d184..b27a3a77f 100644 --- a/plugins/plotly-express/src/deephaven/plot/express/preprocess/TimePreprocessor.py +++ b/plugins/plotly-express/src/deephaven/plot/express/preprocess/TimePreprocessor.py @@ -4,9 +4,10 @@ from ..shared import get_unique_names -from deephaven.time import to_j_instant from deephaven.table import Table +NANOS_PER_MILLI = 1_000_000 + class TimePreprocessor: """ @@ -45,12 +46,13 @@ def preprocess_partitioned_tables( x_diff = get_unique_names(table, ["x_diff"])["x_diff"] for table in tables: + # Times are assumed to be Instants which have nanosecond precision + # We convert them to milliseconds for plotly express yield table.update_view( [ - f"{x_start} = (Instant) to_j_instant({x_start})", - f"{x_end} = (Instant) to_j_instant({x_end})", - f"{x_diff} = ((Instant) to_j_instant({x_end}) - " - f"(Instant) to_j_instant({x_start})) / 1000000", + f"{x_start} = {x_start}", + f"{x_end} = {x_end}", + f"{x_diff} = ({x_end} - {x_start}) / NANOS_PER_MILLI", f"{y}", ] ), {"x_diff": x_diff}