fixes

enso-org · Apr 21, 2023 · 2bf87d2 · 2bf87d2
1 parent add43a7
commit 2bf87d2
Show file tree

Hide file tree

Showing 12 changed files with 84 additions and 121 deletions.
diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Table.enso
@@ -1354,29 +1354,29 @@ type Table
     join : Table -> Join_Kind -> Join_Condition | Text | Vector (Join_Condition | Text) -> Text -> Problem_Behavior -> Table
     join self right join_kind=Join_Kind.Inner on=[Join_Condition.Equals 0 0] right_prefix="Right_" on_problems=Report_Warning =
         if check_table "right" right then
-                # [left_unmatched, matched, right_unmatched]
-                rows_to_keep = case join_kind of
-                    Join_Kind.Inner           -> [False, True, False]
-                    Join_Kind.Left_Outer      -> [True, True, False]
-                    Join_Kind.Right_Outer     -> [False, True, True]
-                    Join_Kind.Full            -> [True, True, True]
-                    Join_Kind.Left_Exclusive  -> [True, False, False]
-                    Join_Kind.Right_Exclusive -> [False, False, True]
-
-                columns_to_keep = case join_kind of
-                    Join_Kind.Left_Exclusive  -> [True, False]
-                    Join_Kind.Right_Exclusive -> [False, True]
-                    _                         -> [True, True]
-
-                join_resolution = make_join_helpers self right . resolve on on_problems
-                right_columns_to_drop = join_resolution.redundant_column_names
-
-                java_conditions = join_resolution.conditions
-                new_java_table = self.java_table.join right.java_table java_conditions (rows_to_keep.at 0) (rows_to_keep.at 1) (rows_to_keep.at 2) (columns_to_keep.at 0) (columns_to_keep.at 1) right_columns_to_drop right_prefix
-
-                on_problems.attach_problems_after (Table.Value new_java_table) <|
-                    problems = new_java_table.getProblems
-                    Java_Problems.parse_aggregated_problems problems
+            # [left_unmatched, matched, right_unmatched]
+            rows_to_keep = case join_kind of
+                Join_Kind.Inner           -> [False, True, False]
+                Join_Kind.Left_Outer      -> [True, True, False]
+                Join_Kind.Right_Outer     -> [False, True, True]
+                Join_Kind.Full            -> [True, True, True]
+                Join_Kind.Left_Exclusive  -> [True, False, False]
+                Join_Kind.Right_Exclusive -> [False, False, True]
+
+            columns_to_keep = case join_kind of
+                Join_Kind.Left_Exclusive  -> [True, False]
+                Join_Kind.Right_Exclusive -> [False, True]
+                _                         -> [True, True]
+
+            join_resolution = make_join_helpers self right . resolve on on_problems
+            right_columns_to_drop = join_resolution.redundant_column_names
+
+            java_conditions = join_resolution.conditions
+            new_java_table = self.java_table.join right.java_table java_conditions (rows_to_keep.at 0) (rows_to_keep.at 1) (rows_to_keep.at 2) (columns_to_keep.at 0) (columns_to_keep.at 1) right_columns_to_drop right_prefix
+
+            on_problems.attach_problems_after (Table.Value new_java_table) <|
+                problems = new_java_table.getProblems
+                Java_Problems.parse_aggregated_problems problems
 
     ## ALIAS Cartesian Join
        Joins tables by pairing every row of the left table with every row of the

diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Type/Value_Type.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Type/Value_Type.enso
@@ -335,7 +335,10 @@ type Auto
 ## PRIVATE
    A helper for generating the `Value_Type.expect_` checks.
 expect_type : Any -> (Value_Type -> Boolean) -> Text|Value_Type -> Any -> Any ! Invalid_Value_Type
-expect_type value predicate type_kind ~action =
-    typ = Value_Type_Helpers.find_argument_type value
-    if predicate typ then action else
-        Value_Type_Helpers.raise_unexpected_type type_kind value
+expect_type value predicate type_kind ~action = case value of
+    # Special handling for `Nothing`. Likely, can be removed with #6281.
+    Nothing -> action
+    _ ->
+        typ = Value_Type_Helpers.find_argument_type value
+        if predicate typ then action else
+            Value_Type_Helpers.raise_unexpected_type type_kind value
diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Type/Value_Type_Helpers.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Data/Type/Value_Type_Helpers.enso
@@ -96,16 +96,14 @@ find_argument_type value = case value of
    A helper which resolves if numeric addition or string concatenation should be
    used when the a `+` operator is used with the two provided types.
    It will return an error if the provided types are incompatible.
