Addressing comments (WIP)

apache · Feb 23, 2022 · ba2ddb0 · ba2ddb0
1 parent eb2d588
commit ba2ddb0
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 26 deletions.
diff --git a/superset/columns/models.py b/superset/columns/models.py
@@ -69,7 +69,7 @@ class Column(
     # Additional metadata describing the column.
     description = sa.Column(sa.Text)
     warning_text = sa.Column(sa.Text)
-    units = sa.Column(sa.Text)
+    unit = sa.Column(sa.Text)
 
     # Is this a time column? Useful for plotting time series.
     is_temporal = sa.Column(sa.Boolean, default=False)
@@ -90,6 +90,6 @@ class Column(
     # additive, so it shouldn't be used in a ``SUM``.
     is_additive = sa.Column(sa.Boolean, default=False)
 
-    # Is an increase good? Useful for displaying the results of A/B tests, or setting up
-    # alerts. Eg, this is true for "revenue", but false for "latency".
-    increase_good = sa.Column(sa.Boolean, default=True)
+    # Is an increase desired? Useful for displaying the results of A/B tests, or setting
+    # up alerts. Eg, this is true for "revenue", but false for "latency".
+    is_increase_desired = sa.Column(sa.Boolean, default=True)
diff --git a/superset/migrations/versions/b8d3a24d9131_new_dataset_models.py b/superset/migrations/versions/b8d3a24d9131_new_dataset_models.py
@@ -57,13 +57,13 @@ def upgrade():
         sa.Column("is_physical", sa.BOOLEAN(), nullable=False, default=True,),
         sa.Column("description", sa.TEXT(), nullable=True),
         sa.Column("warning_text", sa.TEXT(), nullable=True),
-        sa.Column("units", sa.TEXT(), nullable=True),
+        sa.Column("unit", sa.TEXT(), nullable=True),
         sa.Column("is_temporal", sa.BOOLEAN(), nullable=False),
         sa.Column("is_spatial", sa.BOOLEAN(), nullable=False, default=False,),
         sa.Column("is_partition", sa.BOOLEAN(), nullable=False, default=False,),
         sa.Column("is_aggregation", sa.BOOLEAN(), nullable=False, default=False,),
         sa.Column("is_additive", sa.BOOLEAN(), nullable=False, default=False,),
-        sa.Column("increase_good", sa.BOOLEAN(), nullable=False, default=True,),
+        sa.Column("is_increase_desired", sa.BOOLEAN(), nullable=False, default=True,),
         sa.PrimaryKeyConstraint("id"),
     )
     with op.batch_alter_table("sl_columns") as batch_op:

diff --git a/tests/unit_tests/columns/test_models.py b/tests/unit_tests/columns/test_models.py
@@ -44,7 +44,7 @@ def test_column_model(app_context: None, session: Session) -> None:
     # test that default values are set correctly
     assert column.description is None
     assert column.warning_text is None
-    assert column.units is None
+    assert column.unit is None
     assert column.is_temporal is False
     assert column.is_spatial is False
     assert column.is_partition is False

diff --git a/tests/unit_tests/datasets/test_models.py b/tests/unit_tests/datasets/test_models.py
@@ -338,7 +338,7 @@ def test_create_physical_sqlatable(app_context: None, session: Session) -> None:
             "is_temporal": True,
             "name": "ds",
             "type": "TIMESTAMP",
-            "units": None,
+            "unit": None,
             "warning_text": None,
         },
         {
@@ -357,7 +357,7 @@ def test_create_physical_sqlatable(app_context: None, session: Session) -> None:
             "is_temporal": False,
             "name": "user_id",
             "type": "INTEGER",
-            "units": None,
+            "unit": None,
             "warning_text": None,
         },
         {
@@ -376,7 +376,7 @@ def test_create_physical_sqlatable(app_context: None, session: Session) -> None:
             "is_temporal": False,
             "name": "revenue",
             "type": "INTEGER",
-            "units": None,
+            "unit": None,
             "warning_text": None,
         },
         {
@@ -395,7 +395,7 @@ def test_create_physical_sqlatable(app_context: None, session: Session) -> None:
             "is_temporal": False,
             "name": "expenses",
             "type": "INTEGER",
-            "units": None,
+            "unit": None,
             "warning_text": None,
         },
         {
@@ -414,7 +414,7 @@ def test_create_physical_sqlatable(app_context: None, session: Session) -> None:
             "is_temporal": False,
             "name": "profit",
             "type": "INTEGER",
-            "units": None,
+            "unit": None,
             "warning_text": None,
         },
         {
@@ -433,7 +433,7 @@ def test_create_physical_sqlatable(app_context: None, session: Session) -> None:
             "is_temporal": False,
             "name": "cnt",
             "type": "Unknown",
-            "units": None,
+            "unit": None,
             "warning_text": None,
         },
     ]
@@ -587,7 +587,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": None,
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -606,7 +606,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": None,
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -625,7 +625,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": None,
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -644,7 +644,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": None,
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -663,7 +663,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": "ds",
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -682,7 +682,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": "user_id",
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -701,7 +701,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": "revenue",
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -720,7 +720,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": "expenses",
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -739,7 +739,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": "revenue-expenses",
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,
@@ -758,7 +758,7 @@ def test_create_virtual_sqlatable(
             "extra_json": "{}",
             "is_partition": False,
             "expression": "COUNT(*)",
-            "units": None,
+            "unit": None,
             "warning_text": None,
             "created_by": None,
             "increase_good": True,

diff --git a/tests/unit_tests/tables/test_models.py b/tests/unit_tests/tables/test_models.py
@@ -43,11 +43,8 @@ def test_table_model(app_context: None, session: Session) -> None:
 
     assert table.id == 1
     assert table.uuid is not None
-
     assert table.database_id == 1
-
     assert table.catalog == "my_catalog"
     assert table.schema == "my_schema"
     assert table.name == "my_table"
-
     assert [column.name for column in table.columns] == ["ds"]