Skip to content

Commit

Permalink
Downgrade problems in harp_format() to trace (#483)
Browse files Browse the repository at this point in the history
  • Loading branch information
juliasilge authored Aug 26, 2024
1 parent acc8500 commit 3e86243
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 12 deletions.
6 changes: 3 additions & 3 deletions crates/ark/src/data_explorer/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -106,8 +106,8 @@ fn format_values(x: SEXP, format_options: &FormatOptions) -> anyhow::Result<Vec<
}

fn format_object(x: SEXP) -> Vec<FormattedValue> {
// We call r_format() to dispatch the format method
let formatted: Vec<Option<String>> = match r_format(x) {
// We call r_format_vec() to dispatch the format method
let formatted: Vec<Option<String>> = match r_format_vec(x) {
Ok(fmt) => match RObject::from(fmt).try_into() {
Ok(x) => x,
Err(_) => return unknown_format(x),
Expand Down
2 changes: 1 addition & 1 deletion crates/ark/src/variables/variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/harp/src/modules/format.R
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -20,15 +20,15 @@ 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)
))
return(format_fallback(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)
))
Expand All @@ -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)
))
Expand Down
5 changes: 5 additions & 0 deletions crates/harp/src/modules/utils.R
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
12 changes: 10 additions & 2 deletions crates/harp/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,14 @@ impl Sxpinfo {
}
}

#[harp::register]
pub extern "C" fn harp_log_trace(msg: SEXP) -> crate::error::Result<SEXP> {
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<SEXP> {
let msg = String::try_from(RObject::view(msg))?;
Expand Down Expand Up @@ -748,9 +756,9 @@ pub fn r_printf(x: &str) {
}
}

pub fn r_format(x: SEXP) -> Result<SEXP> {
pub fn r_format_vec(x: SEXP) -> Result<SEXP> {
unsafe {
let out = RFunction::new("", "harp_format")
let out = RFunction::new("", "harp_format_vec")
.add(x)
.call_in(HARP_ENV.unwrap())?;
Ok(out.sexp)
Expand Down
4 changes: 2 additions & 2 deletions crates/harp/src/vector/formatted_vector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 3e86243

Please sign in to comment.