-   The `other_type` may be `Nothing` if the other operand is `Nothing`, then the
-   operation will depend only on the `self_type`.
-resolve_addition_kind self other =
-    self_type = find_argument_type self
-    other_type = find_argument_type other
-    if self_type.is_numeric && (other_type.is_nothing || other_type.is_numeric) then 'ADD_NUMBER' else
-        if self_type.is_text && (other_type.is_nothing || other_type.is_text) then 'ADD_TEXT' else
+resolve_addition_kind arg1 arg2 =
+    type_1 = find_argument_type arg1
+    type_2 = find_argument_type arg2
+    if type_1.is_numeric && (type_2.is_nothing || type_2.is_numeric) then 'ADD_NUMBER' else
+        if type_1.is_text && (type_2.is_nothing || type_2.is_text) then 'ADD_TEXT' else
             Error.throw <| Illegal_Argument.Error <|
-                if other_type.is_nothing then "Cannot perform addition on a value of type " + self_type.to_display_text + ". Addition can only be performed if the column is of some numeric type or is text." else
-                    "Cannot perform addition on a pair of values of types " + self_type.to_display_text + " and " + other_type.to_display_text + ". Addition can only be performed if both columns are of some numeric type or are both are text."
+                if type_2.is_nothing then "Cannot perform addition on a value of type " + type_1.to_display_text + ". Addition can only be performed if the column is of some numeric type or is text." else
+                    "Cannot perform addition on a pair of values of types " + type_1.to_display_text + " and " + type_2.to_display_text + ". Addition can only be performed if both columns are of some numeric type or are both are text."
 
 ## PRIVATE
    Checks that both provided arguments have numeric type and runs the action
@@ -132,11 +130,9 @@ check_binary_boolean_op arg1 arg2 ~action =
    - arg_or_args: a single value or column or a vector of values or columns.
    - action: the action to run if the arguments are compatible.
 check_multi_argument_comparable_op column arg_or_args ~action =
-    column_type = column.value_type
     args = Vector.unify_vector_or_element arg_or_args
-    other_types = args.map find_argument_type
-    checked = other_types.map other_type->
-        Value_Type.expect_comparable column_type other_type <|
+    checked = args.map arg->
+        Value_Type.expect_comparable column arg <|
             True
     checked.if_not_error <|
         action

diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Errors.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Errors.enso
@@ -413,7 +413,7 @@ type Invalid_Value_Type
     to_display_text : Text
     to_display_text self = case self of
         Invalid_Value_Type.Column expected actual related_column ->
-            "Expected type "+expected.to_display_text+", but got a column "+related_column+" of type "+actual.to_display_text+"."
+            "Expected type "+expected.to_display_text+", but got a column ["+related_column+"] of type "+actual.to_display_text+"."
         Invalid_Value_Type.Value expected actual value ->
             "Expected type "+expected.to_display_text+", but got a value "+value.to_text+" of type "+actual.to_display_text+"."
         Invalid_Value_Type.Not_Ordered actual ->

diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Filter_Condition_Helpers.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Filter_Condition_Helpers.enso
@@ -31,8 +31,7 @@ make_filter_column source_column filter_condition on_problems = case filter_cond
     # Boolean
     Is_True ->
         Value_Type.expect_boolean source_column <| source_column
-    Is_False ->
-        Value_Type.expect_boolean source_column <| source_column.not
+    Is_False -> source_column.not
     # Comparisons
     Less value -> (source_column < value)
     Equal_Or_Less value -> (source_column <= value)
@@ -41,52 +40,21 @@ make_filter_column source_column filter_condition on_problems = case filter_cond
     Between lower upper -> source_column.between lower upper
     # Text
     Starts_With prefix case_sensitivity ->
-        Value_Type.expect_text source_column <|
-            expect_column_or_value_as_text "prefix" prefix <|
-                source_column.starts_with prefix case_sensitivity
+        source_column.starts_with prefix case_sensitivity
     Ends_With suffix case_sensitivity ->
-        Value_Type.expect_text source_column <|
-            expect_column_or_value_as_text "suffix" suffix <|
-                source_column.ends_with suffix case_sensitivity
+        source_column.ends_with suffix case_sensitivity
     Contains substring case_sensitivity ->
-        Value_Type.expect_text source_column <|
-            expect_column_or_value_as_text "substring" substring <|
-                source_column.contains substring case_sensitivity
+        source_column.contains substring case_sensitivity
     Not_Contains substring case_sensitivity ->
-        Value_Type.expect_text source_column <|
-            expect_column_or_value_as_text "substring" substring <|
-                source_column.contains substring case_sensitivity . not
+        source_column.contains substring case_sensitivity . not
     Is_Empty ->
