Table Visualization and display text changes. #6382

jdunkerley · 2023-04-21T14:47:53Z

Pull Request Description

Missing tests from number parsing.
Fix type signature on some warning methods.
Fix warnings on Standard.Database.Data.Table.parse_values.
Added test for Nothing and empty string on use_first_row_as_names.
New API for Number.format taking a simple format string and Locale.
Add ellipsis to truncated Text.to_display_text.
Adjusted built-in to_display_text for numbers to not include type (but also to display BigInteger as value).
Remove Noise.Generator interface type.
Json: Added to_display_text to JS_Object.
Time: Added to_display_text for Date, Time_Of_Day, Date_Time, Duration and Period.
Text: Added to_display_text to Locale, Case_Sensitivity, Encoding, Text_Sub_Range, Span, Utf_16_Span.
System: Added to_display_text to File, File_Permissions, Process_Result and Exit_Code.
Network: Added to_display_text to URI, HTTP_Status_Code and Header.
Added to_display_text to Maybe, Regression, Pair, Range, Filter_Condition.
Added support for to_js_object and to_display_text to Random_Number_Generator.
Verified all error types have to_display_text.
Removed BigInt, Date, Date_Time and Time_Of_Day JS based rendering as using to_display_text now.
Added support for rendering nested structures in the table viz.

Adjusted Viz

Custom Values Rendering:

Nested Table:

Variable size Matrix view:

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

radeusgd · 2023-04-25T14:41:43Z

