[GitHub] [superset] john-bodley commented on a diff in pull request #23973: feat: Adds Line chart migration logic

["john-bodley (via GitHub)" <gi...@apache.org>]

john-bodley commented on code in PR #23973:
URL: https://github.com/apache/superset/pull/23973#discussion_r1336133470


##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -125,3 +127,62 @@ def _pre_action(self) -> None:
     def _migrate_temporal_filter(self, rv_data: dict[str, Any]) -> None:
         super()._migrate_temporal_filter(rv_data)
         rv_data["adhoc_filters_b"] = rv_data.get("adhoc_filters") or []
+
+
+class TimeseriesChart(MigrateViz):
+    has_x_axis_control = True
+
+    def _pre_action(self) -> None:
+        self.data["contributionMode"] = "row" if self.data.get("contribution") else None
+
+        show_brush = self.data.get("show_brush")
+        self.data["zoomable"] = False if show_brush == "no" else True

Review Comment:
   You could inline the single use `show_brush` variable if you wanted.



##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -125,3 +127,62 @@ def _pre_action(self) -> None:
     def _migrate_temporal_filter(self, rv_data: dict[str, Any]) -> None:
         super()._migrate_temporal_filter(rv_data)
         rv_data["adhoc_filters_b"] = rv_data.get("adhoc_filters") or []
+
+
+class TimeseriesChart(MigrateViz):
+    has_x_axis_control = True
+
+    def _pre_action(self) -> None:
+        self.data["contributionMode"] = "row" if self.data.get("contribution") else None
+
+        show_brush = self.data.get("show_brush")
+        self.data["zoomable"] = False if show_brush == "no" else True
+
+        self.data["markerEnabled"] = self.data.get("show_markers")

Review Comment:
   What if `show_markers` is undefined? Can `markerEnabled` be a tri-state?



##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -125,3 +127,62 @@ def _pre_action(self) -> None:
     def _migrate_temporal_filter(self, rv_data: dict[str, Any]) -> None:
         super()._migrate_temporal_filter(rv_data)
         rv_data["adhoc_filters_b"] = rv_data.get("adhoc_filters") or []
+
+
+class TimeseriesChart(MigrateViz):
+    has_x_axis_control = True
+
+    def _pre_action(self) -> None:
+        self.data["contributionMode"] = "row" if self.data.get("contribution") else None
+
+        show_brush = self.data.get("show_brush")
+        self.data["zoomable"] = False if show_brush == "no" else True
+
+        self.data["markerEnabled"] = self.data.get("show_markers")
+        self.data["y_axis_showminmax"] = True
+
+        bottom_margin = self.data.get("bottom_margin")
+        if self.data.get("x_axis_label") and (
+            not bottom_margin or bottom_margin == "auto"
+        ):
+            self.data["bottom_margin"] = 30
+
+        if (rolling_type := self.data.get("rolling_type")) and rolling_type != "None":
+            self.data["rolling_type"] = rolling_type
+
+        if time_compare := self.data.get("time_compare"):
+            self.data["time_compare"] = [
+                value + " ago" for value in as_list(time_compare) if value
+            ]
+
+        comparison_type = self.data.get("comparison_type") or "values"
+        self.data["comparison_type"] = (
+            "difference" if comparison_type == "absolute" else comparison_type
+        )
+
+
+class MigrateLineChart(TimeseriesChart):
+    source_viz_type = "line"
+    target_viz_type = "echarts_timeseries_line"
+    rename_keys = {
+        "x_axis_label": "x_axis_title",
+        "bottom_margin": "x_axis_title_margin",
+        "x_axis_format": "x_axis_time_format",
+        "y_axis_label": "y_axis_title",
+        "left_margin": "y_axis_title_margin",
+        "y_axis_showminmax": "truncateYAxis",
+        "y_log_scale": "logAxis",
+    }
+
+    def _pre_action(self) -> None:
+        super()._pre_action()
+
+        line_interpolation = self.data.get("line_interpolation")
+        if line_interpolation == "cardinal":
+            self.target_viz_type = "echarts_timeseries_smooth"
+        elif line_interpolation == "step-before":
+            self.target_viz_type = "echarts_timeseries_step"
+            self.data["seriesType"] = "start"
+        elif line_interpolation == "step-after":
+            self.target_viz_type = "echarts_timeseries_step"
+            self.data["seriesType"] = "end"

Review Comment:
   Should there be a fallback?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org