IDE uses new visualization API #3661

4e6 · 2022-08-22T07:54:47Z

Pull Request Description

PR integrates new visualization API that uses method pointers instead of lambdas as visualization expressions.

Important Notes

Checklist

Please include the following checklist in your PR:

The documentation has been updated if necessary.
All code conforms to the
Scala,
Java,
and
Rust
style guides.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed: Enso GUI was tested when built using BOTH
  ./run ide build and ./run ide watch.

JaroslavTulach

#3655 needs an ability to invoke the Enso part of the visualization with arguments. How can one do that with setPreprocessor(module, method)?

app/gui/docs/product/visualizations.md

JaroslavTulach · 2022-08-22T08:26:15Z

app/gui/docs/product/visualizations.md

+  port. In case it is not connected, the visualization becomes an interactive
+  widget allowing the user to specify data. For example, a map visualization
+  will allow the user to manually pick locations. After each change, the new
+  locations will be sent to the output port. Under the hood, widgets are


That almost sounds like "dynamic widgets" to me!

JaroslavTulach · 2022-08-22T08:27:35Z

app/gui/docs/product/visualizations.md

-    data to visualization. If not called, a default unspecified code is used
+  - #### Method `setPreprocessor(module,method)`
+    Set an Enso method which will be evaluated on the server-side before sending
+    data to visualization. If not called, a default unspecified method is used


default unspecified? That's a strange formulation in a specification of the behavior ;-)

Why? Specification explicitly opts-out from defining this.
There'll be some code present, but custom visualization should not make any assumptions about it. Thanks to that, we can adjust it without breaking public API.

JaroslavTulach · 2022-08-22T08:29:18Z

app/gui/docs/product/visualizations.md

-where visualization is defined - you may use any symbol defined or imported in
-that module.
+The code will be evaluated in the context of the module where the preprocessor
+method is defined - you may use any symbol defined or imported in that module.


How can one pass arguments to the method? #3655 needs to be able to invoke the Enso engine part of the visualization with different arguments.

I'd expect that arguments are part of the expression, as the arguments change we update the expression.

Ah, it was removed. Then some mechanism for arguments is necessary,

Co-authored-by: Jaroslav Tulach <[email protected]>

app/gui/controller/double-representation/src/module.rs

app/gui/controller/engine-protocol/src/language_server/types.rs

app/gui/src/presenter/graph/visualization/manager.rs

farmaazon · 2022-08-22T09:49:53Z

app/gui/view/graph-editor/src/component/visualization/foreign/java_script/visualization.js

-     * When visualization is attached to a node, each time a new value is produced from node,
-     * the preprocessor shall be invoked with it. Result such call shall be serialized and
-     * transported to visualization by invoking onDataReceived(data) method.


I miss such detailed documentation in the new version.

We have a special documentation page for contributors wanting to create new visualizations, and these docs AFAIK refers to this file for specific documentation of JS methods they can use inside their visualizations.

I updated the doc with more details and examples. Should be better now

app/gui/src/model/execution_context.rs

mwu-tow · 2022-08-22T10:44:16Z

app/gui/docs/product/visualizations.md

-where visualization is defined - you may use any symbol defined or imported in
-that module.
+The code will be evaluated in the context of the module where the preprocessor
+method is defined - you may use any symbol defined or imported in that module.


Ah, it was removed. Then some mechanism for arguments is necessary,

JaroslavTulach · 2022-08-22T11:34:08Z

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

