Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@widgets for extension functions of Table may not be found #6955

Closed
radeusgd opened this issue Jun 5, 2023 · 22 comments · Fixed by #7115
Closed

@widgets for extension functions of Table may not be found #6955

radeusgd opened this issue Jun 5, 2023 · 22 comments · Fixed by #7115
Assignees
Labels
Milestone

Comments

@radeusgd
Copy link
Member

radeusgd commented Jun 5, 2023

As experienced in #6925, the primary_key annotation on select_into_database_table operation seems to be not registered.

Actual behaviour

image

The widget on primary_key is a Vector Editor but it only allows to add a Nothing, not the actual column names as expected. Checking Widgets.get_widget_json we can see that the annotation for @primary_key seems to not be applied for some reason.

Expected behaviour

The "primary_key" widget JSON should not be null - it is defined to be Widget_Helpers.make_column_name_vector_selector.

The above should then allow the vector editor to display the proper column selector dropdown.

Context

We checked if this may be due to the fact that select_into_database_table was defined on renamed types:

import Standard.Table.Data.Table as In_Memory_Table

...

@primary_key ...
In_Memory_Table.select_into_database_table : ...
In_Memory_Table.select_into_database_table self ... primary_key ... =
    ...

but we split the extensions file into separate versions for in-memory and Database Tables, allowing to get rid of the renames and the issue persisted, so it is probably something else.

@jdunkerley
Copy link
Member

The method is an extension in the Database module.

When the IDE calls through it call with the type and module name correctly and gets passed onto Meta.get_annotation but the methodFunction cannot be resolved.

@jdunkerley jdunkerley removed their assignment Jun 6, 2023
@jdunkerley jdunkerley moved this from ❓New to 📤 Backlog in Issues Board Jun 6, 2023
@jdunkerley jdunkerley added this to the Beta Release milestone Jun 6, 2023
@jdunkerley jdunkerley removed the triage label Jun 6, 2023
@JaroslavTulach
Copy link
Member

What is the program to test on? What shall I search for in that program? I tried to create something, but it doesn't work:

obrazek

@radeusgd
Copy link
Member Author

radeusgd commented Jun 7, 2023

What is the program to test on? What shall I search for in that program? I tried to create something, but it doesn't work:

obrazek

Looks like a typo in SQLite - note the single L.

@radeusgd
Copy link
Member Author

radeusgd commented Jun 7, 2023

Also note that the select_into_database_table operation was added pretty recently, in #6925, so ensure that you've got a recent build.

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Jun 7, 2023

Thank you for spotting the typo. I got over it. Now I cannot use Widgets:

Image

What's your import statement? I have no idea how you code works. I had to:

enso$ git diff
diff --git distribution/lib/Standard/Visualization/0.0.0-dev/src/Main.enso distribution/lib/Standard/Visualization/0.0.0-dev/src/Main.enso
index 7dbda46d96..ca56cf6f68 100644
--- distribution/lib/Standard/Visualization/0.0.0-dev/src/Main.enso
+++ distribution/lib/Standard/Visualization/0.0.0-dev/src/Main.enso
@@ -9,3 +9,6 @@ from project.File_Upload export file_uploading
 export project.Id.Id
 export project.Helpers
 
+import project.Widgets
+export project.Widgets
+

I wish bugreports had simple steps to reproduce. But I think I see the problem now.

@enso-bot
Copy link

enso-bot bot commented Jun 8, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-07):

Progress: - failed to reproduce: #6955 (comment)

Next Day: Bugfixing & Runtime Type Checks

Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

@JaroslavTulach
Copy link
Member

JaroslavTulach commented Jun 8, 2023

This is a dirty fix:

enso$ git diff engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetAnnotationNode.java
diff --git engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetAnnotationNode.java engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetAnnotationNode.java
index bbd28da286..efc90d9e26 100644
--- engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetAnnotationNode.java
+++ engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/meta/GetAnnotationNode.java
@@ -1,7 +1,16 @@
 package org.enso.interpreter.node.expression.builtin.meta;
 
+import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.Truffle;
+import com.oracle.truffle.api.dsl.Cached;
+import com.oracle.truffle.api.dsl.Specialization;
+import com.oracle.truffle.api.frame.FrameInstance;
+import com.oracle.truffle.api.frame.VirtualFrame;
+import com.oracle.truffle.api.library.CachedLibrary;
+import com.oracle.truffle.api.nodes.Node;
 import org.enso.interpreter.dsl.BuiltinMethod;
 import org.enso.interpreter.node.BaseNode;
