john-bodley commented on code in PR #23973:
URL: https://github.com/apache/superset/pull/23973#discussion_r1201207193
##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -31,6 +31,39 @@ def _pre_action(self) -> None:
self.data["metric"] = self.data["metrics"][0]
+class TimeseriesChart(MigrateViz):
+ has_x_axis_control = True
+
+ def _pre_action(self) -> None:
+ contribution = self.data.get("contribution")
+ if contribution:
+ self.data["contributionMode"] = "row" if contribution else None
Review Comment:
Something's not quite right here as on line #39 you're already verifying
that `contribution` is truthy.
##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -31,6 +31,39 @@ def _pre_action(self) -> None:
self.data["metric"] = self.data["metrics"][0]
+class TimeseriesChart(MigrateViz):
+ has_x_axis_control = True
+
+ def _pre_action(self) -> None:
+ contribution = self.data.get("contribution")
+ if contribution:
+ self.data["contributionMode"] = "row" if 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
+
+ x_axis_label = self.data.get("x_axis_label")
+ bottom_margin = self.data.get("bottom_margin")
+ if x_axis_label and (not bottom_margin or bottom_margin == "auto"):
+ self.data["bottom_margin"] = 30
+
+ rolling_type = self.data.get("rolling_type")
+ if rolling_type:
+ self.data["rolling_type"] = None if rolling_type == "None" else
rolling_type
+
+ time_compare = self.data.get("time_compare")
+ if time_compare:
Review Comment:
I'm surprised the auto-walrus didn't change this.
##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -31,6 +31,39 @@ def _pre_action(self) -> None:
self.data["metric"] = self.data["metrics"][0]
+class TimeseriesChart(MigrateViz):
+ has_x_axis_control = True
+
+ def _pre_action(self) -> None:
+ contribution = self.data.get("contribution")
+ if contribution:
+ self.data["contributionMode"] = "row" if 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
+
+ x_axis_label = self.data.get("x_axis_label")
+ bottom_margin = self.data.get("bottom_margin")
+ if x_axis_label and (not bottom_margin or bottom_margin == "auto"):
Review Comment:
```suggestion
if self.data.get("x_axis_label") and self.data.get("bottom_margin"):
```
The `bottom_margin` is truthy includes `bottom_margin == "auto"`.
##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -31,6 +31,39 @@ def _pre_action(self) -> None:
self.data["metric"] = self.data["metrics"][0]
+class TimeseriesChart(MigrateViz):
+ has_x_axis_control = True
+
+ def _pre_action(self) -> None:
+ contribution = self.data.get("contribution")
+ if contribution:
+ self.data["contributionMode"] = "row" if 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
+
+ x_axis_label = self.data.get("x_axis_label")
+ bottom_margin = self.data.get("bottom_margin")
+ if x_axis_label and (not bottom_margin or bottom_margin == "auto"):
+ self.data["bottom_margin"] = 30
+
+ rolling_type = self.data.get("rolling_type")
+ if rolling_type:
+ self.data["rolling_type"] = None if rolling_type == "None" else
rolling_type
Review Comment:
I presume that if the value isn't explicitly defined it defaults to `None`
so maybe,
```python
if (rolling_type := self.data.get("rolling_type")) and rolling_type !=
"None":
self.data["rolling_type"] = rolling_type
```
##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -31,6 +31,39 @@ def _pre_action(self) -> None:
self.data["metric"] = self.data["metrics"][0]
+class TimeseriesChart(MigrateViz):
+ has_x_axis_control = True
+
+ def _pre_action(self) -> None:
+ contribution = self.data.get("contribution")
+ if contribution:
+ self.data["contributionMode"] = "row" if contribution else None
+
+ show_brush = self.data.get("show_brush")
+ self.data["zoomable"] = False if show_brush == "no" else True
Review Comment:
```suggestion
self.data["zoomable"] = not (self.data.get("show_brush") == "no")
```
##########
superset/migrations/shared/migrate_viz/processors.py:
##########
@@ -53,3 +86,30 @@ def _pre_action(self) -> None:
if x_axis_label:
self.data["x_axis_title"] = x_axis_label
self.data["x_axis_title_margin"] = 30
+
+
+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")
Review Comment:
Also surprised that the auto-walrus pre-commit hook didn't change this.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