@@ -44,7 +43,7 @@ class Histogram extends Visualization {

    constructor(data) {
        super(data)
-        this.setPreprocessor('process_to_json_text', 'Standard.Visualization.Histogram')
+        this.setPreprocessor('Standard.Visualization.Histogram', 'process_to_json_text')


Is there any reason to revert the order of arguments?

I like it more. Imagine calling a method by a fully qualified name Foo.Bar.Baz.quux a b c. The setPreprocessor method just splits the call into three sections (arguments are optional in this case)

Please double-check that all calls to this have been update. This is a change that can easily cause issues as the arguments have the same type and this will only fail at runtime.

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

…sions

JaroslavTulach

Thanks. This should be good enough for the scatterplot visualization.

JaroslavTulach · 2022-08-24T04:30:11Z

app/gui/view/graph-editor/src/component/visualization/foreign/java_script/visualization.js

+    setArguments(...args) {
+        if (args !== this.__preprocessorArguments__) {
+            this.__preprocessorArguments__ = args
+            this.__emitPreprocessorChange__()


Ideally changing only the setArguments shall be a lightweight operation. Something that invokes already existing code on the engine side, without re-parsing the code. But I guess this is provided by definition when using method pointers...

JaroslavTulach · 2022-08-24T04:33:39Z

...truments/src/test/scala/org/enso/interpreter/test/instrument/RuntimeVisualizationsTest.scala

@@ -2198,7 +2204,176 @@ class RuntimeVisualizationsTest
                "Enso_Test.Test.Visualisation",
                "Enso_Test.Test.Visualisation",
                "incAndEncode"
+              ),
+              Vector("2", "3")


What other types are supported? Text? Booleans True, False? Complex atoms? Function invocations?

An argument can be an arbitrary Enso expression that can be interpreted in the context of the visualization module

4e6 · 2022-08-24T12:29:11Z

app/gui/src/presenter/graph/visualization/manager.rs

@@ -94,6 +75,7 @@ pub enum Notification {

 /// Describes the state of the visualization on the Language Server.
 #[derive(Clone, Debug, PartialEq)]
+#[allow(clippy::large_enum_variant)]


Clippy complains about the size of enum variant. I though that it is safe to ignore because most of the variants have comparable sizes (the size of Visualization struct). And the variant having two Visualization fields is the one that Clippy complains about.

app/gui/src/model/execution_context.rs

MichaelMauderer · 2022-08-25T09:21:21Z

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

@@ -44,7 +43,7 @@ class Histogram extends Visualization {

    constructor(data) {
        super(data)
-        this.setPreprocessor('process_to_json_text', 'Standard.Visualization.Histogram')
+        this.setPreprocessor('Standard.Visualization.Histogram', 'process_to_json_text')


Please double-check that all calls to this have been update. This is a change that can easily cause issues as the arguments have the same type and this will only fail at runtime.

4e6 · 2022-08-25T09:46:00Z

Please double-check that all calls to this have been update. This is a change that can easily cause issues as the arguments have the same type and this will only fail at runtime.

I'm pretty sure I updated all of them. Unless grep failed me 😅

app/gui/controller/engine-protocol/src/language_server/tests.rs

app/gui/src/model/execution_context.rs

app/gui/docs/product/visualizations.md

4e6 added 4 commits August 19, 2022 12:32

feat: method pointer vis

de6809a

feat: default method pointer visualization

396e3b4

update js

bd73e1c

fix: tests

af3d934

4e6 self-assigned this Aug 22, 2022

4e6 requested review from jdunkerley, radeusgd, JaroslavTulach, MichaelMauderer, wdanilo, farmaazon and mwu-tow as code owners August 22, 2022 07:54

JaroslavTulach requested changes Aug 22, 2022

View reviewed changes

4e6 and others added 3 commits August 22, 2022 11:43

cleanup

8218bf7

Update app/gui/docs/product/visualizations.md

6bc6d01

Co-authored-by: Jaroslav Tulach <[email protected]>

run format

1376ba6

JaroslavTulach mentioned this pull request Aug 22, 2022

Lazy scatterplot for Vector & Table #3655

Merged

5 tasks

farmaazon reviewed Aug 22, 2022

View reviewed changes

mwu-tow reviewed Aug 22, 2022

View reviewed changes

JaroslavTulach reviewed Aug 22, 2022

View reviewed changes

app/gui/view/graph-editor/src/builtin/visualization/java_script/scatterPlot.js Outdated Show resolved Hide resolved

4e6 added 4 commits August 23, 2022 12:26

add: runtime support for positional arguments in visualization expres…

6f44e14

…sions

add: ls support for positional arguments in visualization expressions

68f707a

misc: review comments

0d24bf9

add: ide positional arguments

861ec59

4e6 requested a review from kazcw as a code owner August 23, 2022 15:50

4e6 added 4 commits August 23, 2022 21:21

misc: cleanup

869f80a

Merge branch 'develop' into wip/db/lazy-vis-ide

906645b

update scatterPlot visualization

b2f5f74

misc: javafmt

c78f339

misc: changelog

338aac1

4e6 requested a review from JaroslavTulach August 23, 2022 20:00

JaroslavTulach approved these changes Aug 24, 2022

View reviewed changes

4e6 added 3 commits August 24, 2022 12:50

test: fix

e6ab9c8

doc: update

1a5c9f6

fix: lint

0600286

4e6 commented Aug 24, 2022

View reviewed changes

4e6 added 2 commits August 24, 2022 17:52

fix: test

af756e4

misc: cleanup

9941681

mwu-tow reviewed Aug 25, 2022

View reviewed changes

app/gui/src/model/execution_context.rs Outdated Show resolved Hide resolved

4e6 added 2 commits August 25, 2022 09:31

misc: refactoring

76c69a6

Merge branch 'develop' into wip/db/lazy-vis-ide

949e87f

MichaelMauderer reviewed Aug 25, 2022

View reviewed changes

4e6 added 2 commits August 25, 2022 17:30

misc: fix modify visualization request

1c26a90

misc: review comments

0c4efc7

mwu-tow approved these changes Sep 1, 2022

View reviewed changes

app/gui/controller/engine-protocol/src/language_server/tests.rs Outdated Show resolved Hide resolved

app/gui/src/model/execution_context.rs Outdated Show resolved Hide resolved

app/gui/docs/product/visualizations.md Outdated Show resolved Hide resolved

4e6 added 3 commits September 1, 2022 13:24

Merge branch 'develop' into wip/db/lazy-vis-ide

8353df2

misc: review comments

7f34282

misc: bump wasm-size-limit

020b0b1

4e6 merged commit de0a231 into develop Sep 1, 2022

4e6 deleted the wip/db/lazy-vis-ide branch September 1, 2022 12:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IDE uses new visualization API #3661

IDE uses new visualization API #3661

4e6 commented Aug 22, 2022 •

edited

Loading

JaroslavTulach left a comment

JaroslavTulach Aug 22, 2022

JaroslavTulach Aug 22, 2022

mwu-tow Aug 22, 2022

JaroslavTulach Aug 22, 2022

mwu-tow Aug 22, 2022

mwu-tow Aug 22, 2022

farmaazon Aug 22, 2022

4e6 Aug 24, 2022

mwu-tow Aug 22, 2022

JaroslavTulach Aug 22, 2022

4e6 Aug 24, 2022 •

edited

Loading

MichaelMauderer Aug 25, 2022

JaroslavTulach left a comment

JaroslavTulach Aug 24, 2022

JaroslavTulach Aug 24, 2022

4e6 Aug 24, 2022 •

edited

Loading

4e6 Aug 24, 2022 •

edited

Loading

MichaelMauderer Aug 25, 2022

4e6 commented Aug 25, 2022

IDE uses new visualization API #3661

IDE uses new visualization API #3661

Conversation

4e6 commented Aug 22, 2022 • edited Loading

Pull Request Description

Important Notes

Checklist

JaroslavTulach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4e6 Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaroslavTulach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4e6 Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

4e6 Aug 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

4e6 commented Aug 25, 2022

4e6 commented Aug 22, 2022 •

edited

Loading

4e6 Aug 24, 2022 •

edited

Loading

4e6 Aug 24, 2022 •

edited

Loading

4e6 Aug 24, 2022 •

edited

Loading