-
Notifications
You must be signed in to change notification settings - Fork 25
Add code evaluation API #1085
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
base: main
Are you sure you want to change the base?
Add code evaluation API #1085
Changes from 3 commits
796ef2a
f9393a7
040179d
3400c7a
6bc78c7
9d9f4a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ use amalthea::comm::comm_channel::CommMsg; | |
| use amalthea::comm::ui_comm::CallMethodParams; | ||
| use amalthea::comm::ui_comm::DidChangePlotsRenderSettingsParams; | ||
| use amalthea::comm::ui_comm::EditorContextChangedParams; | ||
| use amalthea::comm::ui_comm::EvaluateCodeParams; | ||
| use amalthea::comm::ui_comm::UiBackendReply; | ||
| use amalthea::comm::ui_comm::UiBackendRequest; | ||
| use amalthea::comm::ui_comm::UiFrontendEvent; | ||
|
|
@@ -18,6 +19,7 @@ use amalthea::wire::input_request::UiCommFrontendRequest; | |
| use crossbeam::channel::Receiver; | ||
| use crossbeam::channel::Sender; | ||
| use crossbeam::select; | ||
| use harp::eval::parse_eval_global; | ||
| use harp::exec::RFunction; | ||
| use harp::exec::RFunctionExt; | ||
| use harp::object::RObject; | ||
|
|
@@ -154,6 +156,7 @@ impl UiComm { | |
| UiBackendRequest::EditorContextChanged(params) => { | ||
| self.handle_editor_context_changed(params) | ||
| }, | ||
| UiBackendRequest::EvaluateCode(params) => self.handle_evaluate_code(params), | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -233,6 +236,31 @@ impl UiComm { | |
| Ok(UiBackendReply::EditorContextChangedReply()) | ||
| } | ||
|
|
||
| fn handle_evaluate_code( | ||
| &self, | ||
| params: EvaluateCodeParams, | ||
| ) -> anyhow::Result<UiBackendReply, anyhow::Error> { | ||
| log::trace!("Evaluating code: {}", params.code); | ||
|
|
||
| let result = r_task(|| { | ||
| let evaluated = parse_eval_global(¶ms.code)?; | ||
| Value::try_from(evaluated) | ||
| }); | ||
|
|
||
| match result { | ||
| Ok(value) => Ok(UiBackendReply::EvaluateCodeReply(value)), | ||
| Err(err) => { | ||
| let message = match &err { | ||
| harp::Error::TryCatchError { message, .. } => message.clone(), | ||
| harp::Error::ParseError { message, .. } => message.clone(), | ||
| harp::Error::ParseSyntaxError { message } => message.clone(), | ||
|
||
| _ => format!("{err}"), | ||
| }; | ||
| Err(anyhow::anyhow!("{message}")) | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Send an RPC request to the frontend. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,18 @@ use crate::object::RObject; | |
| /// {"a": 1, "b": true, "c": "applesauce"} | ||
| /// - Named lists with duplicate keys have the values combined into an array | ||
| /// - e.g.: list(a = 1L, a = 2L, a = 3L) -> {"a": [1, 2, 3]} | ||
| /// Returns the names of an R object, or `None` if the object has no names or | ||
| /// all names are empty/NA. This is used to decide whether a vector should be | ||
| /// serialized as a JSON object (when names are present) or as an array. | ||
| fn non_empty_names(obj: &RObject) -> Option<Vec<Option<String>>> { | ||
| let names = obj.names()?; | ||
| let all_empty = names.iter().all(|name| match name { | ||
| Some(name) => name.is_empty(), | ||
| None => true, | ||
|
||
| }); | ||
| if all_empty { None } else { Some(names) } | ||
| } | ||
|
|
||
| impl TryFrom<RObject> for Value { | ||
| type Error = crate::error::Error; | ||
| fn try_from(obj: RObject) -> Result<Self, Self::Error> { | ||
|
|
@@ -163,17 +175,41 @@ impl TryFrom<RObject> for Value { | |
| Ok(Value::String(str)) | ||
| }, | ||
|
|
||
| // With multiple values, convert to a string array | ||
| // With multiple values, convert to a string array or object | ||
| _ => { | ||
| let mut arr = Vec::<Value>::with_capacity(obj.length().try_into().unwrap()); | ||
| let names = non_empty_names(&obj); | ||
|
|
||
| let n = obj.length(); | ||
| for i in 0..n { | ||
| arr.push(match obj.get_string(i)? { | ||
| Some(str) => Value::String(str), | ||
| None => Value::Null, | ||
| }); | ||
|
|
||
| match names { | ||
| Some(names) => { | ||
| let mut map = Map::new(); | ||
| let n = min(n, names.len().try_into().unwrap()); | ||
| for i in 0..n { | ||
| let key = match &names[i as usize] { | ||
| Some(name) => name.clone(), | ||
| None => String::new(), | ||
| }; | ||
| let val = match obj.get_string(i)? { | ||
| Some(str) => Value::String(str), | ||
| None => Value::Null, | ||
| }; | ||
| map.insert(key, val); | ||
| } | ||
| Ok(Value::Object(map)) | ||
| }, | ||
| None => { | ||
| let mut arr = | ||
| Vec::<Value>::with_capacity(n.try_into().unwrap()); | ||
| for i in 0..n { | ||
| arr.push(match obj.get_string(i)? { | ||
| Some(str) => Value::String(str), | ||
| None => Value::Null, | ||
| }); | ||
| } | ||
| Ok(Value::Array(arr)) | ||
| }, | ||
| } | ||
| Ok(serde_json::Value::Array(arr)) | ||
| }, | ||
| }, | ||
|
|
||
|
|
@@ -186,24 +222,7 @@ impl TryFrom<RObject> for Value { | |
| // See whether the object's values have names. We will try | ||
| // to convert named values into a JSON object (map); unnamed | ||
| // values become an array. | ||
| let mut names = obj.names(); | ||
|
|
||
| // Check to see if all the names are empty. We want to treat | ||
| // this identically to an unnamed list. | ||
| let mut all_empty = true; | ||
| if let Some(names) = &names { | ||
| for name in names { | ||
| if let Some(name) = name { | ||
| if !name.is_empty() { | ||
| all_empty = false; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| if all_empty { | ||
| names = None; | ||
| } | ||
| let names = non_empty_names(&obj); | ||
|
|
||
| match names { | ||
| Some(names) => { | ||
|
|
@@ -436,6 +455,27 @@ mod tests { | |
| }) | ||
| } | ||
|
|
||
| #[test] | ||
| #[allow(non_snake_case)] | ||
| fn test_json_named_character_vectors() { | ||
|
||
| crate::r_task(|| { | ||
| // Named character vectors should serialize to JSON objects | ||
| assert_r_matches_json( | ||
| "c(a = 'one', b = 'two', c = 'three')", | ||
| "{\"a\": \"one\", \"b\": \"two\", \"c\": \"three\"}", | ||
| ); | ||
|
|
||
| // Unnamed character vectors should still serialize to arrays | ||
| assert_r_matches_json("c('one', 'two', 'three')", "[\"one\", \"two\", \"three\"]"); | ||
|
|
||
| // Character vectors with all-empty names should serialize to arrays | ||
| assert_r_matches_json( | ||
| "x <- c('a', 'b'); names(x) <- c('', ''); x", | ||
| "[\"a\", \"b\"]", | ||
| ); | ||
| }) | ||
| } | ||
|
|
||
| #[test] | ||
| #[allow(non_snake_case)] | ||
| fn test_json_na_vectors() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way there's one command that I was using all the time in Emacs and that I'd like to implement in Positron. It's a version of "evaluate statement" that inserts the output of the evaluation below the statement, with #> prefix.
IIUC it's not currently possible to implement this from an extension because the evaluation needs to happen in the background (silently) but the output should still be captured and returned to the caller. It looks like that use case is not covered by posit-dev/positron#7112.
To support this case, perhaps we could consider returning a structured value with
resultandoutputfields. Theoutputfield could be opt-in by language in case the kernel can't easily support it. See #1064 for an example of how to evaluate with output and condition capture.In any case, wrapping the return value in a struct would be more future-proof, allowing new fields to be added as needed.