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.

Sign up for GitHub

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

Jump to bottom

Add safe r_length() function #23

Closed
wants to merge 2 commits into from
Closed

Add safe r_length() function #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
024e1b5
+safe r_length() -> usize
romainfrancois Jun 9, 2023
6a5dcfd
use isize directly
romainfrancois Jun 9, 2023
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
22 changes: 11 additions & 11 deletions crates/ark/src/data_viewer/r_data_viewer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use harp::utils::r_is_data_frame;
use harp::utils::r_is_matrix;
use harp::utils::r_is_simple_vector;
use harp::utils::r_typeof;
use harp::utils::r_xlength;
Copy link
Contributor

@lionel- lionel- Jun 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think this could be called r_usize()? (or r_ssize() if signed, as in the rlang C lib - edit: oh I see from your comment that this is r_isize() in rust)

Copy link
Contributor

Choose a reason for hiding this comment

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

actually it's r_length() in rlang, sorry for the confusion

r_ssize r_length(r_obj* x);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might want to keep _size() for size of object in bytes: posit-dev/positron#478

use harp::vector::CharacterVector;
use harp::vector::Vector;
use libR_sys::R_CallMethodDef;
Expand All @@ -32,7 +33,6 @@ use libR_sys::INTEGER_ELT;
use libR_sys::SEXP;
use libR_sys::STRSXP;
use libR_sys::VECTOR_ELT;
use libR_sys::XLENGTH;
use serde::Deserialize;
use serde::Serialize;
use stdext::spawn;
Expand Down Expand Up @@ -64,7 +64,7 @@ pub struct DataSet {
pub columns: Vec<DataColumn>,

#[serde(rename = "rowCount")]
pub row_count: usize,
pub row_count: isize,
}

