Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions crates/amalthea/src/comm/plot_comm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,22 @@ pub struct IntrinsicSize {
pub source: String
}

/// The plot's metadata
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct PlotMetadata {
/// A human-readable name for the plot
pub name: String,

/// The kind of plot e.g. 'Matplotlib', 'ggplot2', etc.
pub kind: String,

/// The ID of the code fragment that produced the plot
pub execution_id: String,

/// The code fragment that produced the plot
pub code: String
}

/// A rendered plot
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
pub struct PlotResult {
Expand Down Expand Up @@ -133,6 +149,12 @@ pub enum PlotBackendRequest {
#[serde(rename = "get_intrinsic_size")]
GetIntrinsicSize,

/// Get metadata for the plot
///
/// Get metadata for the plot
#[serde(rename = "get_metadata")]
GetMetadata,

/// Render a plot
///
/// Requests a plot to be rendered. The plot data is returned in a
Expand All @@ -151,6 +173,9 @@ pub enum PlotBackendReply {
/// The intrinsic size of a plot, if known
GetIntrinsicSizeReply(Option<IntrinsicSize>),

/// The plot's metadata
GetMetadataReply(PlotMetadata),

/// A rendered plot
RenderReply(PlotResult),

Expand Down
18 changes: 16 additions & 2 deletions crates/ark/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,14 @@ impl RMain {
&self.iopub_tx
}

/// Get the current execution context if an active request exists.
/// Returns (execution_id, code) tuple where execution_id is the Jupyter message ID.
pub fn get_execution_context(&self) -> Option<(String, String)> {
self.active_request
.as_ref()
.map(|req| (req.originator.header.msg_id.clone(), req.request.code.clone()))
}

fn init_execute_request(&mut self, req: &ExecuteRequest) -> (ConsoleInput, u32) {
// Reset the autoprint buffer
self.autoprint_output = String::new();
Expand Down Expand Up @@ -1306,11 +1314,17 @@ impl RMain {
// Save `ExecuteCode` request so we can respond to it at next prompt
self.active_request = Some(ActiveReadConsoleRequest {
exec_count,
request: exec_req,
originator,
request: exec_req.clone(),
originator: originator.clone(),
reply_tx,
});

// Push execution context to graphics device for plot attribution
graphics_device::on_execute_request(
originator.header.msg_id.clone(),
exec_req.code.clone(),
);

input
},

Expand Down
165 changes: 165 additions & 0 deletions crates/ark/src/modules/positron/graphics.R
Original file line number Diff line number Diff line change
Expand Up @@ -529,3 +529,168 @@ render_path <- function(id, format) {
file <- paste0("render-", id, ".", format)
file.path(directory, file)
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a personal request, but would you mind if we isolated this code in graphics-kind.R or something like that?

I like that graphics.R is currently just about the main flow of handling the graphics devices

I have a feeling this bit of code might require a few rounds of iteration / might be a common place we have to update in the future, so it would be nice to have it tucked away in its own file (for some reason that makes it easier for me to reason about).

#' Detect the kind of plot from a recording
#'
#' Uses multiple strategies to determine plot type:
#' 1. Check .Last.value for high-level plot objects (ggplot2, lattice)
#' 2. Check recording's display list for base graphics patterns
#' 3. Fall back to generic "plot"
Comment on lines +536 to +538
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the idea is that for ggplot2, the display list isn't really "rich" enough to give you all the details you need to come up with a good name? And instead you ideally need the plot object?

#'
#' @param id The plot ID
#' @return A string describing the plot kind
#' @export
.ps.graphics.detect_plot_kind <- function(id) {
# Strategy 1: Check .Last.value for recognizable plot objects
# This works for ggplot2, lattice, and some other packages
value <- tryCatch(
get(".Last.value", envir = globalenv()),
error = function(e) NULL
)

if (!is.null(value)) {
kind <- detect_kind_from_value(value)
if (!is.null(kind)) {
return(kind)
}
}

# Strategy 2: Check the recording itself
recording <- get_recording(id)
if (!is.null(recording)) {
# recordPlot() stores display list in first element
dl <- recording[[1]]
if (length(dl) > 0) {
kind <- detect_kind_from_display_list(dl)
if (!is.null(kind)) {
return(kind)
}
}
}

# Default fallback
"plot"
}

# Detect plot kind from .Last.value
# Returns plot kind string or NULL
detect_kind_from_value <- function(value) {
# ggplot2
if (inherits(value, "ggplot")) {
return(detect_ggplot_kind(value))
}

# lattice
if (inherits(value, "trellis")) {
# Extract lattice plot type from call
call_fn <- as.character(value$call[[1]])
kind_map <- c(
"xyplot" = "scatter plot",
"bwplot" = "box plot",
"histogram" = "histogram",
"densityplot" = "density plot",
"barchart" = "bar chart",
"dotplot" = "dot plot",
"levelplot" = "heatmap",
"contourplot" = "contour plot",
"cloud" = "3D scatter",
"wireframe" = "3D surface"
)
if (call_fn %in% names(kind_map)) {
return(paste0("lattice ", kind_map[call_fn]))
}
return("lattice")
}

# Base R objects that have class
if (inherits(value, "histogram")) {
return("histogram")
}
if (inherits(value, "density")) {
return("density")
}
if (inherits(value, "hclust")) {
return("dendrogram")
}
if (inherits(value, "acf")) {
return("autocorrelation")
}

NULL
}

# Detect ggplot2 plot kind from geom layers
# Returns plot kind string
detect_ggplot_kind <- function(gg) {
if (length(gg$layers) == 0) {
return("ggplot2")
}

# Get the first layer's geom class
geom_class <- class(gg$layers[[1]]$geom)[1]
geom_name <- tolower(gsub("^Geom", "", geom_class))

kind_map <- c(
"point" = "scatter plot",
"line" = "line chart",
"bar" = "bar chart",
"col" = "bar chart",
"histogram" = "histogram",
"boxplot" = "box plot",
"violin" = "violin plot",
"density" = "density plot",
"area" = "area chart",
"tile" = "heatmap",
"raster" = "raster",
"contour" = "contour plot",
"smooth" = "smoothed line",
"text" = "text",
"label" = "labels",
"path" = "path",
"polygon" = "polygon",
"ribbon" = "ribbon",
"segment" = "segments",
"abline" = "reference lines",
"hline" = "horizontal lines",
"vline" = "vertical lines"
)

if (geom_name %in% names(kind_map)) {
return(paste0("ggplot2 ", kind_map[geom_name]))
}

"ggplot2"
}

# Detect plot kind from display list (base graphics)
# Returns plot kind string or NULL
detect_kind_from_display_list <- function(dl) {
# Display list entries are lists where first element is the C function name
call_names <- vapply(dl, function(x) {
if (is.list(x) && length(x) > 0) {
name <- x[[1]]
if (is.character(name)) name else ""
} else {
""
}
}, character(1))

# Base graphics C functions to plot types
if (any(call_names == "C_plotHist")) return("histogram")
if (any(call_names == "C_image")) return("image")
if (any(call_names == "C_contour")) return("contour")
if (any(call_names == "C_persp")) return("3D surface")
if (any(call_names == "C_filledcontour")) return("filled contour")

# Check for grid graphics (ggplot2, lattice)
if (any(grepl("^L_", call_names))) {
return("grid")
}

# Check for base graphics
if (any(grepl("^C_", call_names))) {
return("base")
}
Comment on lines +685 to +693
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like just plot is more useful than these?


NULL
}
Loading