-        Value_Type.expect_text source_column <|
-            source_column.is_empty
+        source_column.is_empty
     Not_Empty ->
-        Value_Type.expect_text source_column <|
-            source_column.is_empty.not
+        source_column.is_empty.not
     Like pattern ->
-        Value_Type.expect_text source_column <|
-            expect_column_or_value_as_text "pattern" pattern <|
-                source_column.like pattern
+        source_column.like pattern
     Not_Like pattern ->
-        Value_Type.expect_text source_column <|
-            expect_column_or_value_as_text "pattern" pattern <|
-                source_column.like pattern . not
+        source_column.like pattern . not
     # Vector
     Is_In values -> source_column.is_in values
     Not_In values -> source_column.is_in values . not
-
-## PRIVATE
-expect_column_or_value_as_text field_name column_or_value ~action = case column_or_value of
-    _ : Text -> action
-    ## A bit of a hack, because due to lack of interfaces we cannot check if the
-       thing is a Column (as there are various column implementations based on
-       the backend). So we assume it is a column and if it doesn't quack like a
-       column, we fall back to a type error.
-    maybe_column ->
-        result = Panic.catch No_Such_Method (Value_Type.expect_text maybe_column True) _->
-            Error.throw (Type_Error.Error Text (Meta.type_of maybe_column) field_name)
-        ## We don't run the action above, to avoid catching spurious
-           `No_Such_Method` from the action itself. Instead we just return
-           True there and if it went through successfully we can then execute
-           the action. If it fails, we forward the dataflow error instead.
-        result.if_not_error <|
-            action
diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Join_Helpers.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Join_Helpers.enso
@@ -47,9 +47,10 @@ type Join_Condition_Resolver
             left = resolve_left left_selector
             right = resolve_right right_selector
             if is_nothing left || is_nothing right then Nothing else
-                if left.name == right.name then
-                    redundant_names.append right.name
-                self.make_equals problem_builder left right
+                Value_Type.expect_comparable left right <|
+                    if left.name == right.name then
+                        redundant_names.append right.name
+                    self.make_equals problem_builder left right
         converted = conditions_vector.map condition-> case condition of
             Join_Condition.Equals left_selector right_selector ->
                 handle_equals left_selector right_selector
@@ -66,7 +67,9 @@ type Join_Condition_Resolver
                 right_lower = resolve_right right_lower_selector
                 right_upper = resolve_right right_upper_selector
                 if is_nothing left || is_nothing right_lower || is_nothing right_upper then Nothing else
-                    self.make_between problem_builder left right_lower right_upper
+                    Value_Type.expect_comparable left right_lower <|
+                        Value_Type.expect_comparable left right_upper <|
+                            self.make_between problem_builder left right_lower right_upper
         problem_builder.attach_problems_before on_problems <|
             if converted.contains Nothing then Panic.throw (Illegal_State.Error "Impossible: unresolved columns remaining in the join resolution. This should have raised a dataflow error. This is a bug in the Table library.") else
                 Join_Condition_Resolution.Result converted redundant_names.to_vector

diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Naming_Helpers.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Naming_Helpers.enso
@@ -1,6 +1,8 @@
 from Standard.Base import all
 import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
 
+from project.Internal.Table_Helpers import is_column
+
 polyglot java import org.enso.table.data.table.Column as Java_Column
 
 ## PRIVATE
@@ -43,10 +45,3 @@ type Naming_Helpers
     to_expression_text value =
         if is_column value then "[" + value.name.replace "]" "]]" + "]" else
             value.pretty
-
-## PRIVATE
-   Checks if the value is a column of any backend.
-is_column value =
-    case Meta.get_qualified_type_name value of
-        Nothing -> False
-        typename : Text -> typename.ends_with ".Column"
diff --git a/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Split_Tokenize.enso b/distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Split_Tokenize.enso
@@ -19,7 +19,7 @@ polyglot java import org.enso.table.data.mask.OrderMask
 split_to_columns : Table -> Text | Integer -> Text -> Integer | Nothing -> Problem_Behavior -> Table
 split_to_columns table input_column_id delimiter="," column_count=Nothing on_problems=Report_Error =
     column = table.at input_column_id
-    Value_Type.expect_text (column.value_type) related_column=column <|
+    Value_Type.expect_text column <|
         fan_out_to_columns table input_column_id (handle_nothing (_.split delimiter)) column_count on_problems
 
 ## PRIVATE
@@ -28,7 +28,7 @@ split_to_columns table input_column_id delimiter="," column_count=Nothing on_pro
 split_to_rows : Table -> Text | Integer -> Text -> Table
 split_to_rows table input_column_id delimiter="," =
     column = table.at input_column_id