+import org.enso.interpreter.node.EnsoRootNode;
 import org.enso.interpreter.node.callable.thunk.ThunkExecutorNode;
 import org.enso.interpreter.node.expression.builtin.text.util.ExpectStringNode;
 import org.enso.interpreter.runtime.EnsoContext;
@@ -13,12 +22,6 @@ import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
 import org.enso.interpreter.runtime.scope.ModuleScope;
 import org.enso.interpreter.runtime.state.State;
 
-import com.oracle.truffle.api.CompilerDirectives;
-import com.oracle.truffle.api.dsl.Cached;
-import com.oracle.truffle.api.dsl.Specialization;
-import com.oracle.truffle.api.frame.VirtualFrame;
-import com.oracle.truffle.api.library.CachedLibrary;
-
 @BuiltinMethod(
     type = "Meta",
     name = "get_annotation",
@@ -43,6 +46,15 @@ public abstract class GetAnnotationNode extends BaseNode {
     Type targetType = types.getType(target);
     ModuleScope scope = targetType.getDefinitionScope();
     Function methodFunction = scope.lookupMethodDefinition(targetType, methodName);
+    if (methodFunction == null) {
+      methodFunction = Truffle.getRuntime().iterateFrames((FrameInstance fi) -> {
+        if (fi.getCallNode() instanceof Node call && call.getRootNode() instanceof EnsoRootNode ensoRoot) {
+          return ensoRoot.getModuleScope().lookupMethodDefinition(targetType, methodName);
+        } else {
+          return null;
+        }
+      });
+    }
     if (methodFunction != null) {
       String parameterName = expectStringNode.execute(parameter);
       Annotation annotation = methodFunction.getSchema().getAnnotation(parameterName);

at least with this fix I get proper result from:

from Standard.Base import all
from Standard.Base.Data.Boolean import Boolean
from Standard.Base.Data.Text.Location import Location
from Standard.Database.Connection import Connection_Details
from Standard.Table import all
from Standard.Database import all
from Standard.AWS import all
from Standard.Visualization import Widgets

main =
    operator17 = Database.connect (SQLite location=In_Memory)
    operator18 = Table.new [ ["Y", ['a', 'b', 'c']] ]
    operator19 = operator18.select_into_database_table operator17  temporary=Boolean.True
    operator20 = Widgets.get_widget_json operator18 "select_into_database_table" [ "primary_key" ]
    operator20

@JaroslavTulach JaroslavTulach moved this from 📤 Backlog to ⚙️ Design in Issues Board Jun 8, 2023
@enso-bot
Copy link

enso-bot bot commented Jun 9, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-08):

Progress: - dirty "fix" found for #6955 (comment)

Next Day: Bugfixing & Runtime Type Checks

Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

@JaroslavTulach JaroslavTulach changed the title Widgets not registering at an extension function for Table @widgets for extension functions of Table may not be found Jun 10, 2023
@enso-bot
Copy link

enso-bot bot commented Jun 10, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-09):

Progress: - test cases for Meta.get_annotation: #7000

Next Day: Meta.get_annotations

GitHub
We currently have literal support for Number and Text. We have previously spoken about adding ``` (backtick) to be identified as an expression (so we can make it explicit rather than implied). Foll...
Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

@enso-bot
Copy link

enso-bot bot commented Jun 12, 2023

Jaroslav Tulach reports a new STANDUP for the last Saturday (2023-06-10):

Progress: - automatic conversions of arguments: #7009

Next Day: Meta.get_annotations

Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

@enso-bot
Copy link

enso-bot bot commented Jun 13, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-12):

Progress: - Meta.Function proposal: 5d87c9c

Next Day: Meta.get_annotations

@enso-bot
Copy link

enso-bot bot commented Jun 15, 2023

Jaroslav Tulach reports a new 🔴 DELAY for yesterday (2023-06-14):

Summary: There is 7 days delay in implementation of the @widgets for extension functions of Table may not be found (#6955) task.
It will cause 7 days delay for the delivery of this weekly plan.

Still investigating

Delay Cause: The issue crosscuts all the layers we have IDE, LS, compiler. I am still investigating what the proper solution should be. Design and discussion with owners of other parts of the system needed.

Possible solutions: The most promising solution right now seems to be to let IdExecutionInstrument provide access to instrumented ModuleScope to the visualization. Whether that can solve the problem remains to be seen.

@enso-bot
Copy link

enso-bot bot commented Jun 19, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-18):

Progress: - Saturday & Sunday coding

Next Day: Meta.get_annotations

@enso-bot
Copy link

enso-bot bot commented Jun 20, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-19):

Progress: - debugging QueryData::new & request_widget

Next Day: Meta.get_annotations

@JaroslavTulach
Copy link
Member

I am trying new approach to propagate the ModuleScope via State. It doesn't work, because of

The state is lost in Vector.newFromFunction which is using InteropLibrary to invoke a Function and Function.java:190 is using emptyState() instead of provided state in @ExportMessage and Execute message handler.

This was referenced Jun 20, 2023
@enso-bot
Copy link

enso-bot bot commented Jun 21, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-20):

Progress: - Vector.map & State: #6656 (comment)

Next Day: Meta.get_annotations

Google Docs
Spring 2023 | 5th meeting: 3:15pm – 4pm, Join with Google Meet 4th meeting: June 13, 2023 3:15pm – 4pm, Join with Google Meet 3rd meeting: June 6, 2023 3:15pm – 4pm, Join with Google Meet 2nd meeting: May 30, 2023 ⋅ 3:15pm – 4pm, Join with Google Meet 1st meeting: May 24, 2023 ⋅ 3:15pm – 4...

@enso-bot
Copy link

enso-bot bot commented Jun 22, 2023

Jaroslav Tulach reports a new 🔴 DELAY for yesterday (2023-06-21):

Summary: There is 7 days delay in implementation of the @widgets for extension functions of Table may not be found (#6955) task.
It will cause 7 days delay for the delivery of this weekly plan.

Got it working somehow

Delay Cause: IdExecutionInstrument can provide access to instrumented ModuleScope to the visualization via State - however different solution is requested.

Possible solutions: Take a shortcut, integrate what we have right now. Or investigate better fix. Let's investigate.

@enso-bot
Copy link

enso-bot bot commented Jun 22, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-21):

Progress: - got get_annotation working: #7078 (comment)

Next Day: Meta.get_annotations

@enso-bot
Copy link

enso-bot bot commented Jun 23, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-22):

Progress: - organisation issues: https://discord.com/channels/401396655599124480/1120986567570370651/1121456406915198976

Next Day: Meta.get_annotations

Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.

@JaroslavTulach JaroslavTulach moved this from ⚙️ Design to 👁️ Code review in Issues Board Jun 23, 2023
@enso-bot
Copy link

enso-bot bot commented Jun 24, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-23):

Progress: - breakthru in get_annotations PR: #7115

Next Day: Get reviews and QA for Meta.get_annotations and integrate

GitHub
Fixes #5612 and #6473. Fixes some static method invocations on Any, like Any == Boolean, or Any.to_text. Pull Request Description Previously, static method calls on Any have not worked as expected....

@enso-bot
Copy link

enso-bot bot commented Jun 27, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-26):

Progress: - addressing Adam's & Pawel's comments: 15b9639

Next Day: Finish Meta.get_annotations and integrate

GitHub
Pull Request Description Added a special escape hatch in the searcher to run AI completions. Screencast was posted previously on discord. You'll need to export OPENAI_API_KEY to use the feature...

@mergify mergify bot closed this as completed in #7115 Jun 27, 2023
mergify bot pushed a commit that referenced this issue Jun 27, 2023
Fixes #6955 by:
- using `visualisationModule` to specify the module where the visualization is to be used
- referring to method in `Meta.get_annotation` with `.method_name` - e.g. unresolved symbol notation
- evaluating arguments to `Meta.get_annotation` in the context of the user module (which can access the extension functions)
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Jun 27, 2023
@enso-bot
Copy link

enso-bot bot commented Jun 28, 2023

Jaroslav Tulach reports a new STANDUP for yesterday (2023-06-27):

Progress: - Merged: #7115

Next Day: Investigate performance of node changes

Discord
Discord is the easiest way to communicate over voice, video, and text. Chat, hang out, and stay close with your friends and communities.
GitHub
Motivation - encapsulation Currently, all the entities (modules, types, methods, constructors, fields) from a project/library can be imported and used, even the entities that are meant to be intern...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
3 participants