From 1c821e22cfc9dbeb349f2c033344509e7bcd7e48 Mon Sep 17 00:00:00 2001 From: James Dunkerley Date: Wed, 8 Feb 2023 22:17:00 +0000 Subject: [PATCH] Some fixed form the Anagrams experiment. (#5592) - Fixes the display of Date, Time_Of_Day and Date_Time so doesn't wrap. - Adjust serialization of large integer values for JS and display within table. - Workaround for issue with using `.lines` in the Table (new bug filed). - Disabled warning on no specified `separator` on `Concatenate`. Does not include fix for aggregation on integer values outside of `long` range. --- .../visualization/java_script/table.js | 7 ++++- .../lib/Standard/Base/0.0.0-dev/src/Data.enso | 2 +- .../0.0.0-dev/src/Data/Json/Extensions.enso | 5 ++++ .../org/enso/compiler/ParseStdLibTest.java | 1 + .../enso/table/aggregations/Concatenate.java | 2 +- .../org/enso/table/data/table/Column.java | 29 +++++++++++++++---- .../Aggregate_Spec.enso | 8 +++++ .../Table_Tests/src/In_Memory/Table_Spec.enso | 11 +++++-- 8 files changed, 55 insertions(+), 10 deletions(-) diff --git a/app/gui/view/graph-editor/src/builtin/visualization/java_script/table.js b/app/gui/view/graph-editor/src/builtin/visualization/java_script/table.js index 77c912ef507..608e4864634 100644 --- a/app/gui/view/graph-editor/src/builtin/visualization/java_script/table.js +++ b/app/gui/view/graph-editor/src/builtin/visualization/java_script/table.js @@ -183,10 +183,13 @@ class TableVisualization extends Visualization { let to_render = content if (content instanceof Object) { const type = content.type - if (type === 'Date') { + if (type === 'BigInt') { + to_render = BigInt(content.value) + } else if (type === 'Date') { to_render = new Date(content.year, content.month - 1, content.day) .toISOString() .substring(0, 10) + to_render = '' + to_render + '' } else if (type === 'Time_Of_Day') { const js_date = new Date( 0, @@ -200,6 +203,7 @@ class TableVisualization extends Visualization { to_render = js_date.toTimeString().substring(0, 8) + (js_date.getMilliseconds() === 0 ? '' : '.' + js_date.getMilliseconds()) + to_render = '' + to_render + '' } else if (type === 'Date_Time') { const js_date = new Date( content.year, @@ -213,6 +217,7 @@ class TableVisualization extends Visualization { to_render = js_date.toISOString().substring(0, 19).replace('T', ' ') + (js_date.getMilliseconds() === 0 ? '' : '.' + js_date.getMilliseconds()) + to_render = '' + to_render + '' } } result += '' + to_render + '' diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data.enso index 36c0f37cf9f..4a31e64d9d4 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data.enso @@ -158,7 +158,7 @@ fetch uri method=HTTP_Method.Get headers=[] parse=True = request = Request.new method uri parsed_headers response = HTTP.new.request request - if response.code.is_success.not then Error.throw (Request_Error.Error "Status Code" ("Request failed with status code: " + response.code + ". " + response.body.to_text)) else + if response.code.is_success.not then Error.throw (Request_Error.Error "Status Code" ("Request failed with status code: " + response.code.to_text + ". " + response.body.to_text)) else response_headers = response.headers content_type = response_headers.find if_missing=Nothing h-> "Content-Type".equals_ignore_case h.name if (parse == False) || (content_type == Nothing) then response else diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Json/Extensions.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Json/Extensions.enso index 9610ba27a9f..cf7c3f82690 100644 --- a/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Json/Extensions.enso +++ b/distribution/lib/Standard/Base/0.0.0-dev/src/Data/Json/Extensions.enso @@ -45,6 +45,11 @@ Number.to_js_object self = case self of Number -> JS_Object.from_pairs [["type", "Number"]] Integer -> JS_Object.from_pairs [["type", "Integer"]] Decimal -> JS_Object.from_pairs [["type", "Decimal"]] + _ : Integer -> + ## JS Safe Integer range -(2^53 - 1) to (2^53 - 1) + js_max_integer = 9007199254740991 + if self >= -js_max_integer && self < js_max_integer then self else + JS_Object.from_pairs [["type", "BigInt"], ["value", self.to_text]] _ -> self ## Converts the given value to a JSON serializable object. diff --git a/engine/runtime/src/test/java/org/enso/compiler/ParseStdLibTest.java b/engine/runtime/src/test/java/org/enso/compiler/ParseStdLibTest.java index dde4926bc82..fadd0ca120f 100644 --- a/engine/runtime/src/test/java/org/enso/compiler/ParseStdLibTest.java +++ b/engine/runtime/src/test/java/org/enso/compiler/ParseStdLibTest.java @@ -160,6 +160,7 @@ public final class ParseStdLibTest extends TestCase { // Files containing type expressions not supported by old parser. "Data/Index_Sub_Range.enso", "Data/Json.enso", + "Data/Json/Extensions.enso", "Data/List.enso", "Data/Pair.enso", "Data/Range.enso", diff --git a/std-bits/table/src/main/java/org/enso/table/aggregations/Concatenate.java b/std-bits/table/src/main/java/org/enso/table/aggregations/Concatenate.java index 6b6ddf25a9d..c3088c8db3a 100644 --- a/std-bits/table/src/main/java/org/enso/table/aggregations/Concatenate.java +++ b/std-bits/table/src/main/java/org/enso/table/aggregations/Concatenate.java @@ -33,7 +33,7 @@ public class Concatenate extends Aggregator { if (value == null || value instanceof String) { String textValue = toQuotedString(value, quote, separator); - if (quote.equals("") && textValue.contains(separator)) { + if (!separator.equals("") && quote.equals("") && textValue.contains(separator)) { this.addProblem(new UnquotedDelimiter(this.getName(), row, "Unquoted delimiter.")); } diff --git a/std-bits/table/src/main/java/org/enso/table/data/table/Column.java b/std-bits/table/src/main/java/org/enso/table/data/table/Column.java index f16a4704d86..ec15423ca01 100644 --- a/std-bits/table/src/main/java/org/enso/table/data/table/Column.java +++ b/std-bits/table/src/main/java/org/enso/table/data/table/Column.java @@ -11,6 +11,7 @@ import org.enso.table.data.mask.SliceRange; import org.enso.table.error.UnexpectedColumnTypeException; import org.graalvm.polyglot.Value; +import java.util.ArrayList; import java.util.BitSet; import java.util.List; @@ -104,8 +105,10 @@ public class Column { */ public static Column fromItems(String name, List items) { InferredBuilder builder = new InferredBuilder(items.size()); - for (Value item : items) { - Object converted = Polyglot_Utils.convertPolyglotValue(item); + // ToDo: This a workaround for an issue with polyglot layer. #5590 is related. + // to revert replace with: for (Value item : items) { + for (Object item : items) { + Object converted = item instanceof Value v ? Polyglot_Utils.convertPolyglotValue(v) : item; builder.appendNoGrow(converted); } var storage = builder.seal(); @@ -120,12 +123,28 @@ public class Column { * @return a column with given name and items */ public static Column fromRepeatedItems(String name, List items, int repeat) { + if (repeat < 1) { + throw new IllegalArgumentException("Repeat count must be positive."); + } + + if (repeat == 1) { + return fromItems(name, items); + } + var totalSize = items.size() * repeat; + + var values = new ArrayList(items.size()); + // ToDo: This a workaround for an issue with polyglot layer. #5590 is related. + // to revert replace with: for (Value item : items) { + for (Object item : items) { + Object converted = item instanceof Value v ? Polyglot_Utils.convertPolyglotValue(v) : item; + values.add(converted); + } + var builder = new InferredBuilder(totalSize); for (int i = 0; i < totalSize; i++) { - var item = items.get(i % items.size()); - var converted = Polyglot_Utils.convertPolyglotValue(item); - builder.appendNoGrow(converted); + var item = values.get(i % items.size()); + builder.appendNoGrow(item); } return new Column(name, builder.seal()); } diff --git a/test/Table_Tests/src/Common_Table_Operations/Aggregate_Spec.enso b/test/Table_Tests/src/Common_Table_Operations/Aggregate_Spec.enso index 1440d94b45f..1807088a680 100644 --- a/test/Table_Tests/src/Common_Table_Operations/Aggregate_Spec.enso +++ b/test/Table_Tests/src/Common_Table_Operations/Aggregate_Spec.enso @@ -1378,6 +1378,14 @@ spec setup = tester = expect_column_names ["Concatenate Text"] Problems.test_problem_handling action problems tester + Test.specify "should not fail if trying concatenate unquoted delimiters with no separator" <| + column = Concatenate "Text" separator="" + t = table_builder [["Text", ["A", "BC", "def"]]] + result = t.aggregate [column] on_problems=Report_Error + Problems.assume_no_problems result + result.column_names . should_equal ["Concatenate Text"] + result.at "Concatenate Text" . to_vector . should_equal ["ABCdef"] + Test.specify "should warn if can't compare value for Min or Max" <| [Problem_Behavior.Report_Error, Problem_Behavior.Report_Warning, Problem_Behavior.Ignore].each pb-> Test.with_clue "Problem_Behavior="+pb.to_text+" " <| err = table.aggregate [Maximum "Mixed"] on_problems=pb diff --git a/test/Table_Tests/src/In_Memory/Table_Spec.enso b/test/Table_Tests/src/In_Memory/Table_Spec.enso index 8bd47e08b0f..211cfc7f366 100644 --- a/test/Table_Tests/src/In_Memory/Table_Spec.enso +++ b/test/Table_Tests/src/In_Memory/Table_Spec.enso @@ -105,7 +105,7 @@ spec = pending_python_missing = if Polyglot.is_language_installed "python" . not then "Can't run Python tests, Python is not installed." - Test.specify "should also work with polyglot values coming from Python" pending=pending_python_missing <| + Test.specify "should work with polyglot values coming from Python" pending=pending_python_missing <| enso_dates = ["enso_dates", [Date.new 2022 8 27, Date.new 1999 1 1]] py_dates = ["py_dates", [py_make_date 2022 8 27, py_make_date 1999 1 1]] py_objects = ["py_objects", [py_make_object "a" "b", py_make_object "foo" "bar"]] @@ -115,7 +115,7 @@ spec = (table.at "enso_dates" == table.at "py_dates").to_vector . should_equal [True, True] - Test.specify "should also work with polyglot values coming from JS" <| + Test.specify "should work with polyglot values coming from JS" <| enso_dates = ["enso_dates", [Date.new 2022 8 27, Date.new 1999 1 1]] js_dates = ["js_dates", [js_make_date 2022 8 27, js_make_date 1999 1 1]] js_objects = ["js_objects", [js_make_object "a" "b", js_make_object "foo" "bar"]] @@ -130,6 +130,13 @@ spec = (js_converted_dates == table.at "enso_dates").to_vector . should_equal [True, True] (enso_date_times == table.at "js_dates").to_vector . should_equal [True, True] + Test.specify "should work with a Text value split into lines" <| + ## This tests verifies an issue with passing through a `List` to the table. + words = 'The\nquick\nbrown\nfox\njumps\nover\nthe\nlazy\ndog'.lines + table = Table.new [["words", words]] + table.at "words" . storage_type . should_equal Storage.Text + table.at "words" . to_vector . should_equal words + Test.specify "should handle Unicode normalization when accessing table columns" <| col1 = ['s\u0301ciana', [1, 2, 3]] col2 = ['café', [4, 5, 6]]