app/gui/view/graph-editor/src/builtin/visualization/java_script/table.js

            if (content instanceof Object) {
                const type = content.type
-                if (type === 'BigInt') {
-                    return BigInt(content.value)
-                } else if (content['_display_text_']) {
+                if (content['_display_text_']) {
                    return content['_display_text_']
-                } else if (type === 'Date') {
-                    return new Date(content.year, content.month - 1, content.day)
-                        .toISOString()
-                        .substring(0, 10)


IMO this is so much better now that this part of the logic is handled by Enso 👍

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Filter_Condition.enso

GregoryTravis · 2023-04-25T15:00:12Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Pair.enso

+    to_display_text self =
+        first = self.first.to_display_text
+        second = self.second.to_display_text
+        if first.length + second.length < 75 then "Pair(" + first + ", " + second + ")" else


It might be nice to eventually have helpers for constructing these strings.

JaroslavTulach

Massive PR. I left few comments there and elsewhere, but probably on the .java ones deserve some attention.

...me/src/main/java/org/enso/interpreter/node/expression/builtin/text/AnyToDisplayTextNode.java

.../main/java/org/enso/interpreter/node/expression/builtin/text/util/TypeToDisplayTextNode.java

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Pair.enso

JaroslavTulach · 2023-04-25T15:30:22Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Text/Text_Sub_Range.enso

+    ## PRIVATE
+       Convert to a display representation of this Index_Sub_Range.
+    to_display_text : Text
+    to_display_text self = case self of


There shall be generic implementation for these types with few atoms that would fit the default needs. At least I see some pattern that could be handled in a generic way via Meta or in the engine itself.

distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP/HTTP_Status_Code.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Random.enso

farmaazon

Approving table.js

Add JSON support for Random Number Generator.

Add `to_display_text` to Interval and Index_Sub_Range.

radeusgd · 2023-04-26T10:51:44Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Text/Span.enso

+    ## PRIVATE
+       Convert to a display representation of this Span.
+    to_display_text : Text
+    to_display_text self = self.text


Hm, shouldn't we indicate that this is a Span and not just regular Text?

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Date_Time.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Duration.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Time/Period.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Errors/Common.enso

distribution/lib/Standard/Base/0.0.0-dev/src/Network/URI.enso

distribution/lib/Standard/Base/0.0.0-dev/src/System/File/File_Permissions.enso

distribution/lib/Standard/Base/0.0.0-dev/src/System/Process/Exit_Code.enso

distribution/lib/Standard/Base/0.0.0-dev/src/System/Process/Process_Builder.enso

radeusgd · 2023-04-26T11:22:08Z

distribution/lib/Standard/Database/0.0.0-dev/src/Data/Table.enso

@@ -1497,6 +1497,7 @@ type Table
       suffixing strategy.
    parse_to_columns : Text | Integer -> Text -> Case_Sensitivity -> Boolean -> Problem_Behavior -> Table
    parse_to_columns self column pattern="." case_sensitivity=Case_Sensitivity.Sensitive parse_values=True on_problems=Report_Error =
+        _ = [column, pattern, case_sensitivity, parse_values, on_problems]


Also adding this in my PR 😅
I think it would be cool to have the --warnings-as-errors option to enable it on CI. Shall we create a ticket?

radeusgd · 2023-04-26T11:24:27Z

test/Table_Tests/src/In_Memory/Table_Spec.enso

-            expect_column_names ["H", "1980-01-01", "1", "5.3", "True"] <| table.use_first_row_as_names
+            c_5 = ['ZZ', [Nothing, False, True]]
+            table = Table.new [c_0, c_00, c_1, c_2, c_3, c_4, c_5]
+            expect_column_names ["H", "Column_1", "1980-01-01", "1", "5.3", "True", "Column_2"] table.use_first_row_as_names


I'm wondering if we should actually attach some warning in this case?

It does but it also seems to error when rendering so will look at that.

Then we should have this test checking this warning.

- Change `UTI` to `URI`. - Adjust format for Period and Duration. - Add support for polyglot objects in `No_Such_Method` and `No_Such_Field`. Fix some of the failing tests.

...me/src/main/java/org/enso/interpreter/node/expression/builtin/text/AnyToDisplayTextNode.java

radeusgd

The changes look very good to me.

I think we should add some tests for the to_display_text methods, especially ones doing formatting of numbers/dates/times - as these are not that trivial. We had experience of easily breaking to_display_text when refactoring. This is really problematic as if we don't detect it, we can very easily introduce regressions into the IDE display that may be noticed too late.

I'm ok with not having tests in this PR as we want to move forward and I assume it was tested manually, but I think in that case we should plan a follow up ticket to add these tests - as we definitely want these to_display_text to work correctly and avoid any regressions there.

- Shuffle File_Permission order. - Handle polyglot scenario for missing field and methods. - Tests for Date_Time to_display_text. - Truffle boundaries.

* develop: Passing events to sub-widgets in List Editor and refactoring of the slider component. (#6433) Revert "Cloud/desktop mode switcher (#6308)" (#6444) Widgets integrated with graph nodes (#6347) Table Visualization and display text changes. (#6382) Skip redundant compilations (#6436) Add parse extensions to Text type. #6330 (#6404) Cloud/desktop mode switcher (#6308) Replace Table should_equal with should_equal_verbose (#6405) Rollback event handling changes for the mouse (#6396) Dashboard directory interactivity (#6279) Ability to change the execution environment between design and live. (#6341) Implementation of elements adding to List Editor and a lot of internal API (#6390) Drop method exported from WASM + removing leaks. (#6365) Turn null into UnexpectedExpression when Union type is incomplete (#6415)

jdunkerley linked an issue Apr 21, 2023 that may be closed by this pull request

Table visualization incorrectly renders custom Enso objects #5480

Closed

jdunkerley changed the title ~~Table Visualization and default text changes.~~ Table Visualization and display text changes. Apr 21, 2023

jdunkerley force-pushed the wip/jd/5480-table-visualization-incorrectly-renders-custom-enso-objects branch 2 times, most recently from 128590a to 9df5145 Compare April 25, 2023 14:29

jdunkerley marked this pull request as ready for review April 25, 2023 14:32

jdunkerley requested review from radeusgd, 4e6, JaroslavTulach, hubertp, MichaelMauderer, wdanilo, farmaazon and kazcw as code owners April 25, 2023 14:32

radeusgd reviewed Apr 25, 2023

View reviewed changes

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Index_Sub_Range.enso Outdated Show resolved Hide resolved

jdunkerley added the CI: No changelog needed Do not require a changelog entry for this PR. label Apr 25, 2023

GregoryTravis approved these changes Apr 25, 2023

View reviewed changes

JaroslavTulach approved these changes Apr 25, 2023

View reviewed changes

farmaazon approved these changes Apr 25, 2023

View reviewed changes

enso-bot bot mentioned this pull request Apr 26, 2023

NPE from union type syntax error #6401

Closed

hubertp approved these changes Apr 26, 2023

View reviewed changes

jdunkerley force-pushed the wip/jd/5480-table-visualization-incorrectly-renders-custom-enso-objects branch from 21f8428 to 26e21d4 Compare April 26, 2023 09:14

jdunkerley added 8 commits April 26, 2023 11:07

Tests that weren't committed.

dbd3b1e

Fix type signatures.

f54ecfe

Update Number.format API.

73afdf5

Add JSON support for Random Number Generator.

Add to_display_text for Time types.

bf3c819

Fix couple of missing imports.

3a84a7d

Add `to_display_text` to Interval and Index_Sub_Range.

Add ellipsis if display text is truncated.

5c0573f

Remove Generator interface.

5b271ac

Add to_display_text to JS_Object.

a0a0358