-    Value_Type.expect_text (column.value_type) related_column=column <|
+    Value_Type.expect_text column
         fan_out_to_rows table input_column_id (handle_nothing (_.split delimiter))
 
 ## PRIVATE
@@ -38,7 +38,7 @@ split_to_rows table input_column_id delimiter="," =
 tokenize_to_columns : Table -> Text | Integer -> Text -> Case_Sensitivity -> Integer | Nothing -> Problem_Behavior -> Table
 tokenize_to_columns table input_column_id pattern case_sensitivity column_count on_problems =
     column = table.at input_column_id
-    Value_Type.expect_text (column.value_type) related_column=column <|
+    Value_Type.expect_text column
         fan_out_to_columns table input_column_id (handle_nothing (_.tokenize pattern case_sensitivity)) column_count on_problems
 
 ## PRIVATE
@@ -48,7 +48,7 @@ tokenize_to_columns table input_column_id pattern case_sensitivity column_count
 tokenize_to_rows : Table -> Text | Integer -> Text -> Case_Sensitivity -> Table
 tokenize_to_rows table input_column_id pattern="." case_sensitivity=Case_Sensitivity.Sensitive =
     column = table.at input_column_id
-    Value_Type.expect_text (column.value_type) related_column=column <|
+    Value_Type.expect_text column
         fan_out_to_rows table input_column_id (handle_nothing (_.tokenize pattern case_sensitivity))
 
 ## PRIVATE

diff --git a/test/Table_Tests/src/Common_Table_Operations/Column_Operations_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Column_Operations_Spec.enso
@@ -578,11 +578,11 @@ spec setup =
 
         Test.specify "should check types" <|
             [(.min), (.max)].each op->
-                op a c . should_fail_with Illegal_Argument
-                op a [1, 2, c] . should_fail_with Illegal_Argument
-                op a [1, Nothing, c, Nothing] . should_fail_with Illegal_Argument
-                op c 1 . should_fail_with Illegal_Argument
-                op a True . should_fail_with Illegal_Argument
+                op a c . should_fail_with Invalid_Value_Type
+                op a [1, 2, c] . should_fail_with Invalid_Value_Type
+                op a [1, Nothing, c, Nothing] . should_fail_with Invalid_Value_Type
+                op c 1 . should_fail_with Invalid_Value_Type
+                op a True . should_fail_with Invalid_Value_Type
 
     Test.group prefix+"Column Operations - Text Replace" <|
         if setup.is_database.not then
@@ -608,9 +608,9 @@ spec setup =
 
             Test.specify "should only allow replace on Text columns" <|
                 c.replace "a" "#" . should_fail_with Invalid_Value_Type
-                a.replace 1 "#" . should_fail_with Illegal_Argument
+                a.replace 1 "#" . should_fail_with Invalid_Value_Type
                 a.replace c "#" . should_fail_with Invalid_Value_Type
-                a.replace "a" 1 . should_fail_with Illegal_Argument
+                a.replace "a" 1 . should_fail_with Invalid_Value_Type
                 a.replace "a" c . should_fail_with Invalid_Value_Type
 
             Test.specify "should not replace if Empty term" <|

diff --git a/test/Table_Tests/src/Common_Table_Operations/Date_Time_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Date_Time_Spec.enso
@@ -96,7 +96,7 @@ spec setup =
             t = table_builder [["A", [1, 2, 3]], ["B", ["a", "b", "c"]], ["C", [True, False, True]]]
             r1 = t.at "A" . year
             r1.should_fail_with Invalid_Value_Type
-            r1.catch . to_display_text . should_start_with "Expected A column to have Date or Date_Time type, but got Integer"
+            r1.catch . to_display_text . should_start_with "Expected type Date or Date_Time, but got a column [A] of type Integer"
             t.at "B" . month . should_fail_with Invalid_Value_Type
             t.at "C" . day . should_fail_with Invalid_Value_Type
 
@@ -128,8 +128,8 @@ spec setup =
             t = table_builder [["X", [Date.new 2021 12 3]], ["Y", [Date_Time.new 2021 12 5 12 30 0]], ["Z", [Time_Of_Day.new 12 30 0]]]
 
             [(<), (<=), (>), (>=)].each op->
-                op (t.at "X") (t.at "Y") . should_fail_with Illegal_Argument
-                op (t.at "X") (t.at "Z") . should_fail_with Illegal_Argument
+                op (t.at "X") (t.at "Y") . should_fail_with Invalid_Value_Type
+                op (t.at "X") (t.at "Z") . should_fail_with Invalid_Value_Type
 
     if setup.test_selection.date_time.not then
         Test.group prefix+"partial Date-Time support" <|