From 52846950125ea75ff0aab4ffaecf366e60750b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 17 Jan 2022 18:24:08 +0100 Subject: [PATCH 01/28] checkpoint --- .../Database/0.2.32-SNAPSHOT/src/Data/Table.enso | 4 ++++ .../0.2.32-SNAPSHOT/src/Data/Column_Selector.enso | 8 ++++++++ .../Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 6 ++++++ .../0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso | 13 +++++++++++++ 4 files changed, 31 insertions(+) create mode 100644 distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso create mode 100644 distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index fe419c054368..4521b63775a6 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -77,6 +77,10 @@ type Table internal = candidates.find (p -> p.name == name) this.make_column internal . map_error (_ -> No_Such_Column_Error name) + select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Table + select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) = + Error.throw "Not implemented" + ## PRIVATE Resolves the column name to a column within this table. diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso new file mode 100644 index 000000000000..1c3f23f9f08c --- /dev/null +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso @@ -0,0 +1,8 @@ +from Standard.Base import all + +from Standard.Table.Data.Matching import all + +type Column_Selector + type By_Name (names : Vector Text) (matching_strategy : Matching_Strategy = Exact True) + type By_Index (indexes : Vector Number) + type By_Column (columns : Vector Column) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index 4b11e1e64352..023ea9a44ef3 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -9,6 +9,8 @@ import Standard.Table.Io.Spreadsheet_Write_Mode import Standard.Table.Io.Format from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule +from Standard.Table.Data.Column_Selector as Column_Selector_Module import all +from Standard.Base.Error.Problem_Behavior as Problem_Behavior_Module import all polyglot java import org.enso.table.data.table.Table as Java_Table polyglot java import org.enso.table.operations.OrderBuilder @@ -232,6 +234,10 @@ type Table Nothing -> Error.throw (No_Such_Column_Error name) c -> Column.Column c + select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Table + select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) = + Error.throw "Not implemented" + ## ALIAS Filter Rows ALIAS Mask Columns diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso new file mode 100644 index 000000000000..e036a1dfb741 --- /dev/null +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -0,0 +1,13 @@ +from Standard.Base import all + +## PRIVATE + A helper function encapsulating shared code for `select_columns` + implementations of various Table variants. See the documentation for the + Table type for details. + + It takes a list of columns and returns the selected columns. It is the + responsibility of each implementation to reconstruct a proper table from the + resulting list of columns. +select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> Vector +select_columns internal_columns selector reorder on_problem = + Error.throw "TODO" From c3e5fe20ea48ac35dd39c7574be40b9501133843 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Wed, 19 Jan 2022 15:35:37 +0100 Subject: [PATCH 02/28] checkpoint --- .../Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 1 + .../Table/0.2.32-SNAPSHOT/src/Error.enso | 25 ++++++++++- .../src/Error/Column_Missing.enso | 10 ----- .../src/Internal/Table_Helpers.enso | 43 +++++++++++++++++-- 4 files changed, 64 insertions(+), 15 deletions(-) delete mode 100644 distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error/Column_Missing.enso diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index 023ea9a44ef3..40d6dd82bfa5 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -7,6 +7,7 @@ import Standard.Visualization import Standard.Base.Data.Time.Date import Standard.Table.Io.Spreadsheet_Write_Mode import Standard.Table.Io.Format +import Standard.Table.Internal.Table_Helpers from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule from Standard.Table.Data.Column_Selector as Column_Selector_Module import all diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso index 8a1070176ec6..c5e32b5e8047 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso @@ -1,3 +1,24 @@ -import Standard.Table.Error.Column_Missing +## One or more columns not found in the input table. + Can occur when using By_Name or By_Column. +type Missing_Input_Columns (criteria : [Text]) -from Standard.Table.Error.Column_Missing export all +Missing_Input_Columns.to_display_text : Text +Missing_Input_Columns.to_display_text = + "The criteria "+this.criteria.to_display_text+" did not match any columns." + +## One or more column indexes were invalid on the input table. + Can occur when using By_Index. +type Column_Indexes_Out_Of_Range (indexes : [Number]) + +## More names than the column count provided to the function. + Can occur when using By_Position. +type Too_Many_Column_Names_Provided (column_names : [Text]) + +## One or more column names were invalid during a rename operation. +type Invalid_Output_Column_Names (column_names : [Text]) + +## One or more column names clashed during a rename operation. +type Duplicate_Output_Column_Names (column_names : [Text]) + +## No columns in the output result. +type No_Output_Columns diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error/Column_Missing.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error/Column_Missing.enso deleted file mode 100644 index f6f6337bff49..000000000000 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error/Column_Missing.enso +++ /dev/null @@ -1,10 +0,0 @@ -from Standard.Base import all - -## An error indicating that no column matching the provided criterion has - been found in the input. -type Column_Missing (criterion : Text) - - -Column_Missing.to_display_text : Text -Column_Missing.to_display_text = - "The criterion ["+this.criterion+"] did not match any columns." diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index e036a1dfb741..5f078db068d8 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -1,5 +1,9 @@ from Standard.Base import all +import Standard.Table.Data.Matching +from Standard.Table.Data.Column_Selector as Column_Selector_Module import all +from Standard.Table.Error as Error_Module import all + ## PRIVATE A helper function encapsulating shared code for `select_columns` implementations of various Table variants. See the documentation for the @@ -8,6 +12,39 @@ from Standard.Base import all It takes a list of columns and returns the selected columns. It is the responsibility of each implementation to reconstruct a proper table from the resulting list of columns. -select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> Vector -select_columns internal_columns selector reorder on_problem = - Error.throw "TODO" + + Arguments: + - internal_columns: A list of all columns in a table. + - selector: Column selection criteria. + - reorder: Specifies whether to reorder the matched columns according to the + order of the selection criteria. + If `False`, the matched entries are returned in the same order as in the + input. + If `True`, the matched entries are returned in the order of the criteria + matching them. If a single object has been matched by multiple criteria, it + is placed in the group belonging to the first matching criterion on the + list. If a single criterion's group has more than one element, their + relative order is the same as in the input. + - on_problems: Specifies the behavior when a problem occurs during the + operation. By default, a warning is issued, but the operation proceeds. + If set to `Report_Error`, the operation fails with a dataflow error. + If set to `Ignore`, the operation proceeds without errors or warnings. + - warning_callback: A callback specifying the function to call when issuing a + warning. This is a temporary workaround to allow for testing the warning + mechanism. Once the proper warning system is implemented, this argument + will become obsolete and will be removed. No user code should use this + argument, as it will be removed in the future. +select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> (Any -> Nothing) -> Vector +select_columns internal_columns selector reorder on_problems warning_callback = + result = case selector of + By_Name names matching_strategy -> + Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warning_callback=warning_callback + By_Index indices -> case reorder of + True -> + Error.throw "TODO" + False -> + Error.throw "TODO" + By_Column columns -> + names = columns.map .name + Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warning_callback=warning_callback + # TODO check if empty and possibly report warning/error From a3d624f7af3adf49fb38426497d81f521ce47081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 20 Jan 2022 14:21:32 +0100 Subject: [PATCH 03/28] Utility for mapping errors and warnings --- .../0.2.32-SNAPSHOT/src/Error/Warnings.enso | 20 ++++++++++++ .../Table/0.2.32-SNAPSHOT/src/Error.enso | 11 ++++++- .../src/Internal/Table_Helpers.enso | 32 ++++++++++++------- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso index 86e0d0c1da18..d3485f691c7e 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso @@ -20,7 +20,27 @@ type Warning_System this.warning_callback warning_payload decorated_value +## PRIVATE + The default implementation of a warning system mock. + To be removed once warnings are implemented. default : Warning_System default = Warning_System warning-> IO.println "[WARNING] "+warning.to_display_text + +## UNSTABLE + Maps warnings attached to a value. + + Currently it is not implemented as the warning system is missing. It just + returns the original value without changes. +map_attached_warnings : (Any -> Any) -> Any -> Any +map_attached_warnings mapper decorated_value = + _ = mapper + decorated_value + +## UNSTABLE + An utility function which applies the mapping function both to any attached + warnings and dataflow errors. +map_warnings_and_errors : (Any -> Any) -> Any -> Any +map_warnings_and_errors mapper decorated_value = + (here.map_attached_warnings mapper decorated_value) . map_error mapper diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso index c5e32b5e8047..f8b981d1c8f5 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso @@ -4,12 +4,17 @@ type Missing_Input_Columns (criteria : [Text]) Missing_Input_Columns.to_display_text : Text Missing_Input_Columns.to_display_text = - "The criteria "+this.criteria.to_display_text+" did not match any columns." + "The criteria "+this.criteria.to_text+" did not match any columns." ## One or more column indexes were invalid on the input table. Can occur when using By_Index. type Column_Indexes_Out_Of_Range (indexes : [Number]) +Column_Indexes_Out_Of_Range.to_display_text : Text +Column_Indexes_Out_Of_Range.to_display_text = case this.indexes.length of + 1 -> "The index " + (this.indexes.at 0).to_text + " is out of range." + _ -> "The indexes "+this.indexes.short_display_text+" are out of range." + ## More names than the column count provided to the function. Can occur when using By_Position. type Too_Many_Column_Names_Provided (column_names : [Text]) @@ -22,3 +27,7 @@ type Duplicate_Output_Column_Names (column_names : [Text]) ## No columns in the output result. type No_Output_Columns + +No_Output_Columns.to_display_text : Text +No_Output_Columns.to_display_text = + "The result contains no columns." diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 5f078db068d8..7968f62d562a 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -1,6 +1,8 @@ from Standard.Base import all +import Standard.Base.Error.Warnings import Standard.Table.Data.Matching +from Standard.Base.Error.Problem_Behavior as Problem_Behavior_Module import all from Standard.Table.Data.Column_Selector as Column_Selector_Module import all from Standard.Table.Error as Error_Module import all @@ -29,22 +31,30 @@ from Standard.Table.Error as Error_Module import all operation. By default, a warning is issued, but the operation proceeds. If set to `Report_Error`, the operation fails with a dataflow error. If set to `Ignore`, the operation proceeds without errors or warnings. - - warning_callback: A callback specifying the function to call when issuing a - warning. This is a temporary workaround to allow for testing the warning - mechanism. Once the proper warning system is implemented, this argument - will become obsolete and will be removed. No user code should use this - argument, as it will be removed in the future. -select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> (Any -> Nothing) -> Vector -select_columns internal_columns selector reorder on_problems warning_callback = + - warnings: A Warning_System instance specifying how to handle warnings. This + is a temporary workaround to allow for testing the warning mechanism. Once + the proper warning system is implemented, this argument will become + obsolete and will be removed. No user code should use this argument, as it + will be removed in the future. +select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Vector +select_columns internal_columns selector reorder on_problems warnings = + promote_no_matches_to_missing_columns error = case error of + Matching.No_Matches_Found criteria -> Missing_Input_Columns criteria + _ -> error + result = case selector of By_Name names matching_strategy -> - Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warning_callback=warning_callback - By_Index indices -> case reorder of + result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result + By_Index _ -> case reorder of True -> Error.throw "TODO" False -> Error.throw "TODO" By_Column columns -> names = columns.map .name - Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warning_callback=warning_callback - # TODO check if empty and possibly report warning/error + result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result + + issues = if result.is_empty then [] else [No_Output_Columns] + Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings From aabe9a858f00553b56a73490e8453c943a5e2e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 21 Jan 2022 12:09:56 +0100 Subject: [PATCH 04/28] checkpoint --- .../src/Internal/Table_Helpers.enso | 18 +++++++++++++++--- test/Table_Tests/src/Matching_Spec.enso | 2 +- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 7968f62d562a..fb87d2412c10 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -46,11 +46,14 @@ select_columns internal_columns selector reorder on_problems warnings = By_Name names matching_strategy -> result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result - By_Index _ -> case reorder of + By_Index indices -> + result = case reorder of True -> - Error.throw "TODO" + here.select_indices_reordering internal_columns indices False -> - Error.throw "TODO" + here.select_indices_preserving_order internal_columns indices + # TODO warn if there are any non-unique columns in result? + result.distinct on=_.name By_Column columns -> names = columns.map .name result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings @@ -58,3 +61,12 @@ select_columns internal_columns selector reorder on_problems warnings = issues = if result.is_empty then [] else [No_Output_Columns] Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings + +## PRIVATE +select_indices_reordering vector indices = + indices.map vector.at + +## PRIVATE +select_indices_preserving_order vector indices = + indexed_indices = Map.from_vector (indices.map i-> [i, True]) + vector.map_with_index diff --git a/test/Table_Tests/src/Matching_Spec.enso b/test/Table_Tests/src/Matching_Spec.enso index 776f8bc437bb..eb2f0a792f54 100644 --- a/test/Table_Tests/src/Matching_Spec.enso +++ b/test/Table_Tests/src/Matching_Spec.enso @@ -79,7 +79,7 @@ spec = Test.group 'Matching Helper' <| Matching.match_criteria (Error.throw Foo_Error) [] . should_fail_with Foo_Error Matching.match_criteria [] (Error.throw Foo_Error) . should_fail_with Foo_Error Matching.match_criteria [] [] (Error.throw Foo_Error) . should_fail_with Foo_Error - Matching.match_criteria ["a"] ["a"] name_mapper=(_-> Error.throw Foo_Error) . should_fail_with Foo_Error + Matching.match_criteria [1, 2, 3] ["2"] name_mapper=(x-> if x == 3 then Error.throw Foo_Error else x.to_text) . should_fail_with Foo_Error Matching.match_criteria ["a"] ["a"] name_mapper=_.nonexistent_function . should_fail_with No_Such_Method_Error main = Test.Suite.run_main here.spec From 4149aad354dd51aeef0ed18277b4782d74fb4fe2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Sat, 22 Jan 2022 16:05:37 +0100 Subject: [PATCH 05/28] checkpoint --- .../Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index fb87d2412c10..616c8e284051 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -63,10 +63,16 @@ select_columns internal_columns selector reorder on_problems warnings = Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings ## PRIVATE + Selects element from the vector based on the given indices. + + The elements are returned in the same order as their provided indices. select_indices_reordering vector indices = indices.map vector.at ## PRIVATE + Selects element from the vector based on the given indices. + + The elements are returned in the same order as they appeared in the original vector. select_indices_preserving_order vector indices = - indexed_indices = Map.from_vector (indices.map i-> [i, True]) + indices_to_keep = Map.from_vector (indices.map i-> [i, True]) vector.map_with_index From cb48b8d80e12571da84c07725123af138309c60f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 24 Jan 2022 10:53:55 +0100 Subject: [PATCH 06/28] checkpoint --- .../0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 616c8e284051..a950a30f6673 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -47,6 +47,7 @@ select_columns internal_columns selector reorder on_problems warnings = result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result By_Index indices -> + # TODO partition indices into valid and invalid ones, so that we can report a warning or error result = case reorder of True -> here.select_indices_reordering internal_columns indices @@ -75,4 +76,11 @@ select_indices_reordering vector indices = The elements are returned in the same order as they appeared in the original vector. select_indices_preserving_order vector indices = indices_to_keep = Map.from_vector (indices.map i-> [i, True]) - vector.map_with_index + vector.filter_with_index ix-> elem-> + indices_to_keep.get_or_else ix False + +## PRIVATE + Checks if the given index is in the valid range for the provided vector. +is_index_valid vector ix = + actual_ix = if ix < 0 then vector.length+ix else ix + ix>=0 && ix Date: Tue, 25 Jan 2022 18:10:41 +0100 Subject: [PATCH 07/28] Imlpement By_Index --- .../Standard/Table/0.2.32-SNAPSHOT/src/Error.enso | 2 +- .../0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso index f8b981d1c8f5..64266a0b12b0 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso @@ -8,7 +8,7 @@ Missing_Input_Columns.to_display_text = ## One or more column indexes were invalid on the input table. Can occur when using By_Index. -type Column_Indexes_Out_Of_Range (indexes : [Number]) +type Column_Indexes_Out_Of_Range (indexes : [Integer]) Column_Indexes_Out_Of_Range.to_display_text : Text Column_Indexes_Out_Of_Range.to_display_text = case this.indexes.length of diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index a950a30f6673..87e223536b27 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -47,20 +47,24 @@ select_columns internal_columns selector reorder on_problems warnings = result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result By_Index indices -> - # TODO partition indices into valid and invalid ones, so that we can report a warning or error + partitioned_indices = indices.partition (is_index_valid internal_columns) + good_indices = partitioned_indices.first + bad_indices = partitioned_indices.second + issues = if bad_indices.is_empty then [] else + [Column_Indexes_Out_Of_Range bad_indices] result = case reorder of True -> here.select_indices_reordering internal_columns indices False -> here.select_indices_preserving_order internal_columns indices - # TODO warn if there are any non-unique columns in result? - result.distinct on=_.name + # TODO warn if there are any non-unique columns in result? TODO test cases to illustrate + Problem_Behavior_Module.attach_issues_as_needed (result.distinct on=_.name) on_problems issues warnings=warnings By_Column columns -> names = columns.map .name result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result - issues = if result.is_empty then [] else [No_Output_Columns] + issues = if result.is_empty then [No_Output_Columns] else [] Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings ## PRIVATE From 2ea90a8a1acba5b3606d81638e0225ef44017a87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Tue, 25 Jan 2022 18:29:14 +0100 Subject: [PATCH 08/28] Expose select_columns in InMem and DB. Need testing --- .../0.2.32-SNAPSHOT/src/Data/Table.enso | 29 ++++++++++++++++-- .../Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 30 +++++++++++++++++-- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index 4521b63775a6..cc8d07678e5a 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -77,9 +77,32 @@ type Table internal = candidates.find (p -> p.name == name) this.make_column internal . map_error (_ -> No_Such_Column_Error name) - select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Table - select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) = - Error.throw "Not implemented" + ## Returns a new table with a chosen subset of columns, as specified by the + `columns`, from the input table. Any unmatched input columns will be + dropped from the output. + + Arguments: + - columns: Column selection criteria. + - reorder: By default, or if set to `False`, columns in the output will + be in the same order as in the input table. If `True`, the order in the + output table will match the order in the columns list. + - on_problems: Specifies how to handle problems if they occur, reporting + them as warnings by default. + + The following problems can occur: + - If a column in columns is not in the input table, a + `Missing_Input_Columns`. + - If a column index is out of range, a `Column_Indexes_Out_Of_Range`. + - If there are no columns in the output table, a `No_Output_Columns`. + - warnings: A `Warning_System` instance specifying how to handle + warnings. This is a temporary workaround to allow for testing the + warning mechanism. Once the proper warning system is implemented, this + argument will become obsolete and will be removed. No user code should + use this argument, as it will be removed in the future. + select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Table + select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) (warnings = Warnings.default) = + new_columns = Table_Helpers.select_columns internal_columns=this.internal_columns selector=columns reorder=reorder on_problems=on_problems warnings=warnings + this.updated_columns new_columns ## PRIVATE diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index 40d6dd82bfa5..9469d4cc2089 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -12,6 +12,7 @@ import Standard.Table.Internal.Table_Helpers from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule from Standard.Table.Data.Column_Selector as Column_Selector_Module import all from Standard.Base.Error.Problem_Behavior as Problem_Behavior_Module import all +import Standard.Base.Error.Warnings polyglot java import org.enso.table.data.table.Table as Java_Table polyglot java import org.enso.table.operations.OrderBuilder @@ -235,9 +236,32 @@ type Table Nothing -> Error.throw (No_Such_Column_Error name) c -> Column.Column c - select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Table - select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) = - Error.throw "Not implemented" + ## Returns a new table with a chosen subset of columns, as specified by the + `columns`, from the input table. Any unmatched input columns will be + dropped from the output. + + Arguments: + - columns: Column selection criteria. + - reorder: By default, or if set to `False`, columns in the output will + be in the same order as in the input table. If `True`, the order in the + output table will match the order in the columns list. + - on_problems: Specifies how to handle problems if they occur, reporting + them as warnings by default. + + The following problems can occur: + - If a column in columns is not in the input table, a + `Missing_Input_Columns`. + - If a column index is out of range, a `Column_Indexes_Out_Of_Range`. + - If there are no columns in the output table, a `No_Output_Columns`. + - warnings: A `Warning_System` instance specifying how to handle + warnings. This is a temporary workaround to allow for testing the + warning mechanism. Once the proper warning system is implemented, this + argument will become obsolete and will be removed. No user code should + use this argument, as it will be removed in the future. + select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Table + select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) (warnings = Warnings.default) = + new_columns = Table_Helpers.select_columns internal_columns=this.columns selector=columns reorder=reorder on_problems=on_problems warnings=warnings + here.new new_columns ## ALIAS Filter Rows ALIAS Mask Columns From 7c1c05c4c4acb412c27be078650111f86906287e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 27 Jan 2022 12:06:02 +0100 Subject: [PATCH 09/28] checkpoint: writing tests --- .../0.2.32-SNAPSHOT/src/Data/Table.enso | 22 +++++++ .../src/Data/Column_Selector.enso | 21 +++++++ .../Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 22 +++++++ test/Table_Tests/src/Common_Table_Spec.enso | 61 +++++++++++++++++++ test/Table_Tests/src/Table_Spec.enso | 7 +++ 5 files changed, 133 insertions(+) create mode 100644 test/Table_Tests/src/Common_Table_Spec.enso diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index cc8d07678e5a..1081057494b8 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -99,6 +99,28 @@ type Table warning mechanism. Once the proper warning system is implemented, this argument will become obsolete and will be removed. No user code should use this argument, as it will be removed in the future. + + > Example + Select columns by name. + + table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) + + # TODO [RW] default arguments do not work on atoms, once this is fixed, the above should be replaced with just `Matching.Exact` + + > Example + Select columns matching a regular expression. + + table.select_columns (By_Name ["b.*"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) + + > Example + Select the first two columns and the last column, moving the last one to front. + + table.select_columns (By_Index [-1, 0, 1]) reorder=True + + > Example + Select columns with the same names as the ones provided. + + table.select_columns (By_Column [column1, column2]) select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Table select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) (warnings = Warnings.default) = new_columns = Table_Helpers.select_columns internal_columns=this.internal_columns selector=columns reorder=reorder on_problems=on_problems warnings=warnings diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso index 1c3f23f9f08c..3936d0b7affa 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso @@ -2,7 +2,28 @@ from Standard.Base import all from Standard.Table.Data.Matching import all +## Specifies a selection of columns from the table on which an operation is + going to be performed. type Column_Selector + + ## Selects columns based on their names. + + The `matching_strategy` can be used to specify if the names should be + matched exactly or should be treated as regular expressions. It also + allows to specify if the matching should be case-sensitive. type By_Name (names : Vector Text) (matching_strategy : Matching_Strategy = Exact True) + + ## Selects columns by their index. + + The index of the first column in the table is 0. If the provided index is + negative, it counts from the end of the table (e.g. -1 refers to the last + column in the table). type By_Index (indexes : Vector Number) + + ## Selects columns having exactly the same names as the columns provided in + the input. + + The input columns do not necessarily have to come from the same table, so + this approach can be used to match columns with the same names as a set + of columns of some other table, for example, when preparing for a join. type By_Column (columns : Vector Column) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index 9469d4cc2089..2599b7894f43 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -258,6 +258,28 @@ type Table warning mechanism. Once the proper warning system is implemented, this argument will become obsolete and will be removed. No user code should use this argument, as it will be removed in the future. + + > Example + Select columns by name. + + table.select_columns (By_Name ["foo", "bar"] (Matching.Exact True)) + + # TODO [RW] default arguments do not work on atoms, once this is fixed, the above should be replaced with just `Matching.Exact` + + > Example + Select columns matching a regular expression. + + table.select_columns (By_Name ["foo", "bar"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) + + > Example + Select the first two columns and the last column, moving the last one to front. + + table.select_columns (By_Index [-1, 0, 1]) reorder=True + + > Example + Select columns with the same names as the ones provided. + + table.select_columns (By_Column [column1, column2]) select_columns : Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Table select_columns (columns = By_Index [0]) (reorder = False) (on_problems = Report_Warning) (warnings = Warnings.default) = new_columns = Table_Helpers.select_columns internal_columns=this.columns selector=columns reorder=reorder on_problems=on_problems warnings=warnings diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso new file mode 100644 index 000000000000..868340a0f2eb --- /dev/null +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -0,0 +1,61 @@ +from Standard.Base import all +import Standard.Test + +from Standard.Table.Data.Column_Selector as Column_Selector_Module import all + +## TODO + + TODO [RW] the Any in return type of the builder should ideally be replaced with the Table interface, once that is supported. +spec : Text -> (Vector -> Any) -> Nothing +spec prefix table_builder = + Test.group prefix+" Select" <| + table = + col1 = ["foo", Integer, [1,2,3]] + col2 = ["bar", Integer, [4,5,6]] + col3 = ["Baz", Integer, [7,8,9]] + col4 = ["foo_1", Integer, [10,11,12]] + col5 = ["foo_2", Integer, [13,14,15]] + table_builder [col1, col2, col3, col4, col5] + + expect_column_names names table = + table.columns . map .name . should_equal names + + Test.specify "should work as shown in the doc examples" <| + ## TODO thinking if its enough if we just look at names or do we + want all these tests to also verify that contents got moved + correctly (so that not only names were swapped)? Or maybe it is + enough if just a few of these tests verify this additional check? + expect_column_names ["foo", "bar"] <| table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) + expect_column_names ["bar", "Baz"] <| table.select_columns (By_Name ["b.*"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) + expect_column_names ["foo_2", "foo", "bar"] <| table.select_columns (By_Index [-1, 0, 1]) reorder=True + + column1 = table.at "foo_1" + column2 = table.at "Baz" + expect_column_names ["Baz", "foo_1"] <| table.select_columns (By_Column [column1, column2]) + + Test.specify "should support by name selection" <| + Nothing + + Test.specify "should support regex selection" <| + Nothing + + Test.specify "should support by index selection" <| + Nothing + + Test.specify "should support by column selection" <| + Nothing + + Test.specify "should allow to reorder columns if asked to" <| + Nothing + + Test.specify "should correctly handle problems: out of bounds indices" <| + Nothing + + Test.specify "should correctly handle problems: unmatched names" <| + Nothing + + Test.specify "should correctly handle problems: unmatched columns" <| + Nothing + + Test.specify "should correctly handle problems: no columns in the output" <| + Nothing diff --git a/test/Table_Tests/src/Table_Spec.enso b/test/Table_Tests/src/Table_Spec.enso index 01b85c007ab2..fbcaa72ebb09 100644 --- a/test/Table_Tests/src/Table_Spec.enso +++ b/test/Table_Tests/src/Table_Spec.enso @@ -7,6 +7,8 @@ import Standard.Table.Data.Storage import Standard.Test import Standard.Visualization +import project.Common_Table_Spec + type My x y My.== that = case that of @@ -629,3 +631,8 @@ spec = c_3_3 = ['name', ["foo", "bar", "baz"]] t_3 = Table.new [c_3_1, c_3_2, c_3_3] t_3.default_visualization.should_equal Visualization.Id.table + + table_builder columns = + Table.new columns.map description-> [description.at 0, description.at 2] + + Common_Table_Spec.spec "" table_builder From b15abf81c1cec6dbabf80d52bf3fdefb3701a005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 27 Jan 2022 17:01:15 +0100 Subject: [PATCH 10/28] WIP adding tests --- .../0.2.32-SNAPSHOT/src/Error/Warnings.enso | 15 +++++ .../0.2.32-SNAPSHOT/src/Data/Table.enso | 2 +- .../Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 6 +- .../Table/0.2.32-SNAPSHOT/src/Error.enso | 8 ++- .../src/Internal/Table_Helpers.enso | 10 +-- test/Table_Tests/src/Common_Table_Spec.enso | 61 ++++++++++++------- test/Table_Tests/src/Table_Spec.enso | 4 +- 7 files changed, 72 insertions(+), 34 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso index d3485f691c7e..b2b2dd8e4efb 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso @@ -44,3 +44,18 @@ map_attached_warnings mapper decorated_value = map_warnings_and_errors : (Any -> Any) -> Any -> Any map_warnings_and_errors mapper decorated_value = (here.map_attached_warnings mapper decorated_value) . map_error mapper + +## PRIVATE + A temporary helper for testing warnings. + + Gets a closure which takes a `Warning_System` instance and runs some action. + It returns a `Pair` containing the result of the closure and a list of + warnings reported when running it. +test_warnings : (Warning_System -> Any) -> Pair Any (Vector Any) +test_warnings closure = + builder = Vector.new_builder + warning_system = Warning_System warning-> + builder.append warning + result = closure warning_system + warnings = builder.to_vector + Pair result warnings diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index 1081057494b8..8f5021a08b4f 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -110,7 +110,7 @@ type Table > Example Select columns matching a regular expression. - table.select_columns (By_Name ["b.*"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) + table.select_columns (By_Name ["foo.+", "b.*"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) > Example Select the first two columns and the last column, moving the last one to front. diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index 2599b7894f43..b6c7edd2016e 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -262,14 +262,14 @@ type Table > Example Select columns by name. - table.select_columns (By_Name ["foo", "bar"] (Matching.Exact True)) + table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) - # TODO [RW] default arguments do not work on atoms, once this is fixed, the above should be replaced with just `Matching.Exact` + # TODO [RW] default arguments do not work on atoms, once this is fixed, the above `Matching.Exact` can be removed as it is defaulted > Example Select columns matching a regular expression. - table.select_columns (By_Name ["foo", "bar"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) + table.select_columns (By_Name ["foo.+", "b.*"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) > Example Select the first two columns and the last column, moving the last one to front. diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso index 64266a0b12b0..1509eb13dffe 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso @@ -1,3 +1,5 @@ +from Standard.Base import all + ## One or more columns not found in the input table. Can occur when using By_Name or By_Column. type Missing_Input_Columns (criteria : [Text]) @@ -11,9 +13,9 @@ Missing_Input_Columns.to_display_text = type Column_Indexes_Out_Of_Range (indexes : [Integer]) Column_Indexes_Out_Of_Range.to_display_text : Text -Column_Indexes_Out_Of_Range.to_display_text = case this.indexes.length of - 1 -> "The index " + (this.indexes.at 0).to_text + " is out of range." - _ -> "The indexes "+this.indexes.short_display_text+" are out of range." +Column_Indexes_Out_Of_Range.to_display_text = case this.indexes.length == 1 of + True -> "The index " + (this.indexes.at 0).to_text + " is out of range." + False -> "The indexes "+this.indexes.short_display_text+" are out of range." ## More names than the column count provided to the function. Can occur when using By_Position. diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 87e223536b27..86d2fe7ef2be 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -47,16 +47,16 @@ select_columns internal_columns selector reorder on_problems warnings = result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result By_Index indices -> - partitioned_indices = indices.partition (is_index_valid internal_columns) + partitioned_indices = indices.partition (here.is_index_valid internal_columns) good_indices = partitioned_indices.first bad_indices = partitioned_indices.second issues = if bad_indices.is_empty then [] else [Column_Indexes_Out_Of_Range bad_indices] result = case reorder of True -> - here.select_indices_reordering internal_columns indices + here.select_indices_reordering internal_columns good_indices False -> - here.select_indices_preserving_order internal_columns indices + here.select_indices_preserving_order internal_columns good_indices # TODO warn if there are any non-unique columns in result? TODO test cases to illustrate Problem_Behavior_Module.attach_issues_as_needed (result.distinct on=_.name) on_problems issues warnings=warnings By_Column columns -> @@ -80,11 +80,11 @@ select_indices_reordering vector indices = The elements are returned in the same order as they appeared in the original vector. select_indices_preserving_order vector indices = indices_to_keep = Map.from_vector (indices.map i-> [i, True]) - vector.filter_with_index ix-> elem-> + vector.filter_with_index ix-> _-> indices_to_keep.get_or_else ix False ## PRIVATE Checks if the given index is in the valid range for the provided vector. is_index_valid vector ix = actual_ix = if ix < 0 then vector.length+ix else ix - ix>=0 && ix=0 && actual_ix (Vector -> Any) -> Nothing @@ -12,7 +27,7 @@ spec prefix table_builder = table = col1 = ["foo", Integer, [1,2,3]] col2 = ["bar", Integer, [4,5,6]] - col3 = ["Baz", Integer, [7,8,9]] + col3 = ["Bar", Integer, [7,8,9]] col4 = ["foo_1", Integer, [10,11,12]] col5 = ["foo_2", Integer, [13,14,15]] table_builder [col1, col2, col3, col4, col5] @@ -21,35 +36,39 @@ spec prefix table_builder = table.columns . map .name . should_equal names Test.specify "should work as shown in the doc examples" <| - ## TODO thinking if its enough if we just look at names or do we - want all these tests to also verify that contents got moved - correctly (so that not only names were swapped)? Or maybe it is - enough if just a few of these tests verify this additional check? expect_column_names ["foo", "bar"] <| table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) - expect_column_names ["bar", "Baz"] <| table.select_columns (By_Name ["b.*"] (Matching.Regex case_senitivity=Matching.Case_Insensitive)) + expect_column_names ["bar", "Bar", "foo_1", "foo_2"] <| table.select_columns (By_Name ["foo.+", "b.*"] (Matching.Regex Matching.Case_Insensitive)) expect_column_names ["foo_2", "foo", "bar"] <| table.select_columns (By_Index [-1, 0, 1]) reorder=True column1 = table.at "foo_1" - column2 = table.at "Baz" - expect_column_names ["Baz", "foo_1"] <| table.select_columns (By_Column [column1, column2]) + column2 = table.at "Bar" + expect_column_names ["Bar", "foo_1"] <| table.select_columns (By_Column [column1, column2]) - Test.specify "should support by name selection" <| - Nothing + Test.specify "should allow to reorder columns if asked to" <| + table_2 = table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) reorder=True + expect_column_names ["bar", "foo"] table_2 + table_2 . at "bar" . to_vector . should_equal [4,5,6] + table_2 . at "foo" . to_vector . should_equal [1,2,3] - Test.specify "should support regex selection" <| - Nothing + Test.specify "should correctly handle exact matches matching multiple names due to case insensitivity" <| + expect_column_names ["bar", "Bar"] <| table.select_columns (By_Name ["bar"] (Matching.Exact Matching.Case_Insensitive)) - Test.specify "should support by index selection" <| - Nothing + Test.specify "should correctly handle regexes matching multiple names" <| + expect_column_names ["foo", "bar", "foo_1", "foo_2"] <| table.select_columns (By_Name ["b.*", "f.+"] (Matching.Regex True)) + expect_column_names ["bar", "foo", "foo_1", "foo_2"] <| table.select_columns (By_Name ["b.*", "f.+"] (Matching.Regex True)) reorder=True - Test.specify "should support by column selection" <| - Nothing + Test.specify "should correctly handle problems: out of bounds indices" <| + selector = By_Index [1, 0, 100, 200, 300] + expect_column_names ["foo", "bar"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - Test.specify "should allow to reorder columns if asked to" <| - Nothing + err = table.select_columns selector on_problems=Problem_Behavior.Report_Error + err . should_fail_with Column_Indexes_Out_Of_Range + err.catch . should_equal <| Column_Indexes_Out_Of_Range [100, 200, 300] - Test.specify "should correctly handle problems: out of bounds indices" <| - Nothing + case Warnings.test_warnings (table.select_columns selector warnings=_) of + Pair result warnings -> + expect_column_names ["foo", "bar"] result + warnings.should_equal [Column_Indexes_Out_Of_Range [100, 200, 300]] Test.specify "should correctly handle problems: unmatched names" <| Nothing diff --git a/test/Table_Tests/src/Table_Spec.enso b/test/Table_Tests/src/Table_Spec.enso index fbcaa72ebb09..a5561a39dc49 100644 --- a/test/Table_Tests/src/Table_Spec.enso +++ b/test/Table_Tests/src/Table_Spec.enso @@ -633,6 +633,8 @@ spec = t_3.default_visualization.should_equal Visualization.Id.table table_builder columns = - Table.new columns.map description-> [description.at 0, description.at 2] + Table.new <| columns.map description-> [description.at 0, description.at 2] Common_Table_Spec.spec "" table_builder + +main = Test.Suite.run_main here.spec From c0ea1054a5917f681531e97b2a962261a7b68bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Thu, 27 Jan 2022 17:19:13 +0100 Subject: [PATCH 11/28] WIP more tests --- test/Table_Tests/src/Common_Table_Spec.enso | 71 +++++++++++++++++++-- 1 file changed, 64 insertions(+), 7 deletions(-) diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index ab8c99cdcdd5..d0fcfb6b56e7 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -61,20 +61,77 @@ spec prefix table_builder = selector = By_Index [1, 0, 100, 200, 300] expect_column_names ["foo", "bar"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - err = table.select_columns selector on_problems=Problem_Behavior.Report_Error - err . should_fail_with Column_Indexes_Out_Of_Range - err.catch . should_equal <| Column_Indexes_Out_Of_Range [100, 200, 300] + problem = Column_Indexes_Out_Of_Range [100, 200, 300] + res = table.select_columns selector on_problems=Problem_Behavior.Report_Error + res . should_fail_with Missing_Input_Columns + res.catch . should_equal [problem] case Warnings.test_warnings (table.select_columns selector warnings=_) of Pair result warnings -> expect_column_names ["foo", "bar"] result - warnings.should_equal [Column_Indexes_Out_Of_Range [100, 200, 300]] + warnings.should_equal [problem] Test.specify "should correctly handle problems: unmatched names" <| - Nothing + weird_name = '.*?-!@#!"' + selector = By_Name ["foo", "hmm", weird_name] + expect_column_names ["foo"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore + + problem = Missing_Input_Columns ["hmm", weird_name] + res = table.select_columns selector on_problems=Problem_Behavior.Report_Error + res . should_fail_with Missing_Input_Columns + res.catch . should_equal [problem] + + case Warnings.test_warnings (table.select_columns selector warnings=_) of + Pair result warnings -> + expect_column_names ["foo"] result + warnings.should_equal [problem] Test.specify "should correctly handle problems: unmatched columns" <| - Nothing + table_2 = table_builder [["foo", Integer, [0,0,0]], ["weird_column", Integer, [0,0,0]]] + foo = table_2.at "foo" + weird_column = table_2.at "weird_column" + bar = table.at "bar" + + selector = By_Column [bar, weird_column, foo] + expect_column_names ["bar", "foo"] <| table.select_columns selector reorder=True on_problems=Problem_Behavior.Ignore + + problem = Missing_Input_Columns ["weird_column"] + res = table.select_columns selector reorder=True on_problems=Problem_Behavior.Report_Error + res . should_fail_with Missing_Input_Columns + res.catch . should_equal [problem] + + case Warnings.test_warnings (table.select_columns selector reorder=True warnings=_) of + Pair result warnings -> + expect_column_names ["foo"] result + warnings.should_equal [problem] + # also make sure that by column selects the column from the original table, not the one from the input + result.at "foo" . to_vector . should_equal [1,2,3] Test.specify "should correctly handle problems: no columns in the output" <| - Nothing + selector = By_Name [] + expect_column_names [] <| table.select_columns selector on_problems=Problem_Behavior.Ignore + + problem = No_Output_Columns + res = table.select_columns selector on_problems=Problem_Behavior.Report_Error + res . should_fail_with No_Output_Columns + res.catch . should_equal [problem] + + case Warnings.test_warnings (table.select_columns selector warnings=_) of + Pair result warnings -> + expect_column_names [] result + warnings.should_equal [problem] + + Test.specify "should correctly handle multiple problems" <| + selector = By_Name ["hmmm"] + expect_column_names [] <| table.select_columns selector on_problems=Problem_Behavior.Ignore + + problems = [Missing_Input_Columns ["hmmm"], No_Output_Columns] + res = table.select_columns selector on_problems=Problem_Behavior.Report_Error + res . should_fail_with Missing_Input_Columns + # TODO not sure if this test should rely on the ordering of the warnings + res.catch . should_equal problems.first + + case Warnings.test_warnings (table.select_columns selector warnings=_) of + Pair result warnings -> + expect_column_names [] result + warnings.should_equal problems From 9ee0444f5729263c92c3243e2e0a223b470bcf3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 13:45:05 +0100 Subject: [PATCH 12/28] Fix minor issues, mock warning mapping for testing purposes --- .../0.2.32-SNAPSHOT/src/Error/Warnings.enso | 40 +++++++++++++------ .../src/Internal/Table_Helpers.enso | 16 +++++--- test/Table_Tests/src/Common_Table_Spec.enso | 20 +++++----- 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso index b2b2dd8e4efb..83338ac8bb4f 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso @@ -4,7 +4,7 @@ from Standard.Base import all A placeholder for reporting warnings. It should be replaced once the warning mechanism is designed and implemented. type Warning_System - type Warning_System (warning_callback : Any -> Nothing) + type Warning_System (warning_callback : Any -> Nothing) (mapping : Any -> Any) ## UNSTABLE Attaches a warning to a value. @@ -17,33 +17,48 @@ type Warning_System _ -> case warning_payload of _ -> - this.warning_callback warning_payload + this.warning_callback <| this.mapping warning_payload decorated_value + ## PRIVATE + with_mapping : (Any -> Any) -> Warning_System + with_mapping new_mapping = + Warning_System this.warning_callback (new_mapping << this.mapping) + ## PRIVATE The default implementation of a warning system mock. To be removed once warnings are implemented. default : Warning_System -default = Warning_System warning-> - IO.println "[WARNING] "+warning.to_display_text +default = + callback = warning-> + IO.println "[WARNING] "+warning.to_display_text + Warning_System callback (x->x) ## UNSTABLE Maps warnings attached to a value. Currently it is not implemented as the warning system is missing. It just returns the original value without changes. -map_attached_warnings : (Any -> Any) -> Any -> Any -map_attached_warnings mapper decorated_value = - _ = mapper - decorated_value +map_attached_warnings : (Any -> Any) -> Warning_System -> (Warning_System -> Any) -> Any +map_attached_warnings mapper warnings callback = + new_warnings = warnings.with_mapping mapper + result = callback new_warnings + result ## UNSTABLE An utility function which applies the mapping function both to any attached warnings and dataflow errors. -map_warnings_and_errors : (Any -> Any) -> Any -> Any -map_warnings_and_errors mapper decorated_value = - (here.map_attached_warnings mapper decorated_value) . map_error mapper + + The Warning_System has to be passed through to be able to correctly map the + warnings. + + Once the proper Warning system is implemented, the new signature will be: + (Any -> Any) -> Any -> Any + +map_warnings_and_errors : (Any -> Any) -> Warning_System -> (Warning_System -> Any) -> Any +map_warnings_and_errors mapper warnings callback = + (here.map_attached_warnings mapper warnings callback) . map_error mapper ## PRIVATE A temporary helper for testing warnings. @@ -54,8 +69,7 @@ map_warnings_and_errors mapper decorated_value = test_warnings : (Warning_System -> Any) -> Pair Any (Vector Any) test_warnings closure = builder = Vector.new_builder - warning_system = Warning_System warning-> - builder.append warning + warning_system = Warning_System builder.append (x->x) result = closure warning_system warnings = builder.to_vector Pair result warnings diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 86d2fe7ef2be..09e25de0f652 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -40,12 +40,17 @@ select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> War select_columns internal_columns selector reorder on_problems warnings = promote_no_matches_to_missing_columns error = case error of Matching.No_Matches_Found criteria -> Missing_Input_Columns criteria - _ -> error + _ -> + IO.println error + error result = case selector of By_Name names matching_strategy -> - result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result + ## TODO [RW] Once the proper Warning support is implemented, this passing through of a mock `Warning_System` + instance will no longer be necessary. + closure = warnings-> + Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings closure By_Index indices -> partitioned_indices = indices.partition (here.is_index_valid internal_columns) good_indices = partitioned_indices.first @@ -61,8 +66,9 @@ select_columns internal_columns selector reorder on_problems warnings = Problem_Behavior_Module.attach_issues_as_needed (result.distinct on=_.name) on_problems issues warnings=warnings By_Column columns -> names = columns.map .name - result = Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns result + closure = warnings-> + Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings closure issues = if result.is_empty then [No_Output_Columns] else [] Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index d0fcfb6b56e7..c175b0b6ea20 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -23,7 +23,7 @@ from Standard.Table.Data.Column_Selector as Column_Selector_Module import all TODO [RW] the Any in return type of the builder should ideally be replaced with the Table interface, once that is supported. spec : Text -> (Vector -> Any) -> Nothing spec prefix table_builder = - Test.group prefix+" Select" <| + Test.group prefix+"Select" <| table = col1 = ["foo", Integer, [1,2,3]] col2 = ["bar", Integer, [4,5,6]] @@ -63,8 +63,8 @@ spec prefix table_builder = problem = Column_Indexes_Out_Of_Range [100, 200, 300] res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with Missing_Input_Columns - res.catch . should_equal [problem] + res . should_fail_with Column_Indexes_Out_Of_Range + res.catch . should_equal problem case Warnings.test_warnings (table.select_columns selector warnings=_) of Pair result warnings -> @@ -73,13 +73,13 @@ spec prefix table_builder = Test.specify "should correctly handle problems: unmatched names" <| weird_name = '.*?-!@#!"' - selector = By_Name ["foo", "hmm", weird_name] + selector = By_Name ["foo", "hmm", weird_name] (Matching.Exact True) expect_column_names ["foo"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore problem = Missing_Input_Columns ["hmm", weird_name] res = table.select_columns selector on_problems=Problem_Behavior.Report_Error res . should_fail_with Missing_Input_Columns - res.catch . should_equal [problem] + res.catch . should_equal problem case Warnings.test_warnings (table.select_columns selector warnings=_) of Pair result warnings -> @@ -98,23 +98,23 @@ spec prefix table_builder = problem = Missing_Input_Columns ["weird_column"] res = table.select_columns selector reorder=True on_problems=Problem_Behavior.Report_Error res . should_fail_with Missing_Input_Columns - res.catch . should_equal [problem] + res.catch . should_equal problem case Warnings.test_warnings (table.select_columns selector reorder=True warnings=_) of Pair result warnings -> - expect_column_names ["foo"] result + expect_column_names ["bar", "foo"] result warnings.should_equal [problem] # also make sure that by column selects the column from the original table, not the one from the input result.at "foo" . to_vector . should_equal [1,2,3] Test.specify "should correctly handle problems: no columns in the output" <| - selector = By_Name [] + selector = By_Name [] (Matching.Exact True) expect_column_names [] <| table.select_columns selector on_problems=Problem_Behavior.Ignore problem = No_Output_Columns res = table.select_columns selector on_problems=Problem_Behavior.Report_Error res . should_fail_with No_Output_Columns - res.catch . should_equal [problem] + res.catch . should_equal problem case Warnings.test_warnings (table.select_columns selector warnings=_) of Pair result warnings -> @@ -122,7 +122,7 @@ spec prefix table_builder = warnings.should_equal [problem] Test.specify "should correctly handle multiple problems" <| - selector = By_Name ["hmmm"] + selector = By_Name ["hmmm"] (Matching.Exact True) expect_column_names [] <| table.select_columns selector on_problems=Problem_Behavior.Ignore problems = [Missing_Input_Columns ["hmmm"], No_Output_Columns] From a3c56b0b54c96f2d03d27c33c57ab62430cbcbbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 16:13:24 +0100 Subject: [PATCH 13/28] Improve By_Index error handling --- .../src/Error/Problem_Behavior.enso | 8 ++- .../0.2.32-SNAPSHOT/src/Data/Matching.enso | 2 +- .../Table/0.2.32-SNAPSHOT/src/Error.enso | 21 ++++++ .../src/Internal/Table_Helpers.enso | 68 +++++++++++++++---- test/Table_Tests/src/Common_Table_Spec.enso | 37 ++++++++++ 5 files changed, 118 insertions(+), 18 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso index 599a8a0f457a..a47b31855474 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso @@ -51,7 +51,9 @@ attach_as_needed decorated_value problem_behavior ~payload warnings=Warnings.def TODO: the Warning_System argument is temporary, as the warning system is mocked until the real implementation is shipped. It will be removed soon. -attach_issues_as_needed : Any -> Problem_Behavior -> Vector -> Warning_System -> Any -attach_issues_as_needed decorated_value problem_behavior issues warnings=Warnings.default = - issues.fold decorated_value value-> issue-> +attach_issues_as_needed : Problem_Behavior -> Vector -> Warning_System -> Any -> Any +attach_issues_as_needed problem_behavior issues warnings ~decorated_value = + aggregated = issues.fold Nothing value-> issue-> here.attach_as_needed value problem_behavior issue warnings=warnings + case aggregated of + Nothing -> decorated_value diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso index 7f4af0213bb4..7610f0a9a787 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso @@ -123,7 +123,7 @@ match_criteria objects criteria reorder=False name_mapper=(x->x) matching_strate result = selected_indices.map objects.at issues = if unmatched_criteria.is_empty then [] else [No_Matches_Found unmatched_criteria] - Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings + Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings result ## UNSTABLE diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso index 1509eb13dffe..b085eb56f4f9 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Error.enso @@ -33,3 +33,24 @@ type No_Output_Columns No_Output_Columns.to_display_text : Text No_Output_Columns.to_display_text = "The result contains no columns." + + +## Indicates that the provided Column_Selector has duplicate entries. +type Duplicate_Column_Selectors (duplicate_selectors : [(Text | Integer)]) + +Duplicate_Column_Selectors.to_display_text : Text +Duplicate_Column_Selectors.to_display_text = + "The provided Column_Selector has duplicate entries: "+this.duplicate_selectors.short_display_text+"." + +## Indicates that the provided indices matched columns which were already + matched by other indices, so they do not introduce any new columns to the + input. + + For example, if the table has only one column, then selecting + `By_Index [0, -1]` will only yield this single column and + `Input_Indices_Already_Matched [-1]` will be raised. +type Input_Indices_Already_Matched (indices : [Integer]) + +Input_Indices_Already_Matched.to_display_text : Text +Input_Indices_Already_Matched.to_display_text = + "The indices "+this.indices.short_display_text+" matched columns which have been matched earlier by other indices, so they did not introduce any new columns into the result." diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 09e25de0f652..957f928b3cec 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -48,30 +48,42 @@ select_columns internal_columns selector reorder on_problems warnings = By_Name names matching_strategy -> ## TODO [RW] Once the proper Warning support is implemented, this passing through of a mock `Warning_System` instance will no longer be necessary. - closure = warnings-> + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings closure By_Index indices -> partitioned_indices = indices.partition (here.is_index_valid internal_columns) - good_indices = partitioned_indices.first - bad_indices = partitioned_indices.second - issues = if bad_indices.is_empty then [] else - [Column_Indexes_Out_Of_Range bad_indices] - result = case reorder of + inbound_indices = partitioned_indices.first + oob_indices = partitioned_indices.second + + split_result = here.split_to_distinct_and_duplicates inbound_indices + duplicate_indices = split_result.second + distinct_indices = split_result.first + + resolved_indices = distinct_indices.map ix-> Pair ix (here.resolve_index internal_columns ix) + alias_split_result = here.split_to_distinct_and_duplicates resolved_indices .second + aliasing_indices = alias_split_result.second.map .first + good_indices = alias_split_result.first.map .second + + oob_issues = if oob_indices.is_empty then [] else + [Column_Indexes_Out_Of_Range oob_indices] + duplicate_issues = if duplicate_indices.is_empty then [] else + [Duplicate_Column_Selectors duplicate_indices] + aliasing_issues = if aliasing_indices.is_empty then [] else + [Input_Indices_Already_Matched aliasing_indices] + issues = oob_issues + duplicate_issues + aliasing_issues + + Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings <| case reorder of True -> here.select_indices_reordering internal_columns good_indices False -> here.select_indices_preserving_order internal_columns good_indices - # TODO warn if there are any non-unique columns in result? TODO test cases to illustrate - Problem_Behavior_Module.attach_issues_as_needed (result.distinct on=_.name) on_problems issues warnings=warnings By_Column columns -> names = columns.map .name - closure = warnings-> + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings closure issues = if result.is_empty then [No_Output_Columns] else [] - Problem_Behavior_Module.attach_issues_as_needed result on_problems issues warnings=warnings + Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings result ## PRIVATE Selects element from the vector based on the given indices. @@ -83,14 +95,42 @@ select_indices_reordering vector indices = ## PRIVATE Selects element from the vector based on the given indices. - The elements are returned in the same order as they appeared in the original vector. + The elements are returned in the same order as they appeared in the original + vector. select_indices_preserving_order vector indices = indices_to_keep = Map.from_vector (indices.map i-> [i, True]) vector.filter_with_index ix-> _-> indices_to_keep.get_or_else ix False +## PRIVATE + Returns the actual position in the array that the index points to. + + It resolves negative indices to regular indices. + + If the negative index is sufficiently large, a negative result can still be + returned. This function does not ensure that the resulting indices are within + bounds. +resolve_index vector ix = + if ix < 0 then vector.length+ix else ix + ## PRIVATE Checks if the given index is in the valid range for the provided vector. is_index_valid vector ix = - actual_ix = if ix < 0 then vector.length+ix else ix + actual_ix = here.resolve_index vector ix actual_ix>=0 && actual_ix Any) -> Vector -> Pair Vector Vector +split_to_distinct_and_duplicates vector (on = x->x) = + acc = vector.fold (Repeated_Acc Vector.new_builder Vector.new_builder Map.empty) acc-> item-> + key = on item + already_present = acc.existing.get_or_else key False + case already_present of + True -> + Repeated_Acc acc.distinct_builder (acc.duplicate_builder.append item) acc.existing + False -> + Repeated_Acc (acc.distinct_builder.append item) acc.duplicate_builder (acc.existing.insert key True) + Pair acc.distinct_builder.to_vector acc.duplicate_builder.to_vector diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index c175b0b6ea20..05f1862542bf 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -50,6 +50,9 @@ spec prefix table_builder = table_2 . at "bar" . to_vector . should_equal [4,5,6] table_2 . at "foo" . to_vector . should_equal [1,2,3] + Test.specify "should allow negative indices" <| + expect_column_names ["foo", "bar", "foo_2"] <| table.select_columns (By_Index [-1, 0, 1]) + Test.specify "should correctly handle exact matches matching multiple names due to case insensitivity" <| expect_column_names ["bar", "Bar"] <| table.select_columns (By_Name ["bar"] (Matching.Exact Matching.Case_Insensitive)) @@ -71,6 +74,34 @@ spec prefix table_builder = expect_column_names ["foo", "bar"] result warnings.should_equal [problem] + Test.specify "should correctly handle problems: duplicate indices" <| + selector = By_Index [0, 0, 0] + expect_column_names ["foo"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore + + problem = Duplicate_Column_Selectors [0, 0] + res = table.select_columns selector on_problems=Problem_Behavior.Report_Error + res . should_fail_with Duplicate_Column_Selectors + res.catch . should_equal problem + + case Warnings.test_warnings (table.select_columns selector warnings=_) of + Pair result warnings -> + expect_column_names ["foo"] result + warnings.should_equal [problem] + + Test.specify "should correctly handle problems: aliased indices" <| + selector = By_Index [0, -5, -4, 1] + expect_column_names ["foo", "bar"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore + + problem = Input_Indices_Already_Matched [-5, 1] + res = table.select_columns selector on_problems=Problem_Behavior.Report_Error + res . should_fail_with Input_Indices_Already_Matched + res.catch . should_equal problem + + case Warnings.test_warnings (table.select_columns selector warnings=_) of + Pair result warnings -> + expect_column_names ["foo", "bar"] result + warnings.should_equal [problem] + Test.specify "should correctly handle problems: unmatched names" <| weird_name = '.*?-!@#!"' selector = By_Name ["foo", "hmm", weird_name] (Matching.Exact True) @@ -135,3 +166,9 @@ spec prefix table_builder = Pair result warnings -> expect_column_names [] result warnings.should_equal problems + + case Warnings.test_warnings (table.select_columns (By_Index [0, -5, 0, 100]) warnings=_) of + Pair result warnings -> + expect_column_names ["foo"] result + problems = [Column_Indexes_Out_Of_Range [100], Duplicate_Column_Selectors [0], Input_Indices_Already_Matched [-5]] + warnings.should_equal problems From 25544ddc45d6036f6459322b2e2b9cdd3c44a701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 17:01:04 +0100 Subject: [PATCH 14/28] A helper for testing problem handling --- .../Test/0.2.32-SNAPSHOT/src/Problems.enso | 40 ++++++ test/Table_Tests/src/Common_Table_Spec.enso | 118 +++++------------- 2 files changed, 73 insertions(+), 85 deletions(-) create mode 100644 distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso diff --git a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso new file mode 100644 index 000000000000..564bedc27f7e --- /dev/null +++ b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso @@ -0,0 +1,40 @@ +from Standard.Base import all +import Standard.Test + +import Standard.Base.Error.Problem_Behavior +import Standard.Base.Error.Warnings + +## UNSTABLE + Tests how a specific operation behaves depending on the requested + `Problem_Behavior`. + + Arguments: + - action: The action to execute. It takes a `Problem_Behavior` which + specifies whether it should ignore problems, report them as warnings or + raise a dataflow error on the first encountered problem. + - expected_problems: a list of expected problems, in the order that they are + expected to be reported. It should not be empty. The problems are assumed + to be Atoms. + - result_checker: A function which should verify that the result generated by + the action is correct. It does not return anything, instead it should use + the standard testing approach, like `x.should_equal y`. + + TODO [RW] Once proper Warning handling is implemented, the Warning_System + argument will be removed. +test_problem_handling : (Warnings.Warning_System -> Problem_Behavior -> Any) -> Vector Any -> (Any -> Nothing) -> Nothing +test_problem_handling action expected_problems result_checker = + # First, we check the action ignoring any warnings. + result_checker <| action Warnings.default Problem_Behavior.Ignore + + # Then, we check the fail-on-first-error mode. + first_problem = expected_problems.first + first_problem_type = Meta.meta first_problem . constructor + error_result = action Warnings.default Problem_Behavior.Report_Error + error_result . should_fail_with first_problem_type + error_result.catch . should_equal first_problem + + # Lastly, we check the report warnings mode and ensure that both the result is correct and the warnings are as expected. + case Warnings.test_warnings (action _ Problem_Behavior.Report_Warning) of + Pair result warnings -> + result_checker result + warnings . should_equal expected_problems diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index 05f1862542bf..8d4ab862b176 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -1,5 +1,6 @@ from Standard.Base import all import Standard.Test +import Standard.Test.Problems import Standard.Base.Error.Problem_Behavior import Standard.Base.Error.Warnings @@ -62,60 +63,32 @@ spec prefix table_builder = Test.specify "should correctly handle problems: out of bounds indices" <| selector = By_Index [1, 0, 100, 200, 300] - expect_column_names ["foo", "bar"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - - problem = Column_Indexes_Out_Of_Range [100, 200, 300] - res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with Column_Indexes_Out_Of_Range - res.catch . should_equal problem - - case Warnings.test_warnings (table.select_columns selector warnings=_) of - Pair result warnings -> - expect_column_names ["foo", "bar"] result - warnings.should_equal [problem] + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names ["foo", "bar"] + problems = [Column_Indexes_Out_Of_Range [100, 200, 300]] + Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: duplicate indices" <| selector = By_Index [0, 0, 0] - expect_column_names ["foo"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - - problem = Duplicate_Column_Selectors [0, 0] - res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with Duplicate_Column_Selectors - res.catch . should_equal problem - - case Warnings.test_warnings (table.select_columns selector warnings=_) of - Pair result warnings -> - expect_column_names ["foo"] result - warnings.should_equal [problem] + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names ["foo"] + problems = [Duplicate_Column_Selectors [0, 0]] + Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: aliased indices" <| selector = By_Index [0, -5, -4, 1] - expect_column_names ["foo", "bar"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - - problem = Input_Indices_Already_Matched [-5, 1] - res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with Input_Indices_Already_Matched - res.catch . should_equal problem - - case Warnings.test_warnings (table.select_columns selector warnings=_) of - Pair result warnings -> - expect_column_names ["foo", "bar"] result - warnings.should_equal [problem] + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names ["foo", "bar"] + problems = [Input_Indices_Already_Matched [-5, 1]] + Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: unmatched names" <| weird_name = '.*?-!@#!"' selector = By_Name ["foo", "hmm", weird_name] (Matching.Exact True) - expect_column_names ["foo"] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - - problem = Missing_Input_Columns ["hmm", weird_name] - res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with Missing_Input_Columns - res.catch . should_equal problem - - case Warnings.test_warnings (table.select_columns selector warnings=_) of - Pair result warnings -> - expect_column_names ["foo"] result - warnings.should_equal [problem] + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names ["foo"] + problems = [Missing_Input_Columns ["hmm", weird_name]] + Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: unmatched columns" <| table_2 = table_builder [["foo", Integer, [0,0,0]], ["weird_column", Integer, [0,0,0]]] @@ -124,51 +97,26 @@ spec prefix table_builder = bar = table.at "bar" selector = By_Column [bar, weird_column, foo] - expect_column_names ["bar", "foo"] <| table.select_columns selector reorder=True on_problems=Problem_Behavior.Ignore - - problem = Missing_Input_Columns ["weird_column"] - res = table.select_columns selector reorder=True on_problems=Problem_Behavior.Report_Error - res . should_fail_with Missing_Input_Columns - res.catch . should_equal problem - - case Warnings.test_warnings (table.select_columns selector reorder=True warnings=_) of - Pair result warnings -> - expect_column_names ["bar", "foo"] result - warnings.should_equal [problem] - # also make sure that by column selects the column from the original table, not the one from the input - result.at "foo" . to_vector . should_equal [1,2,3] + action = table.select_columns selector reorder=True warnings=_ on_problems=_ + tester = expect_column_names ["bar", "foo"] + problems = [Missing_Input_Columns ["weird_column"]] + Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: no columns in the output" <| selector = By_Name [] (Matching.Exact True) - expect_column_names [] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - - problem = No_Output_Columns - res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with No_Output_Columns - res.catch . should_equal problem - - case Warnings.test_warnings (table.select_columns selector warnings=_) of - Pair result warnings -> - expect_column_names [] result - warnings.should_equal [problem] + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names [] + problems = [No_Output_Columns] + Problems.test_problem_handling action problems tester Test.specify "should correctly handle multiple problems" <| selector = By_Name ["hmmm"] (Matching.Exact True) - expect_column_names [] <| table.select_columns selector on_problems=Problem_Behavior.Ignore - + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names [] problems = [Missing_Input_Columns ["hmmm"], No_Output_Columns] - res = table.select_columns selector on_problems=Problem_Behavior.Report_Error - res . should_fail_with Missing_Input_Columns - # TODO not sure if this test should rely on the ordering of the warnings - res.catch . should_equal problems.first - - case Warnings.test_warnings (table.select_columns selector warnings=_) of - Pair result warnings -> - expect_column_names [] result - warnings.should_equal problems - - case Warnings.test_warnings (table.select_columns (By_Index [0, -5, 0, 100]) warnings=_) of - Pair result warnings -> - expect_column_names ["foo"] result - problems = [Column_Indexes_Out_Of_Range [100], Duplicate_Column_Selectors [0], Input_Indices_Already_Matched [-5]] - warnings.should_equal problems + Problems.test_problem_handling action problems tester + + action_2 = table.select_columns (By_Index [0, -5, 0, 100]) warnings=_ on_problems=_ + problems_2 = [Column_Indexes_Out_Of_Range [100], Duplicate_Column_Selectors [0], Input_Indices_Already_Matched [-5]] + tester_2 = expect_column_names ["foo"] + Problems.test_problem_handling action_2 problems_2 tester_2 From e368cbc6d3e2683813ef96624edac407eef87839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 17:25:45 +0100 Subject: [PATCH 15/28] More error handling --- .../src/Internal/Table_Helpers.enso | 22 ++++++++++++++----- test/Table_Tests/src/Common_Table_Spec.enso | 15 +++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 957f928b3cec..e235d2ffb217 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -48,8 +48,14 @@ select_columns internal_columns selector reorder on_problems warnings = By_Name names matching_strategy -> ## TODO [RW] Once the proper Warning support is implemented, this passing through of a mock `Warning_System` instance will no longer be necessary. - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> - Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings + split_result = here.split_to_distinct_and_duplicates names + distinct_names = split_result.first + duplicate_names = split_result.second + issues = if duplicate_names.is_empty then [] else + [Duplicate_Column_Selectors duplicate_names] + Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings <| + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> + Matching.match_criteria internal_columns distinct_names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings By_Index indices -> partitioned_indices = indices.partition (here.is_index_valid internal_columns) inbound_indices = partitioned_indices.first @@ -78,9 +84,15 @@ select_columns internal_columns selector reorder on_problems warnings = False -> here.select_indices_preserving_order internal_columns good_indices By_Column columns -> - names = columns.map .name - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> - Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings + split_result = here.split_to_distinct_and_duplicates columns .name + distinct_columns = split_result.first + duplicate_column_names = split_result.second . map .name + issues = if duplicate_column_names.is_empty then [] else + [Duplicate_Column_Selectors duplicate_column_names] + Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings <| + names = distinct_columns.map .name + Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> + Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings issues = if result.is_empty then [No_Output_Columns] else [] Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings result diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index 8d4ab862b176..80c9e5bca259 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -82,6 +82,13 @@ spec prefix table_builder = problems = [Input_Indices_Already_Matched [-5, 1]] Problems.test_problem_handling action problems tester + Test.specify "should correctly handle problems: duplicate names" <| + selector = By_Name ["foo", "foo"] (Matching.Exact True) + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names ["foo"] + problems = [Duplicate_Column_Selectors ["foo"]] + Problems.test_problem_handling action problems tester + Test.specify "should correctly handle problems: unmatched names" <| weird_name = '.*?-!@#!"' selector = By_Name ["foo", "hmm", weird_name] (Matching.Exact True) @@ -90,6 +97,14 @@ spec prefix table_builder = problems = [Missing_Input_Columns ["hmm", weird_name]] Problems.test_problem_handling action problems tester + Test.specify "should correctly handle problems: duplicate columns" <| + foo = table.at "foo" + selector = By_Column [foo, foo] + action = table.select_columns selector warnings=_ on_problems=_ + tester = expect_column_names ["foo"] + problems = [Duplicate_Column_Selectors ["foo"]] + Problems.test_problem_handling action problems tester + Test.specify "should correctly handle problems: unmatched columns" <| table_2 = table_builder [["foo", Integer, [0,0,0]], ["weird_column", Integer, [0,0,0]]] foo = table_2.at "foo" From 273e58bfa6da2ce9545f760e2882ab4c440adb36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 17:28:05 +0100 Subject: [PATCH 16/28] docs --- .../Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso | 5 +++++ .../lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index 8f5021a08b4f..9477b9df0524 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -92,7 +92,12 @@ type Table The following problems can occur: - If a column in columns is not in the input table, a `Missing_Input_Columns`. + - If duplicate columns, names or indices are provided, a + `Duplicate_Column_Selectors`. - If a column index is out of range, a `Column_Indexes_Out_Of_Range`. + - If two distinct indices would refer to the same column, a + `Input_Indices_Already_Matched`, indicating that the additional + indices will not introduce additional columns. - If there are no columns in the output table, a `No_Output_Columns`. - warnings: A `Warning_System` instance specifying how to handle warnings. This is a temporary workaround to allow for testing the diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index b6c7edd2016e..ef838f542dbd 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -251,7 +251,12 @@ type Table The following problems can occur: - If a column in columns is not in the input table, a `Missing_Input_Columns`. + - If duplicate columns, names or indices are provided, a + `Duplicate_Column_Selectors`. - If a column index is out of range, a `Column_Indexes_Out_Of_Range`. + - If two distinct indices would refer to the same column, a + `Input_Indices_Already_Matched`, indicating that the additional + indices will not introduce additional columns. - If there are no columns in the output table, a `No_Output_Columns`. - warnings: A `Warning_System` instance specifying how to handle warnings. This is a temporary workaround to allow for testing the From 18779845bd514ac09b656aa8d5a639cfe9caf5a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 17:29:40 +0100 Subject: [PATCH 17/28] changelog --- app/gui/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/gui/CHANGELOG.md b/app/gui/CHANGELOG.md index e489aed661f1..c7fbcde0de7a 100644 --- a/app/gui/CHANGELOG.md +++ b/app/gui/CHANGELOG.md @@ -27,6 +27,7 @@ `Vector.filter_with_index`. Made `Vector.at` accept negative indices and ensured it fails with a dataflow error on out of bounds access instead of an internal Java exception.][3232] +- [Implemented the `Table.select_columns` operation.][3230] [3153]: https://github.com/enso-org/enso/pull/3153 [3166]: https://github.com/enso-org/enso/pull/3166 @@ -38,6 +39,7 @@ [3229]: https://github.com/enso-org/enso/pull/3229 [3231]: https://github.com/enso-org/enso/pull/3231 [3232]: https://github.com/enso-org/enso/pull/3232 +[3230]: https://github.com/enso-org/enso/pull/3230 # Enso 2.0.0-alpha.18 (2021-10-12) From 3677214d85df2cc821a1b4970214475a75549e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 22:48:12 +0100 Subject: [PATCH 18/28] Fix matching test --- .../0.2.32-SNAPSHOT/src/Data/Table.enso | 5 ++++ .../Test/0.2.32-SNAPSHOT/src/Main.enso | 23 +++++++++++-------- .../Test/0.2.32-SNAPSHOT/src/Problems.enso | 6 ++--- test/Table_Tests/src/Database_Spec.enso | 14 +++++++++++ test/Table_Tests/src/Main.enso | 4 +++- test/Table_Tests/src/Matching_Spec.enso | 21 ++++++++--------- 6 files changed, 48 insertions(+), 25 deletions(-) create mode 100644 test/Table_Tests/src/Database_Spec.enso diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index 9477b9df0524..3fe20cf5bda7 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -6,11 +6,16 @@ import Standard.Database.Data.Sql import Standard.Table.Data.Column as Materialized_Column import Standard.Table.Data.Table as Materialized_Table import Standard.Table.Internal.Java_Exports +import Standard.Table.Internal.Table_Helpers from Standard.Database.Data.Column as Column_Module import all from Standard.Database.Data.Internal.IR import Internal_Column from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule from Standard.Table.Data.Table import No_Such_Column_Error +from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule +from Standard.Table.Data.Column_Selector as Column_Selector_Module import all +from Standard.Base.Error.Problem_Behavior as Problem_Behavior_Module import all +import Standard.Base.Error.Warnings polyglot java import java.sql.JDBCType diff --git a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso index 6b924ad459b0..0090ab24365f 100644 --- a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso +++ b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso @@ -143,6 +143,8 @@ fail message = Panic.throw (Failure message) Arguments: - matcher: The expected type of dataflow error contained in `this`. + - frames_to_skip (optional, advanced): used to alter the location which is + displayed as the source of this error. > Example Assert that a compuation should return an error of a given type. @@ -152,9 +154,9 @@ fail message = Panic.throw (Failure message) example_should_fail_with = Examples.throw_error . should_fail_with Examples.My_Error -Any.should_fail_with : Any -> Assertion -Any.should_fail_with matcher = - loc = Meta.get_source_location 1 +Any.should_fail_with : Any -> Integer -> Assertion +Any.should_fail_with matcher frames_to_skip=0 = + loc = Meta.get_source_location 1+frames_to_skip here.fail ("Expected an error " + matcher.to_text + " but none occurred (at " + loc + ").") ## Expect a function to fail with the provided dataflow error. @@ -170,11 +172,12 @@ Any.should_fail_with matcher = example_should_fail_with = Examples.throw_error . should_fail_with Examples.My_Error -Error.should_fail_with : Any -> Assertion -Error.should_fail_with matcher = +Error.should_fail_with : Any -> Integer -> Assertion +Error.should_fail_with matcher frames_to_skip=0 = caught = this.catch x->x if caught.is_a matcher then Nothing else - here.fail ("Unexpected error " + caught.to_text + " returned.") + loc = Meta.get_source_location 2+frames_to_skip + here.fail ("Unexpected error " + caught.to_text + " returned (at " + loc + ".") ## Expect a function to fail with the provided panic. @@ -203,6 +206,8 @@ expect_panic_with ~action matcher = Arguments: - that: The value to check `this` for equality with. + - frames_to_skip (optional, advanced): used to alter the location which is + displayed as the source of this error. > Example Assert that one value should equal another, @@ -211,11 +216,11 @@ expect_panic_with ~action matcher = import Standard.Test example_should_equal = Examples.add_1_to 1 . should_equal 2 -Any.should_equal : Any -> Assertion -Any.should_equal that = case this == that of +Any.should_equal : Any -> Integer -> Assertion +Any.should_equal that frames_to_skip=0 = case this == that of True -> Success False -> - loc = Meta.get_source_location 2 + loc = Meta.get_source_location 2+frames_to_skip msg = this.to_text + " did not equal " + that.to_text + " (at " + loc + ")." here.fail msg diff --git a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso index 564bedc27f7e..ff3564a5522c 100644 --- a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso +++ b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Problems.enso @@ -30,11 +30,11 @@ test_problem_handling action expected_problems result_checker = first_problem = expected_problems.first first_problem_type = Meta.meta first_problem . constructor error_result = action Warnings.default Problem_Behavior.Report_Error - error_result . should_fail_with first_problem_type - error_result.catch . should_equal first_problem + error_result . should_fail_with first_problem_type frames_to_skip=1 + error_result.catch . should_equal first_problem frames_to_skip=1 # Lastly, we check the report warnings mode and ensure that both the result is correct and the warnings are as expected. case Warnings.test_warnings (action _ Problem_Behavior.Report_Warning) of Pair result warnings -> result_checker result - warnings . should_equal expected_problems + warnings . should_equal expected_problems frames_to_skip=1 diff --git a/test/Table_Tests/src/Database_Spec.enso b/test/Table_Tests/src/Database_Spec.enso new file mode 100644 index 000000000000..17cacac6557b --- /dev/null +++ b/test/Table_Tests/src/Database_Spec.enso @@ -0,0 +1,14 @@ +from Standard.Base import all +from Standard.Database import all + +import Standard.Test + +import project.Common_Table_Spec + +sqlite_spec = + table_builder columns = + Error.throw "TODO" + + #Common_Table_Spec.spec "[SQLite] " table_builder + +main = Test.Suite.run_main here.sqlite_spec diff --git a/test/Table_Tests/src/Main.enso b/test/Table_Tests/src/Main.enso index b17bf4d76f4d..e3f0ed88ae34 100644 --- a/test/Table_Tests/src/Main.enso +++ b/test/Table_Tests/src/Main.enso @@ -2,6 +2,7 @@ from Standard.Base import all import Standard.Test +import project.Database_Spec import project.Matching_Spec import project.Model_Spec import project.Column_Spec @@ -15,6 +16,7 @@ main = Test.Suite.run_main <| Csv_Spec.spec Json_Spec.spec Spreadsheet_Spec.spec - Table_Spec.spec Matching_Spec.spec + Table_Spec.spec + Database_Spec.sqlite_spec Model_Spec.spec diff --git a/test/Table_Tests/src/Matching_Spec.enso b/test/Table_Tests/src/Matching_Spec.enso index eb2f0a792f54..4bdd59993b60 100644 --- a/test/Table_Tests/src/Matching_Spec.enso +++ b/test/Table_Tests/src/Matching_Spec.enso @@ -5,6 +5,7 @@ from Standard.Table.Error as Error_Module import all import Standard.Base.Error.Problem_Behavior import Standard.Base.Error.Warnings import Standard.Test +import Standard.Test.Problems type Foo_Error @@ -55,18 +56,14 @@ spec = Test.group 'Matching Helper' <| Matching.match_criteria ["foo", "bar", "baz", "zap"] [".*z.*", "b.*"] reorder=False matching_strategy=regex . should_equal ["bar", "baz", "zap"] Test.specify 'Should correctly handle criteria which did not match anything' <| - Matching.match_criteria ["foo", "bar", "baz"] ["baz", "unknown_column"] reorder=True on_problems=Problem_Behavior.Report_Error . should_fail_with No_Matches_Found - result = Matching.match_criteria ["foo", "bar", "baz"] ["baz", "unknown_column_1", "unknown_column_2"] reorder=False on_problems=Problem_Behavior.Report_Error . catch - result . should_equal <| No_Matches_Found ["unknown_column_1", "unknown_column_2"] - - warnings_builder = Vector.new_builder - report_warning warning = - warnings_builder.append warning - warning_system = Warnings.Warning_System report_warning - Matching.match_criteria ["foo", "bar", "baz"] ["baz", "unknown_column_1", "unknown_column_2"] reorder=True on_problems=Problem_Behavior.Report_Warning warnings=warning_system . should_equal ["baz"] - reported = warnings_builder.to_vector - reported.length . should_equal 1 - reported.first . should_equal <| No_Matches_Found ["unknown_column_1", "unknown_column_2"] + action = Matching.match_criteria ["foo", "bar", "baz"] ["baz", "unknown_column"] reorder=True warnings=_ on_problems=_ + tester = _.should_equal ["baz"] + problems = [No_Matches_Found ["unknown_column"]] + Problems.test_problem_handling action problems tester + + action_2 = Matching.match_criteria ["foo", "bar", "baz"] ["baz", "unknown_column_1", "unknown_column_2"] reorder=False warnings=_ on_problems=_ + problems_2 = [No_Matches_Found ["unknown_column_1", "unknown_column_2"]] + Problems.test_problem_handling action_2 problems_2 tester Test.specify 'Should correctly work with complex object using a function extracting their names' <| pairs = [Pair "foo" 42, Pair "bar" 33, Pair "baz" 10, Pair "foo" 0, Pair 10 10] From 4067594aeabc2cc0d4570f842a5adfae5edc9e15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Fri, 28 Jan 2022 23:17:41 +0100 Subject: [PATCH 19/28] Add SQLite tests --- .../Test/0.2.32-SNAPSHOT/src/Main.enso | 6 ++-- test/Table_Tests/src/Common_Table_Spec.enso | 22 +++++++++----- test/Table_Tests/src/Database_Spec.enso | 29 +++++++++++++++++-- test/Table_Tests/src/Table_Spec.enso | 2 +- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso index 0090ab24365f..5e0f39b0e7e4 100644 --- a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso +++ b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso @@ -259,11 +259,11 @@ Error.should_equal _ = Panic.throw (Matched_On_Error this) example_should_equal = 1.00000001 . should_equal 1.00000002 epsilon=0.0001 -Decimal.should_equal : Decimal -> Decimal -> Assertion -Decimal.should_equal that (epsilon = 0) = case this.equals that epsilon of +Decimal.should_equal : Decimal -> Decimal -> Integer -> Assertion +Decimal.should_equal that (epsilon = 0) (frames_to_skip=0) = case this.equals that epsilon of True -> Success False -> - loc = Meta.get_source_location 2 + loc = Meta.get_source_location 2+frames_to_skip msg = this.to_text + " did not equal " + that.to_text + " (at " + loc + ")." Panic.throw (Failure msg) diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index 80c9e5bca259..d4897d2a269e 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -22,13 +22,13 @@ from Standard.Table.Data.Column_Selector as Column_Selector_Module import all column elements. TODO [RW] the Any in return type of the builder should ideally be replaced with the Table interface, once that is supported. -spec : Text -> (Vector -> Any) -> Nothing -spec prefix table_builder = +spec : Text -> (Vector -> Any) -> Boolean -> Nothing +spec prefix table_builder supports_case_sensitive_columns = Test.group prefix+"Select" <| table = col1 = ["foo", Integer, [1,2,3]] col2 = ["bar", Integer, [4,5,6]] - col3 = ["Bar", Integer, [7,8,9]] + col3 = ["Baz", Integer, [7,8,9]] col4 = ["foo_1", Integer, [10,11,12]] col5 = ["foo_2", Integer, [13,14,15]] table_builder [col1, col2, col3, col4, col5] @@ -38,12 +38,12 @@ spec prefix table_builder = Test.specify "should work as shown in the doc examples" <| expect_column_names ["foo", "bar"] <| table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) - expect_column_names ["bar", "Bar", "foo_1", "foo_2"] <| table.select_columns (By_Name ["foo.+", "b.*"] (Matching.Regex Matching.Case_Insensitive)) + expect_column_names ["bar", "Baz", "foo_1", "foo_2"] <| table.select_columns (By_Name ["foo.+", "b.*"] (Matching.Regex Matching.Case_Insensitive)) expect_column_names ["foo_2", "foo", "bar"] <| table.select_columns (By_Index [-1, 0, 1]) reorder=True column1 = table.at "foo_1" - column2 = table.at "Bar" - expect_column_names ["Bar", "foo_1"] <| table.select_columns (By_Column [column1, column2]) + column2 = table.at "Baz" + expect_column_names ["Baz", "foo_1"] <| table.select_columns (By_Column [column1, column2]) Test.specify "should allow to reorder columns if asked to" <| table_2 = table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) reorder=True @@ -54,8 +54,14 @@ spec prefix table_builder = Test.specify "should allow negative indices" <| expect_column_names ["foo", "bar", "foo_2"] <| table.select_columns (By_Index [-1, 0, 1]) - Test.specify "should correctly handle exact matches matching multiple names due to case insensitivity" <| - expect_column_names ["bar", "Bar"] <| table.select_columns (By_Name ["bar"] (Matching.Exact Matching.Case_Insensitive)) + if supports_case_sensitive_columns then + Test.specify "should correctly handle exact matches matching multiple names due to case insensitivity" <| + table = + col1 = ["foo", Integer, [1,2,3]] + col2 = ["bar", Integer, [4,5,6]] + col3 = ["Bar", Integer, [7,8,9]] + table_builder [col1, col2, col3] + expect_column_names ["bar", "Bar"] <| table.select_columns (By_Name ["bar"] (Matching.Exact Matching.Case_Insensitive)) Test.specify "should correctly handle regexes matching multiple names" <| expect_column_names ["foo", "bar", "foo_1", "foo_2"] <| table.select_columns (By_Name ["b.*", "f.+"] (Matching.Regex True)) diff --git a/test/Table_Tests/src/Database_Spec.enso b/test/Table_Tests/src/Database_Spec.enso index 17cacac6557b..96c207ce3800 100644 --- a/test/Table_Tests/src/Database_Spec.enso +++ b/test/Table_Tests/src/Database_Spec.enso @@ -6,9 +6,34 @@ import Standard.Test import project.Common_Table_Spec sqlite_spec = + Enso_Project.data.create_directory + file = Enso_Project.data / "sqlite_test.db" + file.delete_if_exists + connection = Database.open_sqlite_file file + + name_counter = Ref.new 0 table_builder columns = - Error.throw "TODO" + ix = Ref.get name_counter + Ref.put name_counter ix+1 + name = "table_"+ix.to_text + quote x = '"' + x + '"' + # TODO this is a hack with no sanitization, just for testing; it should be removed when proper create table is supported by the library + column_definitions = columns.map col-> + name = col.first + typ = case col.second of + Integer -> "INT" + _ -> Panic.throw "The provided type "+col.second+" is not currently supported by the test suite. It may need to be extended." + quote name + " " + typ + sql = "CREATE TABLE " + quote name + " (" + (column_definitions.join ", ") + ")" + Panic.rethrow <| connection.execute_update sql + table = Panic.rethrow <| connection.access_table name + + row_number = columns.first.at 2 . length + 0.up_to row_number . each ix-> + row = columns.map col-> col.at 2 . at ix + table.insert row + table - #Common_Table_Spec.spec "[SQLite] " table_builder + Common_Table_Spec.spec "[SQLite] " table_builder supports_case_sensitive_columns=False main = Test.Suite.run_main here.sqlite_spec diff --git a/test/Table_Tests/src/Table_Spec.enso b/test/Table_Tests/src/Table_Spec.enso index a5561a39dc49..e1c56c4be226 100644 --- a/test/Table_Tests/src/Table_Spec.enso +++ b/test/Table_Tests/src/Table_Spec.enso @@ -635,6 +635,6 @@ spec = table_builder columns = Table.new <| columns.map description-> [description.at 0, description.at 2] - Common_Table_Spec.spec "" table_builder + Common_Table_Spec.spec "" table_builder supports_case_sensitive_columns=True main = Test.Suite.run_main here.spec From 30f188873558da3c46d178a963683e2f4981ed08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 12:00:55 +0100 Subject: [PATCH 20/28] cleanup after test --- test/Table_Tests/src/Database_Spec.enso | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/Table_Tests/src/Database_Spec.enso b/test/Table_Tests/src/Database_Spec.enso index 96c207ce3800..8dbdf48819d3 100644 --- a/test/Table_Tests/src/Database_Spec.enso +++ b/test/Table_Tests/src/Database_Spec.enso @@ -36,4 +36,7 @@ sqlite_spec = Common_Table_Spec.spec "[SQLite] " table_builder supports_case_sensitive_columns=False + connection.close + file.delete + main = Test.Suite.run_main here.sqlite_spec From b425093b82c835a2f9bab3dd17014626c23eb403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 13:54:22 +0100 Subject: [PATCH 21/28] Rework problem handling --- .../src/Error/Problem_Behavior.enso | 138 +++++++++++++----- .../0.2.32-SNAPSHOT/src/Error/Warnings.enso | 10 ++ .../0.2.32-SNAPSHOT/src/Data/Matching.enso | 5 +- .../src/Internal/Table_Helpers.enso | 30 ++-- 4 files changed, 127 insertions(+), 56 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso index a47b31855474..44ec106a7b34 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso @@ -16,44 +16,110 @@ type Problem_Behavior Report the problem as a dataflow error and abort the operation type Report_Error -## UNSTABLE - Attaches an error-value to the given value according to the expected problem - behavior. - - If the problem behavior is set to Ignore, the value is returned as-is. - If it is set to Report_Warning, the value is returned with the error-value - attached as a warning. - If it is set to Report_Error, the error-value is returned in the form of a - dataflow error. - - TODO: the Warning_System argument is temporary, as the warning system is - mocked until the real implementation is shipped. It will be removed soon. -attach_as_needed : Any -> Problem_Behavior -> Vector -> Warning_System -> Any -attach_as_needed decorated_value problem_behavior ~payload warnings=Warnings.default = - case problem_behavior of + ## UNSTABLE + Attaches a problem to the given value according to the expected problem + behavior. + + If the problem behavior is set to Ignore, the value is returned as-is. + If it is set to Report_Warning, the value is returned with the problem + attached as a warning, after any warnings that were already attached to + this value. + If it is set to Report_Error, the problem is returned in the form of a + dataflow error. If the value already contained any dataflow error, that + error takes precedence. + + TODO [RW] the Warning_System argument is temporary, as the warning system + is mocked until the real implementation is shipped. It will be removed + soon. See: https://www.pivotaltracker.com/story/show/180901472 + attach_problem_after : Any -> Any -> Warning_System -> Any + attach_problem_after decorated_value ~problem warnings = case this of Ignore -> decorated_value Report_Warning -> - warnings.attach decorated_value payload + warnings.attach decorated_value problem Report_Error -> case decorated_value of - _ -> Error.throw payload - -## UNSTABLE - Attaches issues to the given value according to the expected problem - behavior. - - If the problem behavior is set to Ignore, the value is returned as-is. - If it is set to Report_Warning, the value is returned with the issues - attached as warnings. - If it is set to Report_Error, the first issue is returned in the form of a - dataflow error. - - TODO: the Warning_System argument is temporary, as the warning system is - mocked until the real implementation is shipped. It will be removed soon. -attach_issues_as_needed : Problem_Behavior -> Vector -> Warning_System -> Any -> Any -attach_issues_as_needed problem_behavior issues warnings ~decorated_value = - aggregated = issues.fold Nothing value-> issue-> - here.attach_as_needed value problem_behavior issue warnings=warnings - case aggregated of - Nothing -> decorated_value + _ -> Error.throw problem + + ## UNSTABLE + Attaches a problem to the given value according to the expected problem + behavior. + + If the problem behavior is set to Ignore, the value is returned as-is. + If it is set to Report_Warning, the value is returned with the problem + attached as a warning, before any warnings that were already attached to + this value. + + TODO [RW] attaching before a warning is not supported in the mock warning + system, it can only be added once the full Warning system is implemented. + See: https://www.pivotaltracker.com/story/show/180901472 + + If it is set to Report_Error, the problem is returned in the form of + a dataflow error. The problem takes precedence over any errors that may + have been contained in the value - in this case the `decorated_value` is + not computed at all. + + TODO [RW] the Warning_System argument is temporary, as the warning system + is mocked until the real implementation is shipped. It will be removed + soon. See: https://www.pivotaltracker.com/story/show/180901472 + attach_problem_before : Any -> Any -> Warning_System -> Any + attach_problem_before problem warnings ~decorated_value = case this of + Ignore -> + decorated_value + Report_Warning -> + # TODO [RW] attach before; see comment above. + warnings.attach decorated_value problem + Report_Error -> + Error.throw problem + + ## UNSTABLE + Attaches problems to the given value according to the expected problem + behavior. + + If the problem behavior is set to Ignore, the value is returned as-is. + If it is set to Report_Warning, the value is returned with the problems + attached as warnings, before any warnings that were already attached to + this value. + If it is set to Report_Error, the first problem is returned in the form + of a dataflow error. The problem takes precedence over any errors that + may have been contained in the value - in this case the `decorated_value` + is not computed at all. + + TODO [RW] the Warning_System argument is temporary, as the warning system + is mocked until the real implementation is shipped. It will be removed + soon. See: https://www.pivotaltracker.com/story/show/180901472 + attach_problems_before : Vector -> Warning_System -> Any -> Any + attach_problems_before problems warnings ~decorated_value = case this of + Ignore -> + decorated_value + Report_Warning -> + # TODO [RW] attach before; see comment above. + warnings.attach_many decorated_value problems + Report_Error -> + if problems.is_empty then decorated_value else + Error.throw problems.first + + ## UNSTABLE + Attaches problems to the given value according to the expected problem + behavior. + + If the problem behavior is set to Ignore, the value is returned as-is. + If it is set to Report_Warning, the value is returned with the problems + attached as warnings, after any warnings that were already attached to + this value. + If it is set to Report_Error, the first problem is returned in the form + of a dataflow error. If the value already contained any dataflow error, + that error takes precedence. + + TODO [RW] the Warning_System argument is temporary, as the warning system + is mocked until the real implementation is shipped. It will be removed + soon. See: https://www.pivotaltracker.com/story/show/180901472 + attach_problems_after : Any -> Vector -> Warning_System -> Any + attach_problems_after decorated_value ~problems warnings = case this of + Ignore -> + decorated_value + Report_Warning -> + warnings.attach_many decorated_value problems + Report_Error -> case decorated_value of + _ -> if problems.is_empty then decorated_value else + Error.throw problems.first diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso index 83338ac8bb4f..5679f7b0df32 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Warnings.enso @@ -20,6 +20,16 @@ type Warning_System this.warning_callback <| this.mapping warning_payload decorated_value + ## UNSTABLE + Attaches multiple warnings to a value. + + If the warning argument holds a dataflow error, the error is also + inherited by the decorated value. + attach_many : Any -> Vector -> Any + attach_many decorated_value warnings = + warnings.fold decorated_value acc-> warning-> + this.attach acc warning + ## PRIVATE with_mapping : (Any -> Any) -> Warning_System with_mapping new_mapping = diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso index 7610f0a9a787..9b40bc92cf33 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Matching.enso @@ -122,8 +122,9 @@ match_criteria objects criteria reorder=False name_mapper=(x->x) matching_strate select_matching_indices is_object_matched_by_anything result = selected_indices.map objects.at - issues = if unmatched_criteria.is_empty then [] else [No_Matches_Found unmatched_criteria] - Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings result + problems = if unmatched_criteria.is_empty then [] else + [No_Matches_Found unmatched_criteria] + on_problems.attach_problems_after result problems warnings ## UNSTABLE diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index e235d2ffb217..4de0b142af40 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -51,9 +51,9 @@ select_columns internal_columns selector reorder on_problems warnings = split_result = here.split_to_distinct_and_duplicates names distinct_names = split_result.first duplicate_names = split_result.second - issues = if duplicate_names.is_empty then [] else + problems = if duplicate_names.is_empty then [] else [Duplicate_Column_Selectors duplicate_names] - Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings <| + on_problems.attach_problems_before problems warnings <| Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> Matching.match_criteria internal_columns distinct_names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings By_Index indices -> @@ -70,32 +70,26 @@ select_columns internal_columns selector reorder on_problems warnings = aliasing_indices = alias_split_result.second.map .first good_indices = alias_split_result.first.map .second - oob_issues = if oob_indices.is_empty then [] else + oob_problems = if oob_indices.is_empty then [] else [Column_Indexes_Out_Of_Range oob_indices] - duplicate_issues = if duplicate_indices.is_empty then [] else + duplicate_problems = if duplicate_indices.is_empty then [] else [Duplicate_Column_Selectors duplicate_indices] - aliasing_issues = if aliasing_indices.is_empty then [] else + aliasing_problems = if aliasing_indices.is_empty then [] else [Input_Indices_Already_Matched aliasing_indices] - issues = oob_issues + duplicate_issues + aliasing_issues + problems = oob_problems + duplicate_problems + aliasing_problems - Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings <| case reorder of + on_problems.attach_problems_before problems warnings <| case reorder of True -> here.select_indices_reordering internal_columns good_indices False -> here.select_indices_preserving_order internal_columns good_indices By_Column columns -> - split_result = here.split_to_distinct_and_duplicates columns .name - distinct_columns = split_result.first - duplicate_column_names = split_result.second . map .name - issues = if duplicate_column_names.is_empty then [] else - [Duplicate_Column_Selectors duplicate_column_names] - Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings <| - names = distinct_columns.map .name - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> - Matching.match_criteria internal_columns names reorder=reorder name_mapper=(_.name) matching_strategy=(Matching.Exact case_sensitivity=True) on_problems=on_problems warnings=warnings + column_names = columns.map .name + new_selector = By_Name column_names (Matching.Exact case_sensitivity=True) + here.select_columns internal_columns new_selector reorder=reorder on_problems=on_problems warnings=warnings - issues = if result.is_empty then [No_Output_Columns] else [] - Problem_Behavior_Module.attach_issues_as_needed on_problems issues warnings result + problems = if result.is_empty then [No_Output_Columns] else [] + on_problems.attach_problems_after result problems warnings ## PRIVATE Selects element from the vector based on the given indices. From 16b8fdea925370241d04f43f18f24fe308912d01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 13:57:53 +0100 Subject: [PATCH 22/28] small refactor --- .../src/Internal/Table_Helpers.enso | 129 +++++++++++------- 1 file changed, 79 insertions(+), 50 deletions(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 4de0b142af40..288ecf6d5d1c 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -38,59 +38,88 @@ from Standard.Table.Error as Error_Module import all will be removed in the future. select_columns : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Vector select_columns internal_columns selector reorder on_problems warnings = - promote_no_matches_to_missing_columns error = case error of - Matching.No_Matches_Found criteria -> Missing_Input_Columns criteria - _ -> - IO.println error - error - - result = case selector of - By_Name names matching_strategy -> - ## TODO [RW] Once the proper Warning support is implemented, this passing through of a mock `Warning_System` - instance will no longer be necessary. - split_result = here.split_to_distinct_and_duplicates names - distinct_names = split_result.first - duplicate_names = split_result.second - problems = if duplicate_names.is_empty then [] else - [Duplicate_Column_Selectors duplicate_names] - on_problems.attach_problems_before problems warnings <| - Warnings.map_warnings_and_errors promote_no_matches_to_missing_columns warnings warnings-> - Matching.match_criteria internal_columns distinct_names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings - By_Index indices -> - partitioned_indices = indices.partition (here.is_index_valid internal_columns) - inbound_indices = partitioned_indices.first - oob_indices = partitioned_indices.second - - split_result = here.split_to_distinct_and_duplicates inbound_indices - duplicate_indices = split_result.second - distinct_indices = split_result.first - - resolved_indices = distinct_indices.map ix-> Pair ix (here.resolve_index internal_columns ix) - alias_split_result = here.split_to_distinct_and_duplicates resolved_indices .second - aliasing_indices = alias_split_result.second.map .first - good_indices = alias_split_result.first.map .second - - oob_problems = if oob_indices.is_empty then [] else - [Column_Indexes_Out_Of_Range oob_indices] - duplicate_problems = if duplicate_indices.is_empty then [] else - [Duplicate_Column_Selectors duplicate_indices] - aliasing_problems = if aliasing_indices.is_empty then [] else - [Input_Indices_Already_Matched aliasing_indices] - problems = oob_problems + duplicate_problems + aliasing_problems - - on_problems.attach_problems_before problems warnings <| case reorder of - True -> - here.select_indices_reordering internal_columns good_indices - False -> - here.select_indices_preserving_order internal_columns good_indices - By_Column columns -> - column_names = columns.map .name - new_selector = By_Name column_names (Matching.Exact case_sensitivity=True) - here.select_columns internal_columns new_selector reorder=reorder on_problems=on_problems warnings=warnings - + result = here.select_columns_helper internal_columns selector reorder on_problems warnings problems = if result.is_empty then [No_Output_Columns] else [] on_problems.attach_problems_after result problems warnings +## PRIVATE + A helper function which selects columns from the table based on the provided + selection criteria. + + Arguments: + - internal_columns: A list of all columns in a table. + - selector: Column selection criteria. + - reorder: Specifies whether to reorder the matched columns according to the + order of the selection criteria. + If `False`, the matched entries are returned in the same order as in the + input. + If `True`, the matched entries are returned in the order of the criteria + matching them. If a single object has been matched by multiple criteria, it + is placed in the group belonging to the first matching criterion on the + list. If a single criterion's group has more than one element, their + relative order is the same as in the input. + - on_problems: Specifies the behavior when a problem occurs during the + operation. By default, a warning is issued, but the operation proceeds. + If set to `Report_Error`, the operation fails with a dataflow error. + If set to `Ignore`, the operation proceeds without errors or warnings. + - warnings: A Warning_System instance specifying how to handle warnings. This + is a temporary workaround to allow for testing the warning mechanism. Once + the proper warning system is implemented, this argument will become + obsolete and will be removed. No user code should use this argument, as it + will be removed in the future. +select_columns_helper : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Vector +select_columns_helper internal_columns selector reorder on_problems warnings = case selector of + By_Name names matching_strategy -> + ## TODO [RW] Once the proper Warning support is implemented, this passing through of a mock `Warning_System` + instance will no longer be necessary. + split_result = here.split_to_distinct_and_duplicates names + distinct_names = split_result.first + duplicate_names = split_result.second + problems = if duplicate_names.is_empty then [] else + [Duplicate_Column_Selectors duplicate_names] + on_problems.attach_problems_before problems warnings <| + Warnings.map_warnings_and_errors here.promote_no_matches_to_missing_columns warnings warnings-> + Matching.match_criteria internal_columns distinct_names reorder=reorder name_mapper=(_.name) matching_strategy=matching_strategy on_problems=on_problems warnings=warnings + By_Index indices -> + partitioned_indices = indices.partition (here.is_index_valid internal_columns) + inbound_indices = partitioned_indices.first + oob_indices = partitioned_indices.second + + split_result = here.split_to_distinct_and_duplicates inbound_indices + duplicate_indices = split_result.second + distinct_indices = split_result.first + + resolved_indices = distinct_indices.map ix-> Pair ix (here.resolve_index internal_columns ix) + alias_split_result = here.split_to_distinct_and_duplicates resolved_indices .second + aliasing_indices = alias_split_result.second.map .first + good_indices = alias_split_result.first.map .second + + oob_problems = if oob_indices.is_empty then [] else + [Column_Indexes_Out_Of_Range oob_indices] + duplicate_problems = if duplicate_indices.is_empty then [] else + [Duplicate_Column_Selectors duplicate_indices] + aliasing_problems = if aliasing_indices.is_empty then [] else + [Input_Indices_Already_Matched aliasing_indices] + problems = oob_problems + duplicate_problems + aliasing_problems + + on_problems.attach_problems_before problems warnings <| case reorder of + True -> + here.select_indices_reordering internal_columns good_indices + False -> + here.select_indices_preserving_order internal_columns good_indices + By_Column columns -> + column_names = columns.map .name + new_selector = By_Name column_names (Matching.Exact case_sensitivity=True) + here.select_columns internal_columns new_selector reorder=reorder on_problems=on_problems warnings=warnings + +## PRIVATE + Converts the generic `No_Matches_Found` error to a more specific + `Missing_Input_Columns`. Any other errors are returned as-is. +promote_no_matches_to_missing_columns error = case error of + Matching.No_Matches_Found criteria -> Missing_Input_Columns criteria + _ -> + error + ## PRIVATE Selects element from the vector based on the given indices. From 81d65392aaabbd6e57509d7a6e0fb5f38dfd63b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 14:12:14 +0100 Subject: [PATCH 23/28] add examples --- .../src/Error/Problem_Behavior.enso | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso index 44ec106a7b34..5ace2176e0c3 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Error/Problem_Behavior.enso @@ -16,7 +16,8 @@ type Problem_Behavior Report the problem as a dataflow error and abort the operation type Report_Error - ## UNSTABLE + ## ADVANCED + UNSTABLE Attaches a problem to the given value according to the expected problem behavior. @@ -41,7 +42,8 @@ type Problem_Behavior case decorated_value of _ -> Error.throw problem - ## UNSTABLE + ## ADVANCED + UNSTABLE Attaches a problem to the given value according to the expected problem behavior. @@ -72,7 +74,8 @@ type Problem_Behavior Report_Error -> Error.throw problem - ## UNSTABLE + ## ADVANCED + UNSTABLE Attaches problems to the given value according to the expected problem behavior. @@ -88,6 +91,14 @@ type Problem_Behavior TODO [RW] the Warning_System argument is temporary, as the warning system is mocked until the real implementation is shipped. It will be removed soon. See: https://www.pivotaltracker.com/story/show/180901472 + + > Example + Perform pre-flight checks and then compute the actual result only if needed. + + problems = preflight_checks + problem_behavior.attach_problems_before problems Warnings.default <| + expensive_computation + attach_problems_before : Vector -> Warning_System -> Any -> Any attach_problems_before problems warnings ~decorated_value = case this of Ignore -> @@ -99,7 +110,8 @@ type Problem_Behavior if problems.is_empty then decorated_value else Error.throw problems.first - ## UNSTABLE + ## ADVANCED + UNSTABLE Attaches problems to the given value according to the expected problem behavior. @@ -114,6 +126,15 @@ type Problem_Behavior TODO [RW] the Warning_System argument is temporary, as the warning system is mocked until the real implementation is shipped. It will be removed soon. See: https://www.pivotaltracker.com/story/show/180901472 + + > Example + First compute a result and then, only if the computation has succeeded, + perform any postprocessing checks which may raise warnings/errors. + + result = compute_result + # TODO [RW] the underscore will be able to be removed once the `warnings` argument is deprecated, see above. + problem_behavior.attach_problems_after result _ Warnings.default <| + perform_post_process_checks_and_return_problems attach_problems_after : Any -> Vector -> Warning_System -> Any attach_problems_after decorated_value ~problems warnings = case this of Ignore -> From f1a176cebe36bfb36439533f9f662c4842cc4079 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 14:26:11 +0100 Subject: [PATCH 24/28] cr --- .../Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso | 6 ++++-- .../Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso | 2 +- .../lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso | 4 +++- .../lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso | 4 ++++ 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso index 3fe20cf5bda7..ed8ca08bd178 100644 --- a/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Database/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -12,7 +12,6 @@ from Standard.Database.Data.Column as Column_Module import all from Standard.Database.Data.Internal.IR import Internal_Column from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule from Standard.Table.Data.Table import No_Such_Column_Error -from Standard.Table.Data.Order_Rule as Order_Rule_Module import Order_Rule from Standard.Table.Data.Column_Selector as Column_Selector_Module import all from Standard.Base.Error.Problem_Behavior as Problem_Behavior_Module import all import Standard.Base.Error.Warnings @@ -115,7 +114,10 @@ type Table table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) - # TODO [RW] default arguments do not work on atoms, once this is fixed, the above should be replaced with just `Matching.Exact` + ## TODO [RW] default arguments do not work on atoms, once this is fixed, + the above should be replaced with just `Matching.Exact`. + See: https://github.com/enso-org/enso/issues/1600 + > Example Select columns matching a regular expression. diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso index 3936d0b7affa..31e0a629c3dd 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Column_Selector.enso @@ -1,6 +1,6 @@ from Standard.Base import all -from Standard.Table.Data.Matching import all +from Standard.Table.Data.Matching import Matching_Strategy, Exact ## Specifies a selection of columns from the table on which an operation is going to be performed. diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso index ef838f542dbd..7ecc03cb1d6a 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Data/Table.enso @@ -269,7 +269,9 @@ type Table table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) - # TODO [RW] default arguments do not work on atoms, once this is fixed, the above `Matching.Exact` can be removed as it is defaulted + ## TODO [RW] default arguments do not work on atoms, once this is fixed, + the above should be replaced with just `Matching.Exact`. + See: https://github.com/enso-org/enso/issues/1600 > Example Select columns matching a regular expression. diff --git a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso index 5e0f39b0e7e4..0ab1bf518195 100644 --- a/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso +++ b/distribution/lib/Standard/Test/0.2.32-SNAPSHOT/src/Main.enso @@ -163,6 +163,8 @@ Any.should_fail_with matcher frames_to_skip=0 = Arguments: - matcher: The expected type of dataflow error contained in `this`. + - frames_to_skip (optional, advanced): used to alter the location which is + displayed as the source of this error. > Example Assert that a compuation should return an error of a given type. @@ -244,6 +246,8 @@ Error.should_equal _ = Panic.throw (Matched_On_Error this) Arguments: - that: The value to compare `this` for equality with. - epsilon: The epislon for comparing two decimal numbers. + - frames_to_skip (optional, advanced): used to alter the location which is + displayed as the source of this error. > Example Compare two decimal values. From 0016f43b600258d2435a71ae67cfda04926b9792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 14:37:57 +0100 Subject: [PATCH 25/28] Add more test cases for regex matching Found a bug --- test/Table_Tests/src/Common_Table_Spec.enso | 29 ++++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/test/Table_Tests/src/Common_Table_Spec.enso b/test/Table_Tests/src/Common_Table_Spec.enso index d4897d2a269e..6592ec003472 100644 --- a/test/Table_Tests/src/Common_Table_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Spec.enso @@ -31,15 +31,17 @@ spec prefix table_builder supports_case_sensitive_columns = col3 = ["Baz", Integer, [7,8,9]] col4 = ["foo_1", Integer, [10,11,12]] col5 = ["foo_2", Integer, [13,14,15]] - table_builder [col1, col2, col3, col4, col5] + col6 = ["ab.+123", Integer, [16,17,18]] + col7 = ["abcd123", Integer, [19,20,21]] + table_builder [col1, col2, col3, col4, col5, col6, col7] expect_column_names names table = - table.columns . map .name . should_equal names + table.columns . map .name . should_equal names frames_to_skip=2 Test.specify "should work as shown in the doc examples" <| expect_column_names ["foo", "bar"] <| table.select_columns (By_Name ["bar", "foo"] (Matching.Exact True)) expect_column_names ["bar", "Baz", "foo_1", "foo_2"] <| table.select_columns (By_Name ["foo.+", "b.*"] (Matching.Regex Matching.Case_Insensitive)) - expect_column_names ["foo_2", "foo", "bar"] <| table.select_columns (By_Index [-1, 0, 1]) reorder=True + expect_column_names ["abcd123", "foo", "bar"] <| table.select_columns (By_Index [-1, 0, 1]) reorder=True column1 = table.at "foo_1" column2 = table.at "Baz" @@ -51,8 +53,15 @@ spec prefix table_builder supports_case_sensitive_columns = table_2 . at "bar" . to_vector . should_equal [4,5,6] table_2 . at "foo" . to_vector . should_equal [1,2,3] + Test.specify "should correctly handle regex matching" <| + expect_column_names ["foo"] <| table.select_columns (By_Name ["foo"] (Matching.Regex True)) + expect_column_names ["ab.+123", "abcd123"] <| table.select_columns (By_Name ["a.*"] (Matching.Regex True)) + expect_column_names ["ab.+123", "abcd123"] <| table.select_columns (By_Name ["ab.+123"] (Matching.Regex True)) + expect_column_names ["ab.+123"] <| table.select_columns (By_Name ["ab.+123"] (Matching.Exact True)) + expect_column_names ["abcd123"] <| table.select_columns (By_Name ["abcd123"] (Matching.Regex True)) + Test.specify "should allow negative indices" <| - expect_column_names ["foo", "bar", "foo_2"] <| table.select_columns (By_Index [-1, 0, 1]) + expect_column_names ["foo", "bar", "foo_2"] <| table.select_columns (By_Index [-3, 0, 1]) if supports_case_sensitive_columns then Test.specify "should correctly handle exact matches matching multiple names due to case insensitivity" <| @@ -68,10 +77,10 @@ spec prefix table_builder supports_case_sensitive_columns = expect_column_names ["bar", "foo", "foo_1", "foo_2"] <| table.select_columns (By_Name ["b.*", "f.+"] (Matching.Regex True)) reorder=True Test.specify "should correctly handle problems: out of bounds indices" <| - selector = By_Index [1, 0, 100, 200, 300] + selector = By_Index [1, 0, 100, -200, 300] action = table.select_columns selector warnings=_ on_problems=_ tester = expect_column_names ["foo", "bar"] - problems = [Column_Indexes_Out_Of_Range [100, 200, 300]] + problems = [Column_Indexes_Out_Of_Range [100, -200, 300]] Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: duplicate indices" <| @@ -82,10 +91,10 @@ spec prefix table_builder supports_case_sensitive_columns = Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: aliased indices" <| - selector = By_Index [0, -5, -4, 1] + selector = By_Index [0, -7, -6, 1] action = table.select_columns selector warnings=_ on_problems=_ tester = expect_column_names ["foo", "bar"] - problems = [Input_Indices_Already_Matched [-5, 1]] + problems = [Input_Indices_Already_Matched [-7, 1]] Problems.test_problem_handling action problems tester Test.specify "should correctly handle problems: duplicate names" <| @@ -137,7 +146,7 @@ spec prefix table_builder supports_case_sensitive_columns = problems = [Missing_Input_Columns ["hmmm"], No_Output_Columns] Problems.test_problem_handling action problems tester - action_2 = table.select_columns (By_Index [0, -5, 0, 100]) warnings=_ on_problems=_ - problems_2 = [Column_Indexes_Out_Of_Range [100], Duplicate_Column_Selectors [0], Input_Indices_Already_Matched [-5]] + action_2 = table.select_columns (By_Index [0, -7, 0, 100]) warnings=_ on_problems=_ + problems_2 = [Column_Indexes_Out_Of_Range [100], Duplicate_Column_Selectors [0], Input_Indices_Already_Matched [-7]] tester_2 = expect_column_names ["foo"] Problems.test_problem_handling action_2 problems_2 tester_2 From 0d550ae81a96283d41ad04546bfe17c9e0d6406a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 14:56:23 +0100 Subject: [PATCH 26/28] Fix Regex.Patter.matches to match full string --- .../src/Data/Text/Regex/Engine/Default.enso | 4 +- test/Table_Tests/src/Matching_Spec.enso | 59 ++++++++++--------- .../Data/Text/Default_Regex_Engine_Spec.enso | 5 ++ 3 files changed, 38 insertions(+), 30 deletions(-) diff --git a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Data/Text/Regex/Engine/Default.enso b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Data/Text/Regex/Engine/Default.enso index 4d995ff68b02..de5d1457a135 100644 --- a/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Data/Text/Regex/Engine/Default.enso +++ b/distribution/lib/Standard/Base/0.2.32-SNAPSHOT/src/Data/Text/Regex/Engine/Default.enso @@ -308,7 +308,7 @@ type Pattern Panic.throw Invalid_Bounds_Error _ -> do_match_mode mode 0 input.length - ## ADVANDED + ## ADVANCED Returns `True` if the input matches against the pattern described by `this`, otherwise `False`. @@ -327,7 +327,7 @@ type Pattern input = "aa" pattern.matches input matches : Text -> Boolean - matches input = case this.match input mode=Mode.First of + matches input = case this.match input mode=Mode.Full of Match _ _ _ -> True Vector.Vector _ -> True _ -> False diff --git a/test/Table_Tests/src/Matching_Spec.enso b/test/Table_Tests/src/Matching_Spec.enso index 4bdd59993b60..cbdc9377e39e 100644 --- a/test/Table_Tests/src/Matching_Spec.enso +++ b/test/Table_Tests/src/Matching_Spec.enso @@ -16,46 +16,49 @@ spec = Test.group 'Matching Helper' <| Once this is fixed, the tests should be updated accordingly. exact = Exact case_sensitivity=True regex = Regex case_sensitivity=True - Test.specify 'Should match a single name with a single exact criterion' <| - Matching.match_single_criterion "foo" "foo" exact . should_equal True - Matching.match_single_criterion "foo" "f.*" exact . should_equal False - Matching.match_single_criterion "foo" "Foo" exact . should_equal False - - Test.specify 'Should correctly handle Unicode folding with exact matching' <| - Matching.match_single_criterion '\u00E9' '\u0065\u{301}' exact . should_equal True - Matching.match_single_criterion 'é' '\u00E9' exact . should_equal True - Matching.match_single_criterion 'é' 'ę' exact . should_equal False - - Test.specify 'Should match a single name with a single regex criterion' <| - Matching.match_single_criterion "foo" "foo" regex . should_equal True - Matching.match_single_criterion "foo" "f.*" regex . should_equal True - Matching.match_single_criterion "foo" "F.*" regex . should_equal False - - Test.specify 'Should support case-insensitive matching' <| - Matching.match_single_criterion "foo" "F.*" (Regex case_sensitivity=Case_Insensitive) . should_equal True - Matching.match_single_criterion "foo" "Foo" (Exact case_sensitivity=Case_Insensitive) . should_equal True - - Matching.match_single_criterion "foo" "fF.*" (Regex case_sensitivity=Case_Insensitive) . should_equal False - Matching.match_single_criterion "foo" "Foos" (Exact case_sensitivity=Case_Insensitive) . should_equal False + Test.specify 'should match a single name with a single exact criterion' <| + Matching.match_single_criterion "foo" "foo" exact . should_be_true + Matching.match_single_criterion "foobar" "foo" exact . should_be_false + Matching.match_single_criterion "foo" "f.*" exact . should_be_false + Matching.match_single_criterion "foo" "Foo" exact . should_be_false + + Test.specify 'should correctly handle Unicode folding with exact matching' <| + Matching.match_single_criterion '\u00E9' '\u0065\u{301}' exact . should_be_true + Matching.match_single_criterion 'é' '\u00E9' exact . should_be_true + Matching.match_single_criterion 'é' 'ę' exact . should_be_false + + Test.specify 'should match a single name with a single regex criterion' <| + Matching.match_single_criterion "foo" "foo" regex . should_be_true + Matching.match_single_criterion "foobar" "foo" regex . should_be_false + Matching.match_single_criterion "foo" "f.*" regex . should_be_true + Matching.match_single_criterion "foo" "foo.*" regex . should_be_true + Matching.match_single_criterion "foo" "F.*" regex . should_be_false + + Test.specify 'should support case-insensitive matching' <| + Matching.match_single_criterion "foo" "F.*" (Regex case_sensitivity=Case_Insensitive) . should_be_true + Matching.match_single_criterion "foO" "FOo" (Exact case_sensitivity=Case_Insensitive) . should_be_true + + Matching.match_single_criterion "foo" "fF.*" (Regex case_sensitivity=Case_Insensitive) . should_be_false + Matching.match_single_criterion "foo" "Foos" (Exact case_sensitivity=Case_Insensitive) . should_be_false ## TODO this may not be how we want this to work, but this test is included to explicitly illustrate how the current implementation behaves in such corner cases - Matching.match_single_criterion "β" "B" (Exact case_sensitivity=Case_Insensitive) . should_equal False + Matching.match_single_criterion "β" "B" (Exact case_sensitivity=Case_Insensitive) . should_be_false - Test.specify 'Should match a list of names with a list of criteria, correctly handling reordering' <| + Test.specify 'should match a list of names with a list of criteria, correctly handling reordering' <| Matching.match_criteria ["foo", "bar", "baz"] ["baz", "foo"] reorder=True . should_equal ["baz", "foo"] Matching.match_criteria ["foo", "bar", "baz"] ["baz", "foo"] reorder=False . should_equal ["foo", "baz"] - Test.specify 'Should allow multiple matches to a single criterion (Regex)' <| + Test.specify 'should allow multiple matches to a single criterion (Regex)' <| Matching.match_criteria ["foo", "bar", "baz", "quux"] ["b.*"] reorder=True matching_strategy=regex . should_equal ["bar", "baz"] Matching.match_criteria ["foo", "bar", "baz", "quux"] ["b.*", "foo"] reorder=False matching_strategy=regex . should_equal ["foo", "bar", "baz"] - Test.specify 'Should include the object only with the first criterion that matched it, avoiding duplication' <| + Test.specify 'should include the object only with the first criterion that matched it, avoiding duplication' <| Matching.match_criteria ["foo", "bar", "baz", "zap"] [".*z.*", "b.*"] reorder=True matching_strategy=regex . should_equal ["baz", "zap", "bar"] Matching.match_criteria ["foo", "bar", "baz", "zap"] [".*z.*", "b.*"] reorder=False matching_strategy=regex . should_equal ["bar", "baz", "zap"] - Test.specify 'Should correctly handle criteria which did not match anything' <| + Test.specify 'should correctly handle criteria which did not match anything' <| action = Matching.match_criteria ["foo", "bar", "baz"] ["baz", "unknown_column"] reorder=True warnings=_ on_problems=_ tester = _.should_equal ["baz"] problems = [No_Matches_Found ["unknown_column"]] @@ -65,14 +68,14 @@ spec = Test.group 'Matching Helper' <| problems_2 = [No_Matches_Found ["unknown_column_1", "unknown_column_2"]] Problems.test_problem_handling action_2 problems_2 tester - Test.specify 'Should correctly work with complex object using a function extracting their names' <| + Test.specify 'should correctly work with complex object using a function extracting their names' <| pairs = [Pair "foo" 42, Pair "bar" 33, Pair "baz" 10, Pair "foo" 0, Pair 10 10] selected = [Pair "bar" 33, Pair "foo" 42, Pair "foo" 0] Matching.match_criteria pairs ["bar", "foo"] reorder=True name_mapper=_.first . should_equal selected Matching.match_criteria [1, 2, 3] ["2"] name_mapper=_.to_text . should_equal [2] - Test.specify 'Should correctly forward errors' <| + Test.specify 'should correctly forward errors' <| Matching.match_criteria (Error.throw Foo_Error) [] . should_fail_with Foo_Error Matching.match_criteria [] (Error.throw Foo_Error) . should_fail_with Foo_Error Matching.match_criteria [] [] (Error.throw Foo_Error) . should_fail_with Foo_Error diff --git a/test/Tests/src/Data/Text/Default_Regex_Engine_Spec.enso b/test/Tests/src/Data/Text/Default_Regex_Engine_Spec.enso index 833c103f9133..b5c5e4d8605d 100644 --- a/test/Tests/src/Data/Text/Default_Regex_Engine_Spec.enso +++ b/test/Tests/src/Data/Text/Default_Regex_Engine_Spec.enso @@ -96,6 +96,11 @@ spec = input = "aa ab abc a bc bcd" pattern.matches input . should_be_false + Test.specify "should check for full matches" <| + pattern = engine.compile "f.o" [] + pattern.matches "foo" . should_be_true + pattern.matches "foobar" . should_be_false + Test.group "The default regex engine's Pattern.match" <| engine = Default_Engine.new From bab6fb1138721d4d03d15c3eb608281f745cd0c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 15:19:21 +0100 Subject: [PATCH 27/28] "Fix" tests --- test/Tests/src/Data/Text_Spec.enso | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/Tests/src/Data/Text_Spec.enso b/test/Tests/src/Data/Text_Spec.enso index e587ac1f50f3..4cfd612ba7f9 100644 --- a/test/Tests/src/Data/Text_Spec.enso +++ b/test/Tests/src/Data/Text_Spec.enso @@ -155,14 +155,19 @@ spec = Test.specify "should be possible in dot_matches_newline mode" <| 'Foo\n'.matches "(....)" dot_matches_newline=True . should_be_true - Test.specify "should be possible in multiline mode" <| + multiline_matches_message = """ + This test does not make sense once we require matches to match the + whole string. The `multiline` parameter may not make sense for the + `matches` function. This should be revisited when Text library is + being redesigned. + Test.specify "should be possible in multiline mode" pending=multiline_matches_message <| text = """ Foo bar text.matches "^(...)$" multiline=True . should_be_true Test.specify "should be possible in comments mode" <| - "abcde".matches "(..) # Match two of any character" comments=True . should_be_true + "abcde".matches "(.....) # Match any five characters" comments=True . should_be_true Test.group "Regex finding" <| Test.specify "should be possible on text" <| From 0a090e1f61d4e70af70085dff5585ead1def7409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Wa=C5=9Bko?= Date: Mon, 31 Jan 2022 15:26:43 +0100 Subject: [PATCH 28/28] cr --- .../Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso | 2 -- 1 file changed, 2 deletions(-) diff --git a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso index 288ecf6d5d1c..112b452bfbab 100644 --- a/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso +++ b/distribution/lib/Standard/Table/0.2.32-SNAPSHOT/src/Internal/Table_Helpers.enso @@ -70,8 +70,6 @@ select_columns internal_columns selector reorder on_problems warnings = select_columns_helper : Vector -> Column_Selector -> Boolean -> Problem_Behavior -> Warnings.Warning_System -> Vector select_columns_helper internal_columns selector reorder on_problems warnings = case selector of By_Name names matching_strategy -> - ## TODO [RW] Once the proper Warning support is implemented, this passing through of a mock `Warning_System` - instance will no longer be necessary. split_result = here.split_to_distinct_and_duplicates names distinct_names = split_result.first duplicate_names = split_result.second