From 3e8624311a978d3aea94603b7ed5a3e647f2cd1d Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Mon, 26 Aug 2024 07:27:44 -0700 Subject: [PATCH] Downgrade problems in `harp_format()` to trace (#483) --- crates/ark/src/data_explorer/format.rs | 6 +++--- crates/ark/src/variables/variable.rs | 2 +- crates/harp/src/modules/format.R | 8 ++++---- crates/harp/src/modules/utils.R | 5 +++++ crates/harp/src/utils.rs | 12 ++++++++++-- crates/harp/src/vector/formatted_vector.rs | 4 ++-- 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/crates/ark/src/data_explorer/format.rs b/crates/ark/src/data_explorer/format.rs index 9f0156558..cbc122a4a 100644 --- a/crates/ark/src/data_explorer/format.rs +++ b/crates/ark/src/data_explorer/format.rs @@ -15,7 +15,7 @@ use harp::object::r_length; use harp::object::RObject; use harp::r_null; use harp::utils::r_classes; -use harp::utils::r_format; +use harp::utils::r_format_vec; use harp::utils::r_is_null; use harp::utils::r_typeof; use harp::vector::CharacterVector; @@ -106,8 +106,8 @@ fn format_values(x: SEXP, format_options: &FormatOptions) -> anyhow::Result Vec { - // We call r_format() to dispatch the format method - let formatted: Vec> = match r_format(x) { + // We call r_format_vec() to dispatch the format method + let formatted: Vec> = match r_format_vec(x) { Ok(fmt) => match RObject::from(fmt).try_into() { Ok(x) => x, Err(_) => return unknown_format(x), diff --git a/crates/ark/src/variables/variable.rs b/crates/ark/src/variables/variable.rs index a0e5997f4..91f1fc8ec 100644 --- a/crates/ark/src/variables/variable.rs +++ b/crates/ark/src/variables/variable.rs @@ -301,7 +301,7 @@ impl WorkspaceVariableDisplayValue { } fn from_error(err: Error) -> Self { - log::warn!("Error while formatting variable: {err:?}"); + log::trace!("Error while formatting variable: {err:?}"); Self::new(String::from("??"), true) } } diff --git a/crates/harp/src/modules/format.R b/crates/harp/src/modules/format.R index ccb9ae677..8af3d4a61 100644 --- a/crates/harp/src/modules/format.R +++ b/crates/harp/src/modules/format.R @@ -8,7 +8,7 @@ # Works around possibly unconforming methods in `base::format()`. Tries # hard to recover from failed assumptions, including by unclassing and # reformatting with the default method. -harp_format <- function(x, ...) { +harp_format_vec <- function(x, ...) { if (is.object(x)) { format_oo(x, ...) } else { @@ -20,7 +20,7 @@ format_oo <- function(x, ...) { out <- base::format(x, ...) if (!is.character(out)) { - log_warning(sprintf( + log_trace(sprintf( "`format()` method for <%s> should return a character vector.", class_collapsed(x) )) @@ -28,7 +28,7 @@ format_oo <- function(x, ...) { } if (length(x) != length(out)) { - log_warning(sprintf( + log_trace(sprintf( "`format()` method for <%s> should return the same number of elements.", class_collapsed(x) )) @@ -38,7 +38,7 @@ format_oo <- function(x, ...) { # Try to recover if dimensions don't agree (for example `format.Surv()` # doesn't preserve dimensions, see https://github.com/posit-dev/positron/issues/1862) if (!identical(dim(x), dim(out))) { - log_warning(sprintf( + log_trace(sprintf( "`format()` method for <%s> should return conforming dimensions.", class_collapsed(x) )) diff --git a/crates/harp/src/modules/utils.R b/crates/harp/src/modules/utils.R index 49ed70ee6..be81d8299 100644 --- a/crates/harp/src/modules/utils.R +++ b/crates/harp/src/modules/utils.R @@ -1,3 +1,8 @@ +log_trace <- function(msg) { + stopifnot(is_string(msg)) + .Call("harp_log_trace", msg) +} + log_warning <- function(msg) { stopifnot(is_string(msg)) .Call("harp_log_warning", msg) diff --git a/crates/harp/src/utils.rs b/crates/harp/src/utils.rs index bef39d6ab..b0346b779 100644 --- a/crates/harp/src/utils.rs +++ b/crates/harp/src/utils.rs @@ -106,6 +106,14 @@ impl Sxpinfo { } } +#[harp::register] +pub extern "C" fn harp_log_trace(msg: SEXP) -> crate::error::Result { + let msg = String::try_from(RObject::view(msg))?; + log::trace!("{msg}"); + + unsafe { Ok(libr::R_NilValue) } +} + #[harp::register] pub extern "C" fn harp_log_warning(msg: SEXP) -> crate::error::Result { let msg = String::try_from(RObject::view(msg))?; @@ -748,9 +756,9 @@ pub fn r_printf(x: &str) { } } -pub fn r_format(x: SEXP) -> Result { +pub fn r_format_vec(x: SEXP) -> Result { unsafe { - let out = RFunction::new("", "harp_format") + let out = RFunction::new("", "harp_format_vec") .add(x) .call_in(HARP_ENV.unwrap())?; Ok(out.sexp) diff --git a/crates/harp/src/vector/formatted_vector.rs b/crates/harp/src/vector/formatted_vector.rs index 2085ded3d..9cbcb39ad 100644 --- a/crates/harp/src/vector/formatted_vector.rs +++ b/crates/harp/src/vector/formatted_vector.rs @@ -18,7 +18,7 @@ use libr::STRSXP; use crate::error::Error; use crate::error::Result; -use crate::r_format; +use crate::r_format_vec; use crate::utils::r_assert_type; use crate::utils::r_inherits; use crate::utils::r_is_null; @@ -119,7 +119,7 @@ impl FormattedVector { vector: Factor::new_unchecked(vector), }) } else { - let formatted = r_format(vector)?; + let formatted = r_format_vec(vector)?; r_assert_type(formatted, &[STRSXP])?; Ok(Self::FormattedVector {