struct ColumnNames {
Expand Down Expand Up @@ -99,14 +99,14 @@ impl DataSet {
unsafe fn extract_columns(
object: SEXP,
prefix: Option<String>,
row_count: usize,
row_count: isize,
columns: &mut Vec<DataColumn>,
) -> Result<(), anyhow::Error> {
if r_is_data_frame(object) {
unsafe {
let names = ColumnNames::new(Rf_getAttrib(object, R_NamesSymbol));

let n_columns = XLENGTH(object);
let n_columns = r_xlength(object);
for i in 0..n_columns {
let col_name = names.get_unchecked(i);

Expand All @@ -123,15 +123,15 @@ impl DataSet {
};

// Protecting with `RObject` in case `object` happens to be an ALTLIST
let column = RObject::from(VECTOR_ELT(object, i));
let column = RObject::from(VECTOR_ELT(object, i as isize));
Self::extract_columns(*column, Some(name), row_count, columns)?;
}
}
} else if r_is_matrix(object) {
unsafe {
let dim = Rf_getAttrib(object, R_DimSymbol);
let n_columns = INTEGER_ELT(dim, 1);
let n_rows = INTEGER_ELT(dim, 0) as usize;
let n_columns = INTEGER_ELT(dim, 1) as isize;
let n_rows = INTEGER_ELT(dim, 0) as isize;
if n_rows != row_count {
bail!("matrix column with incompatible number of rows");
}
Expand All @@ -140,7 +140,7 @@ impl DataSet {
let colnames = ColumnNames::new(*colnames);

for i in 0..n_columns {
let col_name = colnames.get_unchecked(i as isize);
let col_name = colnames.get_unchecked(i);

let name = match prefix {
None => match col_name {
Expand All @@ -156,7 +156,7 @@ impl DataSet {
let matrix_column = RFunction::from("[")
.add(object)
.param("i", R_MissingArg)
.param("j", i + 1)
.param("j", (i + 1) as i32)
.call()?;

Self::extract_columns(*matrix_column, Some(name), row_count, columns)?;
Expand Down Expand Up @@ -193,10 +193,10 @@ impl DataSet {
let row_count = {
if r_is_data_frame(*object) {
let row_names = Rf_getAttrib(*object, R_RowNamesSymbol);
XLENGTH(row_names) as usize
r_xlength(row_names)
} else if r_is_matrix(*object) {
let dim = Rf_getAttrib(*object, R_DimSymbol);
INTEGER_ELT(dim, 0) as usize
INTEGER_ELT(dim, 0) as isize
} else {
bail!("data viewer only handles data frames and matrices")
}
Expand Down
56 changes: 26 additions & 30 deletions crates/ark/src/environment/variable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use harp::utils::r_is_simple_vector;
use harp::utils::r_typeof;
use harp::utils::r_vec_shape;
use harp::utils::r_vec_type;
use harp::utils::r_xlength;
use harp::vector::collapse;
use harp::vector::CharacterVector;
use harp::vector::Collapse;
Expand Down Expand Up @@ -205,7 +206,7 @@ impl WorkspaceVariableDisplayValue {
let formatted = CharacterVector::new_unchecked(formatted);
let out = formatted
.iter()
.take(formatted.len() - 1)
.take(formatted.len() as usize - 1)
.map(|o| o.unwrap())
.join("");
Self::new(out, false)
Expand Down Expand Up @@ -259,9 +260,7 @@ impl WorkspaceVariableDisplayType {

let rtype = r_typeof(value);
match rtype {
EXPRSXP => {
Self::from_class(value, format!("expression [{}]", unsafe { XLENGTH(value) }))
},
EXPRSXP => Self::from_class(value, format!("expression [{}]", r_xlength(value))),
LANGSXP => Self::from_class(value, String::from("language")),
CLOSXP => Self::from_class(value, String::from("function")),
ENVSXP => Self::from_class(value, String::from("environment")),
Expand Down Expand Up @@ -291,7 +290,7 @@ impl WorkspaceVariableDisplayType {

Self::simple(format!("{} [{}]", dfclass, shape))
} else {
Self::from_class(value, format!("list [{}]", XLENGTH(value)))
Self::from_class(value, format!("list [{}]", r_xlength(value)))
}
},
_ => Self::from_class(value, String::from("???")),
Expand Down Expand Up @@ -335,7 +334,7 @@ fn has_children(value: SEXP) -> bool {
}
} else {
match r_typeof(value) {
VECSXP | EXPRSXP => unsafe { XLENGTH(value) != 0 },
VECSXP | EXPRSXP => r_xlength(value) != 0,
LISTSXP => true,
ENVSXP => !Environment::new(RObject::view(value)).is_empty(),
_ => false,
Expand Down Expand Up @@ -386,7 +385,7 @@ impl EnvironmentVariable {
display_type,
type_info,
kind,
length: Self::variable_length(x),
length: Self::variable_length(x) as usize,
size: RObject::view(x).size(),
has_children: has_children(x),
is_truncated,
Expand Down Expand Up @@ -443,28 +442,25 @@ impl EnvironmentVariable {
}
}

fn variable_length(x: SEXP) -> usize {
fn variable_length(x: SEXP) -> isize {
let rtype = r_typeof(x);
match rtype {
LGLSXP | RAWSXP | INTSXP | REALSXP | CPLXSXP | STRSXP => unsafe { XLENGTH(x) as usize },
LGLSXP | RAWSXP | INTSXP | REALSXP | CPLXSXP | STRSXP => r_xlength(x),
VECSXP => unsafe {
if r_inherits(x, "POSIXlt") {
XLENGTH(VECTOR_ELT(x, 0)) as usize
r_xlength(VECTOR_ELT(x, 0))
} else if r_is_data_frame(x) {
let dim = RFunction::new("base", "dim.data.frame")
.add(x)
.call()
.unwrap();

INTEGER_ELT(*dim, 0) as usize
INTEGER_ELT(*dim, 0) as isize
} else {
XLENGTH(x) as usize
r_xlength(x)
}
},
LISTSXP => match pairlist_size(x) {
Ok(n) => n as usize,
Err(_) => 0,
},
LISTSXP => pairlist_size(x).unwrap_or(0),
_ => 0,
}
}
Expand Down Expand Up @@ -500,7 +496,7 @@ impl EnvironmentVariable {

VECSXP => unsafe {
let dim = Rf_getAttrib(x, R_DimSymbol);
if dim != R_NilValue && XLENGTH(dim) == 2 {
if dim != R_NilValue && r_xlength(dim) == 2 {
ValueKind::Table
} else {
ValueKind::Map
Expand All @@ -509,9 +505,9 @@ impl EnvironmentVariable {

LGLSXP => unsafe {
let dim = Rf_getAttrib(x, R_DimSymbol);
if dim != R_NilValue && XLENGTH(dim) == 2 {
if dim != R_NilValue && r_xlength(dim) == 2 {
ValueKind::Table
} else if XLENGTH(x) == 1 {
} else if r_xlength(x) == 1 {
if LOGICAL_ELT(x, 0) == R_NaInt {
ValueKind::Empty
} else {
Expand All @@ -524,9 +520,9 @@ impl EnvironmentVariable {

INTSXP => unsafe {
let dim = Rf_getAttrib(x, R_DimSymbol);
if dim != R_NilValue && XLENGTH(dim) == 2 {
if dim != R_NilValue && r_xlength(dim) == 2 {
ValueKind::Table
} else if XLENGTH(x) == 1 {
} else if r_xlength(x) == 1 {
if INTEGER_ELT(x, 0) == R_NaInt {
ValueKind::Empty
} else {
Expand All @@ -539,9 +535,9 @@ impl EnvironmentVariable {

REALSXP => unsafe {
let dim = Rf_getAttrib(x, R_DimSymbol);
if dim != R_NilValue && XLENGTH(dim) == 2 {
if dim != R_NilValue && r_xlength(dim) == 2 {
ValueKind::Table
} else if XLENGTH(x) == 1 {
} else if r_xlength(x) == 1 {
if R_IsNA(REAL_ELT(x, 0)) == 1 {
ValueKind::Empty
} else {
Expand All @@ -554,9 +550,9 @@ impl EnvironmentVariable {

CPLXSXP => unsafe {
let dim = Rf_getAttrib(x, R_DimSymbol);
if dim != R_NilValue && XLENGTH(dim) == 2 {
if dim != R_NilValue && r_xlength(dim) == 2 {
ValueKind::Table
} else if XLENGTH(x) == 1 {
} else if r_xlength(x) == 1 {
let value = COMPLEX_ELT(x, 0);
if R_IsNA(value.r) == 1 || R_IsNA(value.i) == 1 {
ValueKind::Empty
Expand All @@ -570,9 +566,9 @@ impl EnvironmentVariable {

STRSXP => unsafe {
let dim = Rf_getAttrib(x, R_DimSymbol);
if dim != R_NilValue && XLENGTH(dim) == 2 {
if dim != R_NilValue && r_xlength(dim) == 2 {
ValueKind::Table
} else if XLENGTH(x) == 1 {
} else if r_xlength(x) == 1 {
if STRING_ELT(x, 0) == R_NaString {
ValueKind::Empty
} else {
Expand Down Expand Up @@ -780,7 +776,7 @@ impl EnvironmentVariable {

fn inspect_list(value: SEXP) -> Result<Vec<Self>, harp::error::Error> {
let mut out: Vec<Self> = vec![];
let n = unsafe { XLENGTH(value) };
let n = r_xlength(value);

let names = unsafe {
CharacterVector::new_unchecked(
Expand All @@ -793,8 +789,8 @@ impl EnvironmentVariable {
for i in 0..n {
out.push(Self::from(
i.to_string(),
names.get_unchecked(i).unwrap(),
unsafe { VECTOR_ELT(value, i) },
names.get_unchecked(i as isize).unwrap(),
unsafe { VECTOR_ELT(value, i as isize) },
));
}

Expand Down
Loading