diff --git a/NAMESPACE b/NAMESPACE index ca284d3d8..0c6d61e2c 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -227,6 +227,7 @@ S3method(vec_cast,difftime) S3method(vec_cast,double) S3method(vec_cast,double.exclude) S3method(vec_cast,double.omit) +S3method(vec_cast,double.ts) S3method(vec_cast,exclude.double) S3method(vec_cast,exclude.exclude) S3method(vec_cast,exclude.integer) @@ -236,6 +237,7 @@ S3method(vec_cast,factor.factor) S3method(vec_cast,integer) S3method(vec_cast,integer.exclude) S3method(vec_cast,integer.omit) +S3method(vec_cast,integer.ts) S3method(vec_cast,integer64) S3method(vec_cast,list) S3method(vec_cast,list.vctrs_list_of) @@ -248,6 +250,7 @@ S3method(vec_cast,ordered.character) S3method(vec_cast,ordered.ordered) S3method(vec_cast,raw) S3method(vec_cast,table.table) +S3method(vec_cast,ts.ts) S3method(vec_cast,vctrs_list_of) S3method(vec_cast,vctrs_list_of.list) S3method(vec_cast,vctrs_rcrd) @@ -327,6 +330,7 @@ S3method(vec_proxy_order,array) S3method(vec_proxy_order,default) S3method(vec_proxy_order,list) S3method(vec_proxy_order,raw) +S3method(vec_ptype,ts) S3method(vec_ptype2,AsIs) S3method(vec_ptype2,Date) S3method(vec_ptype2,POSIXct) @@ -366,6 +370,7 @@ S3method(vec_ptype2,ordered.factor) S3method(vec_ptype2,ordered.ordered) S3method(vec_ptype2,raw) S3method(vec_ptype2,table.table) +S3method(vec_ptype2,ts.ts) S3method(vec_ptype2,vctrs_list_of) S3method(vec_ptype2,vctrs_list_of.list) S3method(vec_ptype2.AsIs,AsIs) diff --git a/R/proxy.R b/R/proxy.R index 1d2e6e8dc..1871c8dec 100644 --- a/R/proxy.R +++ b/R/proxy.R @@ -202,20 +202,9 @@ vec_restore_recurse <- function(x, to, ...) { vec_data <- function(x) { obj_check_vector(x) x <- vec_proxy(x) - - if (is.data.frame(x)) { - return(new_data_frame(x, row.names = .row_names_info(x, 0L))) - } - - if (has_dim(x)) { - x <- vec_set_attributes(x, list(dim = dim(x), dimnames = dimnames(x))) - } else { - x <- vec_set_attributes(x, list(names = names(x))) - } - - # Unset S4 bit in vector-like S4 objects - as_not_s4(x) + proxy_data(x) } -as_not_s4 <- function(x) { - .Call(ffi_as_not_s4, x) + +proxy_data <- function(x) { + .Call(ffi_proxy_data, x) } diff --git a/R/shape.R b/R/shape.R index 6eb3bfc77..16f584604 100644 --- a/R/shape.R +++ b/R/shape.R @@ -7,9 +7,13 @@ new_shape <- function(type, shape = integer()) { structure(type, dim = c(0L, shape)) } -vec_shaped_ptype <- function(ptype, x, y, ..., x_arg = "", y_arg = "") { +vec_shaped_ptype <- function(ptype, x) { + .Call(ffi_vec_shaped_ptype, ptype, x) +} + +vec_shaped_ptype2 <- function(ptype, x, y, ..., x_arg = "", y_arg = "") { check_dots_empty0(...) - .Call(ffi_vec_shaped_ptype, ptype, x, y, environment()) + .Call(ffi_vec_shaped_ptype2, ptype, x, y, environment()) } vec_shape2 <- function(x, y, ..., x_arg = "", y_arg = "") { diff --git a/R/type-table.R b/R/type-table.R index a70035015..8767c0d31 100644 --- a/R/type-table.R +++ b/R/type-table.R @@ -29,7 +29,7 @@ vec_ptype_abbr.table <- function(x, ...) { #' @export vec_ptype2.table.table <- function(x, y, ..., x_arg = "", y_arg = "") { ptype <- vec_ptype2(unstructure(x), unstructure(y)) - vec_shaped_ptype(new_table(ptype), x, y, x_arg = x_arg, y_arg = y_arg) + vec_shaped_ptype2(new_table(ptype), x, y, x_arg = x_arg, y_arg = y_arg) } #' @export diff --git a/R/type-ts.R b/R/type-ts.R new file mode 100644 index 000000000..89d7dbc29 --- /dev/null +++ b/R/type-ts.R @@ -0,0 +1,53 @@ +#' `ts` S3 class +#' +#' These functions help the base `ts` class fit into the vctrs type system +#' by providing coercion and casting functions. +#' +#' The `ts` class is a bit strange for vctrs: +#' +#' - It allows for arbitrary storage types. +#' - It has a data dependent `tsp` attribute. +#' - The `ts()` function prevents you from creating a size 0 version of a `ts` +#' object, presumably because you can't make the `tsp` attribute without some +#' data. +#' - The `stats::[.ts` method drops off all attributes. +#' - The `c()` default method drops off all attributes. +#' +#' Because of all of this, we aggressively drive coercion towards the underlying +#' atomic type, which mostly matches the `[` and `c()` methods. +#' +#' @keywords internal +#' @name vctrs-ts +NULL + +#' @export +vec_ptype.ts <- function(x, ...) { + vec_ptype(proxy_data(x)) +} + +#' @export +vec_ptype2.ts.ts <- function(x, y, ..., x_arg = "", y_arg = "") { + vec_ptype2(proxy_data(x), proxy_data(y)) +} + +#' @export +vec_cast.ts.ts <- function(x, to, ...) { + abort("Can't cast directly from to .") +} + +# Needed because `vec_ptype2()` returns underlying storage +#' @export +vec_cast.integer.ts <- function(x, to, ...) { + ts_cast_atomic(x, to) +} + +# Needed because `vec_ptype2()` returns underlying storage +#' @export +vec_cast.double.ts <- function(x, to, ...) { + ts_cast_atomic(x, to) +} + +ts_cast_atomic <- function(x, to) { + x <- proxy_data(x) + vec_cast(x, to) +} diff --git a/man/vctrs-ts.Rd b/man/vctrs-ts.Rd new file mode 100644 index 000000000..84933c81e --- /dev/null +++ b/man/vctrs-ts.Rd @@ -0,0 +1,25 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/type-ts.R +\name{vctrs-ts} +\alias{vctrs-ts} +\title{\code{ts} S3 class} +\description{ +These functions help the base \code{ts} class fit into the vctrs type system +by providing coercion and casting functions. +} +\details{ +The \code{ts} class is a bit strange for vctrs: +\itemize{ +\item It allows for arbitrary storage types. +\item It has a data dependent \code{tsp} attribute. +\item The \code{ts()} function prevents you from creating a size 0 version of a \code{ts} +object, presumably because you can't make the \code{tsp} attribute without some +data. +\item The \verb{stats::[.ts} method drops off all attributes. +\item The \code{c()} default method drops off all attributes. +} + +Because of all of this, we aggressively drive coercion towards the underlying +atomic type, which mostly matches the \code{[} and \code{c()} methods. +} +\keyword{internal} diff --git a/src/decl/proxy-data-decl.h b/src/decl/proxy-data-decl.h new file mode 100644 index 000000000..82c25206d --- /dev/null +++ b/src/decl/proxy-data-decl.h @@ -0,0 +1,8 @@ +static inline +r_obj* df_proxy_data(r_obj* x); + +static inline +r_obj* array_proxy_data(r_obj* x); + +static inline +r_obj* atomic_proxy_data(r_obj* x); diff --git a/src/decl/ptype-decl.h b/src/decl/ptype-decl.h index 9e93fcc0b..71c2ea74a 100644 --- a/src/decl/ptype-decl.h +++ b/src/decl/ptype-decl.h @@ -2,18 +2,13 @@ static SEXP syms_vec_ptype; static inline -r_obj* vec_ptype_slice(r_obj* x, r_obj* empty); +r_obj* df_ptype(r_obj* x, bool tibble); -static -r_obj* df_ptype(r_obj* x, bool bare); - -static +static inline r_obj* col_ptype(r_obj* x); static -r_obj* s3_ptype(r_obj* x, - struct vctrs_arg* x_arg, - struct r_lazy call); +r_obj* s3_ptype(r_obj* x, struct vctrs_arg* p_x_arg, struct r_lazy call); static inline r_obj* vec_ptype_method(r_obj* x); diff --git a/src/init.c b/src/init.c index 03dd45aa8..a27bccab2 100644 --- a/src/init.c +++ b/src/init.c @@ -88,7 +88,6 @@ extern r_obj* ffi_outer_names(r_obj*, r_obj*, r_obj*); extern SEXP vctrs_df_size(SEXP); extern r_obj* ffi_as_df_col(r_obj*, r_obj*, r_obj*); extern r_obj* ffi_apply_name_spec(r_obj*, r_obj*, r_obj*, r_obj*); -extern r_obj* ffi_as_not_s4(r_obj*); extern SEXP vctrs_validate_name_repair_arg(SEXP); extern SEXP vctrs_validate_minimal_names(SEXP, SEXP); extern r_obj* ffi_vec_as_names(r_obj*, r_obj*, r_obj*, r_obj*); @@ -108,7 +107,8 @@ extern r_obj* ffi_vec_rep(r_obj*, r_obj*, r_obj*); extern r_obj* ffi_vec_rep_each(r_obj*, r_obj*, r_obj*); extern SEXP vctrs_maybe_shared_col(SEXP, SEXP); extern SEXP vctrs_new_df_unshared_col(void); -extern r_obj* ffi_vec_shaped_ptype(r_obj*, r_obj*, r_obj*, r_obj*); +extern r_obj* ffi_vec_shaped_ptype(r_obj*, r_obj*); +extern r_obj* ffi_vec_shaped_ptype2(r_obj*, r_obj*, r_obj*, r_obj*); extern r_obj* ffi_vec_shape2(r_obj*, r_obj*, r_obj*); extern SEXP vctrs_new_date(SEXP); extern SEXP vctrs_date_validate(SEXP); @@ -173,6 +173,7 @@ extern r_obj* ffi_vec_replace_values(r_obj*, r_obj*, r_obj*, r_obj*, r_obj*, r_o extern r_obj* ffi_vec_if_else(r_obj*, r_obj*, r_obj*, r_obj*, r_obj*, r_obj*); extern r_obj* ffi_vec_pany(r_obj*, r_obj*, r_obj*, r_obj*); extern r_obj* ffi_vec_pall(r_obj*, r_obj*, r_obj*, r_obj*); +extern r_obj* ffi_proxy_data(r_obj*); // Maturing @@ -287,7 +288,6 @@ static const R_CallMethodDef CallEntries[] = { {"vctrs_df_size", (DL_FUNC) &vctrs_df_size, 1}, {"ffi_as_df_col", (DL_FUNC) &ffi_as_df_col, 3}, {"ffi_apply_name_spec", (DL_FUNC) &ffi_apply_name_spec, 4}, - {"ffi_as_not_s4", (DL_FUNC) &ffi_as_not_s4, 1}, {"vctrs_altrep_rle_Make", (DL_FUNC) &altrep_rle_Make, 1}, {"vctrs_altrep_rle_is_materialized", (DL_FUNC) &altrep_rle_is_materialized, 1}, {"ffi_altrep_new_lazy_character", (DL_FUNC) &ffi_altrep_new_lazy_character, 1}, @@ -311,7 +311,8 @@ static const R_CallMethodDef CallEntries[] = { {"ffi_vec_rep_each", (DL_FUNC) &ffi_vec_rep_each, 3}, {"vctrs_maybe_shared_col", (DL_FUNC) &vctrs_maybe_shared_col, 2}, {"vctrs_new_df_unshared_col", (DL_FUNC) &vctrs_new_df_unshared_col, 0}, - {"ffi_vec_shaped_ptype", (DL_FUNC) &ffi_vec_shaped_ptype, 4}, + {"ffi_vec_shaped_ptype", (DL_FUNC) &ffi_vec_shaped_ptype, 2}, + {"ffi_vec_shaped_ptype2", (DL_FUNC) &ffi_vec_shaped_ptype2, 4}, {"ffi_vec_shape2", (DL_FUNC) &ffi_vec_shape2, 3}, {"vctrs_new_date", (DL_FUNC) &vctrs_new_date, 1}, {"vctrs_date_validate", (DL_FUNC) &vctrs_date_validate, 1}, @@ -376,6 +377,7 @@ static const R_CallMethodDef CallEntries[] = { {"ffi_vec_if_else", (DL_FUNC) &ffi_vec_if_else, 6}, {"ffi_vec_pany", (DL_FUNC) &ffi_vec_pany, 4}, {"ffi_vec_pall", (DL_FUNC) &ffi_vec_pall, 4}, + {"ffi_proxy_data", (DL_FUNC) &ffi_proxy_data, 1}, {"ffi_exp_vec_cast", (DL_FUNC) &exp_vec_cast, 2}, {NULL, NULL, 0} }; diff --git a/src/proxy-data.c b/src/proxy-data.c new file mode 100644 index 000000000..5234e6217 --- /dev/null +++ b/src/proxy-data.c @@ -0,0 +1,82 @@ +#include "proxy-data.h" +#include "dim.h" +#include "type-data-frame.h" +#include "utils.h" + +#include "decl/proxy-data-decl.h" + +r_obj* ffi_proxy_data(r_obj* x) { + return proxy_data(x); +} + +r_obj* proxy_data(r_obj* x) { + if (is_data_frame(x)) { + return df_proxy_data(x); + } else if (has_dim(x)) { + return array_proxy_data(x); + } else { + return atomic_proxy_data(x); + } +} + +static inline +r_obj* df_proxy_data(r_obj* x) { + // We do need to protect these, as we are about to clear `x` + r_obj* names = KEEP(r_names(x)); + r_obj* row_names = KEEP(df_rownames(x)); + const enum rownames_type row_names_type = rownames_type(row_names); + const r_ssize size = rownames_size(row_names, row_names_type); + + // TODO!: At least add documentation on how this always ALTREP clones, + // and some golden tests about it for `proxy_data()` itself. + // + // Presumably we are modifying `x` in place, which WE created + // at the C level but ALSO need it to remain unmodified for + // some future use of our own. + + // This clones `x` as required, and creates a cheap ALTREP shallow duplicate + // of `x` with its own attribute pairlist + r_obj* out = KEEP(vec_set_attributes(x, r_null)); + + r_attrib_poke_names(out, names); + r_init_data_frame(out, size); + + if (row_names_type == ROWNAMES_TYPE_identifiers) { + r_attrib_poke(out, r_syms.row_names, row_names); + } + + FREE(3); + return out; +} + +static inline +r_obj* array_proxy_data(r_obj* x) { + // We do need to protect these, as we are about to clear `x` + r_obj* dim = KEEP(r_dim(x)); + r_obj* dim_names = KEEP(r_dim_names(x)); + + // This clones `x` as required, and creates a cheap ALTREP shallow duplicate + // of `x` with its own attribute pairlist + r_obj* out = KEEP(vec_set_attributes(x, r_null)); + + r_attrib_poke_dim(out, dim); + r_attrib_poke_dim_names(out, dim_names); + + FREE(3); + return out; +} + +static inline +r_obj* atomic_proxy_data(r_obj* x) { + // We do need to protect this, as we are about to clear `x` + r_obj* names = KEEP(r_names(x)); + + // This clones `x` as required, and creates a cheap ALTREP shallow duplicate + // of `x` with its own attribute pairlist + r_obj* out = KEEP(vec_set_attributes(x, r_null)); + + r_attrib_poke_names(out, names); + + FREE(2); + return out; +} diff --git a/src/proxy-data.h b/src/proxy-data.h new file mode 100644 index 000000000..6d2c6bf20 --- /dev/null +++ b/src/proxy-data.h @@ -0,0 +1,17 @@ +#ifndef VCTRS_PROXY_DATA_H +#define VCTRS_PROXY_DATA_H + +#include "vctrs-core.h" + +/// Returns the "core" data that underlies a proxy +/// +/// Should only be called on the result of a call to `vec_proxy()`. Can be used +/// to ensure that all extraneous attributes and classes have been stripped from +/// a proxy object, which may help avoid inflooping in some cases. +/// +/// - For atomics, clears all attributes but `names`. +/// - For arrays, clears all attributes but `dim` and `dimnames`. +/// - For data frames, clears all attributes but `names`, `row.names`, and a `"data.frame"` class. +r_obj* proxy_data(r_obj* x); + +#endif diff --git a/src/proxy.c b/src/proxy.c index 0e6645819..a4d3750f7 100644 --- a/src/proxy.c +++ b/src/proxy.c @@ -247,11 +247,6 @@ r_obj* vec_proxy_unwrap(r_obj* x) { } -r_obj* ffi_as_not_s4(r_obj* x) { - return r_as_not_s4(x); -} - - void vctrs_init_data(r_obj* ns) { syms_vec_proxy = r_sym("vec_proxy"); diff --git a/src/ptype.c b/src/ptype.c index 89e639001..867bdd98c 100644 --- a/src/ptype.c +++ b/src/ptype.c @@ -1,5 +1,6 @@ #include "vctrs.h" -#include "type-data-frame.h" +#include "proxy-data.h" + #include "decl/ptype-decl.h" @@ -10,72 +11,145 @@ r_obj* ffi_ptype(r_obj* x, r_obj* x_arg_ffi, r_obj* frame) { return vec_ptype(x, &x_arg, call); } -r_obj* vec_ptype(r_obj* x, struct vctrs_arg* x_arg, struct r_lazy call) { +/// Compute the prototype of `x` +/// +/// Morally, a prototype is a size 0 representation of the minimum amount of +/// information needed to uniquely identify the type of `x`. +/// +/// - For `NULL`, `NULL` +/// +/// - For unclassed vectors, a size 0 vector +/// - All attributes are cleared +/// - Notably, `names` are cleared +/// +/// - For unclassed arrays, a size 0 array +/// - Almost all attributes are cleared +/// - `dim` is retained to hold the "shape", as that is part of the type +/// - Notably, `dimnames` are cleared +/// +/// - For bare data frames / tibbles, a size 0 data frame / tibble +/// - Almost all attributes are cleared +/// - `names` are retained to hold the column names, which are part of the type +/// - `class` is retained, as that is part of the type +/// - `vec_ptype()` is called on each column +/// +/// - For S3 objects +/// +/// - If a `vec_ptype()` S3 method is implemented, we call that. No further +/// checking is done, for maximum performance. We rely on the package author +/// to implement a well formed ptype that strips all unrelated attributes. +/// +/// - Otherwise, we do the following: +/// +/// ```r +/// # Retrieve R object appropriate for C level manipulation +/// proxy <- vec_proxy(x) +/// +/// # Strip that R object down to its most "native" form +/// # - For vectors, only `names` are retained +/// # - For arrays, only `dim` and `dimnames` are retained +/// # - For data frames, only `names`, `row.names`, and the `"data.frame"` class are retained +/// proxy <- proxy_data(proxy) +/// +/// # Compute ptype of this "native" form +/// proxy <- vec_ptype(proxy) +/// +/// # Restore back to original type +/// vec_restore(proxy, x) +/// ``` +/// +/// Note that this is a "best effort" fallback that does retain all attributes +/// from `x` except for the `names`, `dimnames`, or `row.names`. +/// This is a "best effort" fallback that does retain all other attributes, as we +/// have no way to know what is an extraneous attribute (beyond names), and +/// what is an attribute that is core to the type itself. +r_obj* vec_ptype(r_obj* x, struct vctrs_arg* p_x_arg, struct r_lazy call) { switch (vec_typeof(x)) { - case VCTRS_TYPE_null: return r_null; - case VCTRS_TYPE_unspecified: return vctrs_shared_empty_uns; - case VCTRS_TYPE_logical: return vec_ptype_slice(x, r_globals.empty_lgl); - case VCTRS_TYPE_integer: return vec_ptype_slice(x, r_globals.empty_int); - case VCTRS_TYPE_double: return vec_ptype_slice(x, r_globals.empty_dbl); - case VCTRS_TYPE_complex: return vec_ptype_slice(x, r_globals.empty_cpl); - case VCTRS_TYPE_character: return vec_ptype_slice(x, r_globals.empty_chr); - case VCTRS_TYPE_raw: return vec_ptype_slice(x, r_globals.empty_raw); - case VCTRS_TYPE_list: return vec_ptype_slice(x, r_globals.empty_list); - case VCTRS_TYPE_dataframe: return df_ptype(x, true); - case VCTRS_TYPE_s3: return s3_ptype(x, x_arg, call); - case VCTRS_TYPE_scalar: stop_scalar_type(x, x_arg, call); - } - r_stop_unreachable(); -} - -static -r_obj* col_ptype(r_obj* x) { - return vec_ptype(x, vec_args.empty, r_lazy_null); -} - -static inline -r_obj* vec_ptype_slice(r_obj* x, r_obj* empty) { - if (r_attrib(x) == r_null) { - return empty; - } else { - // Slicing preserves attributes - return vec_slice(x, r_null); + case VCTRS_TYPE_null: { + return r_null; + } + case VCTRS_TYPE_unspecified: { + return vctrs_shared_empty_uns; + } + case VCTRS_TYPE_logical: { + return vec_shaped_ptype(r_globals.empty_lgl, x); + } + case VCTRS_TYPE_integer: { + return vec_shaped_ptype(r_globals.empty_int, x); + } + case VCTRS_TYPE_double: { + return vec_shaped_ptype(r_globals.empty_dbl, x); + } + case VCTRS_TYPE_complex: { + return vec_shaped_ptype(r_globals.empty_cpl, x); + } + case VCTRS_TYPE_character: { + return vec_shaped_ptype(r_globals.empty_chr, x); + } + case VCTRS_TYPE_raw: { + return vec_shaped_ptype(r_globals.empty_raw, x); + } + case VCTRS_TYPE_list: { + return vec_shaped_ptype(r_globals.empty_list, x); + } + case VCTRS_TYPE_dataframe: { + return df_ptype(x, /*tibble=*/false); + } + case VCTRS_TYPE_s3: { + switch (class_type(x)) { + case VCTRS_CLASS_none: { + r_stop_unreachable(); + } + case VCTRS_CLASS_bare_data_frame: { + r_stop_unreachable(); + } + case VCTRS_CLASS_bare_tibble: { + return df_ptype(x, /*tibble=*/true); + } + default: { + return s3_ptype(x, p_x_arg, call); + } + } + } + case VCTRS_TYPE_scalar: { + stop_scalar_type(x, p_x_arg, call); + } + default: { + r_stop_unreachable(); + } } } static -r_obj* s3_ptype(r_obj* x, - struct vctrs_arg* x_arg, - struct r_lazy call) { - switch (class_type(x)) { - case VCTRS_CLASS_bare_tibble: - return df_ptype(x, true); - - case VCTRS_CLASS_data_frame: - return df_ptype(x, false); - - case VCTRS_CLASS_bare_data_frame: - r_stop_internal("Bare data frames should be handled by `vec_ptype()`."); - - case VCTRS_CLASS_none: - r_stop_internal("Non-S3 classes should be handled by `vec_ptype()`."); - - default: - break; +r_obj* s3_ptype(r_obj* x, struct vctrs_arg* p_x_arg, struct r_lazy call) { + // Use `vec_ptype()` S3 method if one exists. + // For maximal performance, no additional checking is done. + r_obj* method = vec_ptype_method(x); + if (method != r_null) { + KEEP(method); + r_obj* out = vec_ptype_invoke(x, method); + FREE(1); + return out; } - r_obj* method = KEEP(vec_ptype_method(x)); + // This check is done after checking for an S3 method! + obj_check_vector(x, VCTRS_ALLOW_NULL_no, p_x_arg, call); - r_obj* out; + // Otherwise use "fallback" approach of calling `vec_ptype()` on the proxy's + // native data. `proxy_data()` prevents this from being an infloop. + r_obj* proxy = KEEP(vec_proxy(x)); + proxy = KEEP(proxy_data(proxy)); + proxy = KEEP(vec_ptype(proxy, vec_args.empty, r_lazy_null)); - if (method == r_null) { - obj_check_vector(x, VCTRS_ALLOW_NULL_no, x_arg, call); - out = vec_slice(x, r_null); - } else { - out = vec_ptype_invoke(x, method); - } + // Must be `VCTRS_OWNERSHIP_foreign`, as `vec_ptype()` returns shared objects + struct vec_restore_opts opts = { + .ownership = VCTRS_OWNERSHIP_foreign, + .recursively_proxied = false + }; - FREE(1); + r_obj* out = vec_restore_opts(proxy, x, &opts); + + FREE(3); return out; } @@ -92,26 +166,25 @@ r_obj* vec_ptype_invoke(r_obj* x, r_obj* method) { return vctrs_dispatch1(syms_vec_ptype, method, syms_x, x); } -r_obj* df_ptype(r_obj* x, bool bare) { - r_obj* row_nms = KEEP(df_rownames(x)); - - r_obj* ptype = r_null; - if (bare) { - ptype = KEEP(bare_df_map(x, &col_ptype)); +static inline +r_obj* df_ptype(r_obj* x, bool tibble) { + r_obj* out = KEEP(map(x, &col_ptype)); + if (tibble) { + r_init_tibble(out, 0); } else { - ptype = KEEP(df_map(x, &col_ptype)); - } - - if (r_typeof(row_nms) == R_TYPE_character) { - r_attrib_poke(ptype, r_syms.row_names, r_globals.empty_chr); + r_init_data_frame(out, 0); } + FREE(1); + return out; +} - FREE(2); - return ptype; +static inline +r_obj* col_ptype(r_obj* x) { + return vec_ptype(x, vec_args.empty, r_lazy_null); } -r_obj* vec_ptype_final(r_obj* x, struct vctrs_arg* x_arg, struct r_lazy call) { - r_obj* out = KEEP(vec_ptype(x, x_arg, call)); +r_obj* vec_ptype_final(r_obj* x, struct vctrs_arg* p_x_arg, struct r_lazy call) { + r_obj* out = KEEP(vec_ptype(x, p_x_arg, call)); out = vec_ptype_finalise(out); FREE(1); return out; diff --git a/src/ptype.h b/src/ptype.h index eab8534ce..a7605dc5f 100644 --- a/src/ptype.h +++ b/src/ptype.h @@ -3,7 +3,7 @@ #include "vctrs-core.h" -r_obj* vec_ptype(r_obj* x, struct vctrs_arg* x_arg, struct r_lazy call); -r_obj* vec_ptype_final(r_obj* x, struct vctrs_arg* x_arg, struct r_lazy call); +r_obj* vec_ptype(r_obj* x, struct vctrs_arg* p_x_arg, struct r_lazy call); +r_obj* vec_ptype_final(r_obj* x, struct vctrs_arg* p_x_arg, struct r_lazy call); #endif diff --git a/src/ptype2.c b/src/ptype2.c index 24e206bb3..e88da163e 100644 --- a/src/ptype2.c +++ b/src/ptype2.c @@ -72,7 +72,7 @@ r_obj* vec_ptype2_opts_impl(const struct ptype2_opts* opts, r_obj* out = KEEP(vec_ptype2_dispatch_native(opts, x_type, y_type, left)); if (out != r_null) { - out = vec_shaped_ptype(out, x, y, x_arg, y_arg); + out = vec_shaped_ptype2(out, x, y, x_arg, y_arg); FREE(1); return out; } @@ -118,30 +118,30 @@ r_obj* vec_ptype2_switch_native(const struct ptype2_opts* opts, return r_null; case VCTRS_TYPE2_logical_logical: - return vec_shaped_ptype(r_globals.empty_lgl, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_lgl, x, y, x_arg, y_arg); case VCTRS_TYPE2_logical_integer: case VCTRS_TYPE2_integer_integer: - return vec_shaped_ptype(r_globals.empty_int, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_int, x, y, x_arg, y_arg); case VCTRS_TYPE2_logical_double: case VCTRS_TYPE2_integer_double: case VCTRS_TYPE2_double_double: - return vec_shaped_ptype(r_globals.empty_dbl, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_dbl, x, y, x_arg, y_arg); case VCTRS_TYPE2_integer_complex: case VCTRS_TYPE2_double_complex: case VCTRS_TYPE2_complex_complex: - return vec_shaped_ptype(r_globals.empty_cpl, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_cpl, x, y, x_arg, y_arg); case VCTRS_TYPE2_character_character: - return vec_shaped_ptype(r_globals.empty_chr, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_chr, x, y, x_arg, y_arg); case VCTRS_TYPE2_raw_raw: - return vec_shaped_ptype(r_globals.empty_raw, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_raw, x, y, x_arg, y_arg); case VCTRS_TYPE2_list_list: - return vec_shaped_ptype(r_globals.empty_list, x, y, x_arg, y_arg); + return vec_shaped_ptype2(r_globals.empty_list, x, y, x_arg, y_arg); case VCTRS_TYPE2_dataframe_dataframe: return df_ptype2(opts); diff --git a/src/shape.c b/src/shape.c index a52d60457..d21f4ec3f 100644 --- a/src/shape.c +++ b/src/shape.c @@ -1,17 +1,41 @@ #include "vctrs.h" #include "decl/shape-decl.h" -r_obj* ffi_vec_shaped_ptype(r_obj* ptype, r_obj* x, r_obj* y, r_obj* frame) { +// ----------------------------------------------------------------------------- + +r_obj* ffi_vec_shaped_ptype(r_obj* ffi_ptype, r_obj* ffi_x) { + return vec_shaped_ptype(ffi_ptype, ffi_x); +} + +r_obj* vec_shaped_ptype(r_obj* ptype, r_obj* x) { + if (!has_dim(x)) { + // By far the most common case + return ptype; + } + + r_obj* x_dimensions = r_dim(x); + r_obj* x_shape = KEEP(dims_shape(x_dimensions)); + + ptype = KEEP(r_clone_referenced(ptype)); + r_attrib_poke_dim(ptype, x_shape); + + FREE(2); + return ptype; +} + +// ----------------------------------------------------------------------------- + +r_obj* ffi_vec_shaped_ptype2(r_obj* ptype, r_obj* x, r_obj* y, r_obj* frame) { struct r_lazy x_arg_ = { .x = syms.x_arg, .env = frame }; struct vctrs_arg x_arg = new_lazy_arg(&x_arg_); struct r_lazy y_arg_ = { .x = syms.y_arg, .env = frame }; struct vctrs_arg y_arg = new_lazy_arg(&y_arg_); - return vec_shaped_ptype(ptype, x, y, &x_arg, &y_arg); + return vec_shaped_ptype2(ptype, x, y, &x_arg, &y_arg); } -r_obj* vec_shaped_ptype( +r_obj* vec_shaped_ptype2( r_obj* ptype, r_obj* x, r_obj* y, @@ -54,7 +78,7 @@ r_obj* vec_shape2( struct vctrs_arg* p_y_arg ) { // Expect that `r_dim()` does not allocate, so we don't protect these! - // This is somewhat important for performance, because `vec_shaped_ptype()` + // This is somewhat important for performance, because `vec_shaped_ptype2()` // is called on every ptype2 iteration. r_obj* x_dimensions = r_dim(x); r_obj* y_dimensions = r_dim(y); diff --git a/src/shape.h b/src/shape.h index 940766772..348363135 100644 --- a/src/shape.h +++ b/src/shape.h @@ -3,10 +3,14 @@ #include "vctrs-core.h" +// Attaches the shape of `x` as the dimensions of `ptype`. +// If `x` is atomic with `NULL` dimensions, then `ptype` is returned unmodified. +r_obj* vec_shaped_ptype(r_obj* ptype, r_obj* x); + // Computes the common shape of `x` and `y` and attaches it as the // dimensions of `ptype`. If `x` and `y` are both atomic with `NULL` dimensions, // then no dimensions are attached and `ptype` is returned unmodified. -r_obj* vec_shaped_ptype( +r_obj* vec_shaped_ptype2( r_obj* ptype, r_obj* x, r_obj* y, diff --git a/src/size.c b/src/size.c index e57cb08b6..3237d55a8 100644 --- a/src/size.c +++ b/src/size.c @@ -107,7 +107,8 @@ r_ssize df_rownames_size(r_obj* x) { continue; } - return rownames_size(r_node_car(attr)); + r_obj* rn = r_node_car(attr); + return rownames_size(rn, rownames_type(rn)); } return -1; diff --git a/src/type-data-frame.c b/src/type-data-frame.c index 009ed46df..d984b159a 100644 --- a/src/type-data-frame.c +++ b/src/type-data-frame.c @@ -420,8 +420,8 @@ r_ssize compact_rownames_length(r_obj* x) { } // [[ include("type-data-frame.h") ]] -r_ssize rownames_size(r_obj* rn) { - switch (rownames_type(rn)) { +r_ssize rownames_size(r_obj* rn, enum rownames_type type) { + switch (type) { case ROWNAMES_TYPE_identifiers: case ROWNAMES_TYPE_automatic: return r_length(rn); diff --git a/src/type-data-frame.h b/src/type-data-frame.h index d124b7109..1f2264f05 100644 --- a/src/type-data-frame.h +++ b/src/type-data-frame.h @@ -46,7 +46,7 @@ enum rownames_type { ROWNAMES_TYPE_identifiers }; enum rownames_type rownames_type(r_obj* rn); -r_ssize rownames_size(r_obj* rn); +r_ssize rownames_size(r_obj* rn, enum rownames_type type); r_obj* df_ptype2(const struct ptype2_opts* opts); diff --git a/src/unspecified.c b/src/unspecified.c index 0babfce68..f9d48c415 100644 --- a/src/unspecified.c +++ b/src/unspecified.c @@ -103,6 +103,10 @@ r_obj* vec_ptype_finalise(r_obj* x) { obj_check_vector(x, VCTRS_ALLOW_NULL_no, vec_args.x, call); + // TODO!: Should act more like `df_ptype()` for the bare cases, and + // `s3_ptype()` for the classed df case (proxy/vec_ptype_finalise/restore). + // Then remove `bare_df_map()` and `df_map()` and unexpose + // `vec_bare_df_restore()` and `vec_df_restore()`. switch (class_type(x)) { case VCTRS_CLASS_bare_tibble: case VCTRS_CLASS_bare_data_frame: diff --git a/src/utils.c b/src/utils.c index f3fa2af98..ad25225b0 100644 --- a/src/utils.c +++ b/src/utils.c @@ -31,6 +31,8 @@ SEXP classes_vctrs_group_rle = NULL; static SEXP syms_as_data_frame2 = NULL; static SEXP fns_as_data_frame2 = NULL; +static SEXP syms_vec_set_attributes = NULL; +static SEXP fns_vec_set_attributes = NULL; static SEXP vctrs_eval_mask_n_impl(SEXP fn_sym, SEXP fn, SEXP* syms, SEXP* args, SEXP env); @@ -1424,6 +1426,28 @@ SEXP r_as_data_frame(SEXP x) { } } +/// Set attributes on `x` +/// +/// ``` +/// attributes(x) <- attrib +/// x +/// ``` +/// +/// It can be useful to call this from C when you don't own `x` and just need to +/// tweak some attributes on it because it creates a cheap ALTREP shallow +/// duplicate of `x` with `R_shallow_duplicate_attr()`, which we don't have +/// access to. +SEXP vec_set_attributes(SEXP x, SEXP attrib) { + return vctrs_dispatch2( + syms_vec_set_attributes, + fns_vec_set_attributes, + syms_x, + x, + syms_attrib, + attrib + ); +} + static SEXP syms_try_catch_hnd = NULL; static inline SEXP try_catch_hnd(SEXP ptr) { SEXP call = PROTECT(Rf_lang2(syms_try_catch_hnd, ptr)); @@ -1687,6 +1711,7 @@ SEXP syms_which = NULL; SEXP syms_slice_value = NULL; SEXP syms_index_style = NULL; SEXP syms_loc = NULL; +SEXP syms_attrib = NULL; SEXP fns_bracket = NULL; SEXP fns_quote = NULL; @@ -1989,6 +2014,7 @@ void vctrs_init_utils(SEXP ns) { syms_slice_value = Rf_install("slice_value"); syms_index_style = Rf_install("index_style"); syms_loc = Rf_install("loc"); + syms_attrib = Rf_install("attrib"); fns_bracket = Rf_findVar(syms_bracket, R_BaseEnv); fns_quote = Rf_findVar(Rf_install("quote"), R_BaseEnv); @@ -2003,9 +2029,11 @@ void vctrs_init_utils(SEXP ns) { rlang_sym_as_character = (SEXP (*)(SEXP)) R_GetCCallable("rlang", "rlang_sym_as_character"); syms_as_data_frame2 = Rf_install("as.data.frame2"); + syms_vec_set_attributes = Rf_install("vec_set_attributes"); syms_colnames = Rf_install("colnames"); fns_as_data_frame2 = r_env_get(ns, syms_as_data_frame2); + fns_vec_set_attributes = r_env_get(ns, syms_vec_set_attributes); fns_colnames = r_env_get(R_BaseEnv, syms_colnames); compact_seq_attrib = Rf_cons(R_NilValue, R_NilValue); diff --git a/src/utils.h b/src/utils.h index f8cb9cbaf..bcc1f2bbc 100644 --- a/src/utils.h +++ b/src/utils.h @@ -390,6 +390,8 @@ static inline const void* vec_type_missing_value(enum vctrs_type type) { } } +SEXP vec_set_attributes(SEXP x, SEXP attrib); + void c_print_backtrace(void); SEXP chr_c(SEXP x, SEXP y); @@ -535,6 +537,7 @@ extern SEXP syms_which; extern SEXP syms_slice_value; extern SEXP syms_index_style; extern SEXP syms_loc; +extern SEXP syms_attrib; static const char * const c_strs_vctrs_common_class_fallback = "vctrs:::common_class_fallback"; diff --git a/src/vctrs.h b/src/vctrs.h index d5f8e7260..21b328563 100644 --- a/src/vctrs.h +++ b/src/vctrs.h @@ -77,7 +77,6 @@ SEXP vec_proxy_order(SEXP x); SEXP vec_proxy_unwrap(SEXP x); SEXP vec_slice_shaped(enum vctrs_type type, SEXP x, SEXP index); bool vec_requires_fallback(SEXP x, struct vctrs_proxy_info info); -r_obj* vec_ptype(r_obj* x, struct vctrs_arg* x_arg, struct r_lazy call); SEXP vec_names(SEXP x); SEXP vec_proxy_names(SEXP x); SEXP vec_group_loc(SEXP x); diff --git a/tests/testthat/_snaps/bind.md b/tests/testthat/_snaps/bind.md index 52c8f99cd..bf56d6497 100644 --- a/tests/testthat/_snaps/bind.md +++ b/tests/testthat/_snaps/bind.md @@ -93,6 +93,17 @@ ! Can't merge the outer name `foo` with a vector of length > 1. Please supply a `.name_spec` specification. +# vec_rbind() requires a data frame proxy for data frame ptypes + + Code + vec_rbind(df, df) + Condition + Error in `vec_rbind()`: + ! Attempt to restore data frame from a double. + i In file 'proxy-restore.c' at line . + i This is an internal error that was detected in the vctrs package. + Please report it at with a reprex () and the full backtrace. + # names of `...` are used for type and cast errors even when zapped Code diff --git a/tests/testthat/_snaps/type-sf.md b/tests/testthat/_snaps/type-sf.md new file mode 100644 index 000000000..bf214d42b --- /dev/null +++ b/tests/testthat/_snaps/type-sf.md @@ -0,0 +1,8 @@ +# `crs` attributes of `sfc` vectors must be the same + + Code + vctrs::vec_c(x, y) + Condition + Error: + ! arguments have different crs + diff --git a/tests/testthat/_snaps/type-ts.md b/tests/testthat/_snaps/type-ts.md new file mode 100644 index 000000000..22d280a07 --- /dev/null +++ b/tests/testthat/_snaps/type-ts.md @@ -0,0 +1,8 @@ +# can't do a self cast + + Code + vec_cast(x, y) + Condition + Error in `vec_cast.ts.ts()`: + ! Can't cast directly from to . + diff --git a/tests/testthat/test-assert.R b/tests/testthat/test-assert.R index fc0285198..3f6010654 100644 --- a/tests/testthat/test-assert.R +++ b/tests/testthat/test-assert.R @@ -225,13 +225,20 @@ test_that("assertion is not applied on proxy", { expect_error(vec_assert(x, x), regexp = NA) }) -test_that("attributes of unclassed vectors are asserted", { +test_that("attributes of unclassed vectors are not considered part of the type (#2025)", { x <- structure(FALSE, foo = "bar") - y <- structure(TRUE, foo = "bar") - expect_false(vec_is(x, FALSE)) - expect_false(vec_is(FALSE, x)) - expect_true(vec_is(y, x)) + y <- structure(TRUE, bar = "foo") + + expect_true(vec_is(x, x)) + + expect_true(vec_is(x, logical())) + expect_true(vec_is(logical(), x)) + expect_true(vec_is(x, y)) + expect_true(vec_is(y, x)) + + # This is consistent with `vec_ptype2()`! + expect_identical(vec_ptype2(x, y), logical()) }) test_that("unspecified is finalised before assertion", { diff --git a/tests/testthat/test-bind.R b/tests/testthat/test-bind.R index c14aa6c8c..82bdf0483 100644 --- a/tests/testthat/test-bind.R +++ b/tests/testthat/test-bind.R @@ -414,10 +414,24 @@ test_that("vec_rbind() requires a data frame proxy for data frame ptypes", { vec_proxy.vctrs_foobar = function(x, ...) 1 ) - expect_error( - vec_rbind(df, df), - "Can't fill a data frame that doesn't have a data frame proxy" - ) + expect_snapshot( + error = TRUE, + transform = scrub_internal_error_line_number, + vec_rbind(df, df) + ) + + # It used to be the case that `vec_ptype()` would do `vec_slice(x, 0L)`, which + # would work fine on this `df` and we'd eventually fall through to this rbind + # specific error. But now it does proxy/ptype/restore, and we end up with a + # different restoration error about trying to restore a double to a data + # frame, which is also a good error (#2025). We no longer have a test that + # hits this error in particular, but I'm not sure we can actually hit it + # anymore. + # + # expect_error( + # vec_rbind(df, df), + # "Can't fill a data frame that doesn't have a data frame proxy" + # ) }) test_that("names of `...` are used for type and cast errors even when zapped", { diff --git a/tests/testthat/test-recode.R b/tests/testthat/test-recode.R index b85ca81d7..9da57bd56 100644 --- a/tests/testthat/test-recode.R +++ b/tests/testthat/test-recode.R @@ -476,34 +476,23 @@ test_that("proof that `ptype` finalization is important", { test_that("extraneous `to` attributes don't end up on the final output", { x <- c(1, 2, 3) - # TODO: Ideally the attributes wouldn't show up on the output, but - # `list_combine()` doesn't clear them because `vec_ptype(to)` retains - # them for some reason - # https://github.com/r-lib/vctrs/issues/2025 from <- c(2, 3) to <- structure(c(0, -1), foo = "bar") expect_identical( vec_recode_values(x, from = from, to = to), - structure(c(NA, 0, -1), foo = "bar") + c(NA, 0, -1) ) - # TODO: Ideally the attributes wouldn't show up on the output, but - # `list_combine()` doesn't clear them because `vec_ptype(to)` retains - # them for some reason - # https://github.com/r-lib/vctrs/issues/2025 from <- 2 to <- list( structure(c(0, -1, -2), foo = "bar") ) expect_identical( vec_recode_values(x, from = from, to = to, to_as_list_of_vectors = TRUE), - structure(c(NA, -1, NA), foo = "bar") + c(NA, -1, NA) ) - # Note that as soon as you force a `ptype2` computation, the attributes - # disappear anyways, suggesting an inconsistency - - # `ptype2` forced by `default` + # With `ptype2` computation forced by `default` from <- c(2, 3) to <- structure(c(0, -1), foo = "bar") expect_identical( @@ -511,7 +500,7 @@ test_that("extraneous `to` attributes don't end up on the final output", { c(NA, 0, -1) ) - # `ptype2` forced by multiple `to` values + # With `ptype2` computation forced by multiple `to` values from <- c(2, 3) to <- list( structure(c(0, -1, -2), foo = "bar"), @@ -524,16 +513,12 @@ test_that("extraneous `to` attributes don't end up on the final output", { }) test_that("extraneous `x` attributes don't end up on the final output", { - # TODO: Ideally the attributes wouldn't show up on the output, but - # `list_combine()` doesn't clear them because `vec_ptype(x)` retains - # them for some reason - # https://github.com/r-lib/vctrs/issues/2025 + # Because it is built on `vec_recode_values()` x <- structure(1, foo = "bar") expect_identical( vec_replace_values(x, from = 1, to = 2), - structure(2, foo = "bar") - # 2 + 2 ) }) diff --git a/tests/testthat/test-s4.R b/tests/testthat/test-s4.R index 53c64dd91..a245a92d8 100644 --- a/tests/testthat/test-s4.R +++ b/tests/testthat/test-s4.R @@ -60,16 +60,12 @@ test_that("proxy and data", { expect_true(isS4(vec_restore(vec_data(x), x))) }) -test_that("as_not_s4() copies and works", { - # Initial condition +test_that("vec_data() unsets S4 bit", { x <- rando() - expect_true(isS4(x)) + + # Unsetting actually works + expect_false(isS4(vec_data(x))) # Unsetting has no side effect on x - as_not_s4(x) expect_true(isS4(x)) - - # Unsetting actually works - y <- as_not_s4(x) - expect_false(isS4(y)) }) diff --git a/tests/testthat/test-slice.R b/tests/testthat/test-slice.R index 57023dfb8..6bb462c1e 100644 --- a/tests/testthat/test-slice.R +++ b/tests/testthat/test-slice.R @@ -867,9 +867,11 @@ test_that("vec_slice() restores unrestored but named foreign classes", { expect_identical(vec_slice(x, 1), x) expect_identical(vec_chop(x), list(x)) expect_identical(vec_chop(x, indices = list(1)), list(x)) - expect_identical(vec_ptype(x), foobar(named(dbl()))) - expect_identical(vec_ptype(x), foobar(named(dbl()))) - expect_identical(vec_ptype_common(x, x), foobar(named(dbl()))) + + # Note that the `ptype` is not named! (#2025) + expect_identical(vec_ptype(x), foobar(dbl())) + expect_identical(vec_ptype(x), foobar(dbl())) + expect_identical(vec_ptype_common(x, x), foobar(dbl())) out <- vec_ptype_common_fallback(x, x) expect_true(is_common_class_fallback(out)) diff --git a/tests/testthat/test-type-bare.R b/tests/testthat/test-type-bare.R index fab2d83c8..84f35014e 100644 --- a/tests/testthat/test-type-bare.R +++ b/tests/testthat/test-type-bare.R @@ -24,6 +24,56 @@ test_that("default cast allows objects with the same type", { # vec_shaped_ptype ------------------------------------------------------- +test_that("non-arrays don't adjust the `ptype`", { + expect_identical(vec_shaped_ptype(integer(), 1L), integer()) +}) + +test_that("arrays pass on their shape", { + ptype <- logical() + + # 1D arrays do turn `ptype` into an empty 1D array + x <- array(logical(), dim = 0) + expect_identical( + vec_shaped_ptype(ptype, x), + array(logical(), dim = 0) + ) + + x <- array(logical(), dim = 1) + expect_identical( + vec_shaped_ptype(ptype, x), + array(logical(), dim = 0) + ) + + x <- array(logical(), dim = c(2, 3, 4)) + expect_identical( + vec_shaped_ptype(ptype, x), + array(logical(), dim = c(0, 3, 4)) + ) +}) + +test_that("dim names aren't passed along as part of the shape", { + ptype <- logical() + + x <- array( + logical(), + dim = c(0, 2, 3), + dimnames = list(character(), c("a", "b"), c("x", "y", "z")) + ) + + expect_identical( + vec_shaped_ptype(ptype, x), + array(logical(), dim = c(0, 2, 3)) + ) +}) + +test_that("`ptype` isn't modified in place", { + ptype <- logical() + vec_shaped_ptype(ptype, array(logical(), dim = c(0, 2, 3))) + expect_identical(ptype, logical()) +}) + +# vec_shaped_ptype2 ------------------------------------------------------ + test_that("array dimensions are preserved", { mat1 <- matrix(lgl(), nrow = 1, ncol = 1) mat2 <- matrix(lgl(), nrow = 2, ncol = 2) @@ -34,31 +84,31 @@ test_that("array dimensions are preserved", { expect_error(vec_ptype2(mat2, mat3), class = "vctrs_error_incompatible_type") }) -test_that("vec_shaped_ptype()", { +test_that("vec_shaped_ptype2()", { int <- function(...) array(NA_integer_, c(...)) expect_identical( - vec_shaped_ptype(integer(), int(5), int(10)), + vec_shaped_ptype2(integer(), int(5), int(10)), new_shape(integer()) ) expect_identical( - vec_shaped_ptype(integer(), int(5, 1), int(10, 1)), + vec_shaped_ptype2(integer(), int(5, 1), int(10, 1)), new_shape(integer(), 1) ) expect_identical( - vec_shaped_ptype(integer(), int(5, 1, 2), int(10, 1, 2)), + vec_shaped_ptype2(integer(), int(5, 1, 2), int(10, 1, 2)), new_shape(integer(), 1:2) ) }) -test_that("vec_shaped_ptype() evaluates arg lazily", { - expect_silent(vec_shaped_ptype( +test_that("vec_shaped_ptype2() evaluates arg lazily", { + expect_silent(vec_shaped_ptype2( integer(), int(5), int(10), x_arg = print("oof") )) - expect_silent(vec_shaped_ptype( + expect_silent(vec_shaped_ptype2( integer(), int(5), int(10), diff --git a/tests/testthat/test-type-idate.R b/tests/testthat/test-type-idate.R index 7ae700510..c9a7a83a7 100644 --- a/tests/testthat/test-type-idate.R +++ b/tests/testthat/test-type-idate.R @@ -17,6 +17,15 @@ as_IDate_with_names <- function(x) { # ------------------------------------------------------------------------------ # ptype +test_that("ptype", { + x <- as.IDate("2019-01-01") + expect_identical(vec_ptype(x), as.IDate(integer())) + + # Note how names are correctly dropped (#2025) + x <- as_IDate_with_names(c(a = "2019-01-01")) + expect_identical(vec_ptype(x), as.IDate(integer())) +}) + test_that("ptype abbr", { x <- as.IDate("2019-01-01") expect_identical(vec_ptype_abbr(x), "IDate") @@ -31,11 +40,9 @@ test_that("ptype full", { # ptype2 test_that("can get common type of IDate and IDate", { + # Note how names are correctly dropped (#2025) x <- as_IDate_with_names(c(a = "2019-01-01")) - - # It shouldn't have names, but thats a vctrs problem - expect <- as_IDate_with_names(set_names(integer(), character())) - + expect <- as.IDate(integer()) expect_identical(vec_ptype2(x, x), expect) }) diff --git a/tests/testthat/test-type-misc.R b/tests/testthat/test-type-misc.R index cbc7bcad0..8939286cd 100644 --- a/tests/testthat/test-type-misc.R +++ b/tests/testthat/test-type-misc.R @@ -186,20 +186,6 @@ test_that("`package_version` and `R_system_version` use the `numeric_version` pr expect_identical(vec_proxy_equal(z), vec_proxy_equal(x)) }) -test_that("can slice `ts` vectors", { - x <- ts(1:3) - expect_identical(vec_ptype(x), x[0]) - expect_identical(vec_slice(x, 2), x[2]) -}) - -test_that("can concatenate `ts` vectors", { - x <- ts(1:3) - expect_identical(vec_c(x, x), c(x, x)) - - df <- data_frame(x = x) - expect_identical(vec_rbind(df, df), data_frame(x = c(x, x))) -}) - test_that("`omit` class is numeric (#1160)", { x <- c(NA, 1:3, NA) omit <- attr(na.omit(x), "na.action") diff --git a/tests/testthat/test-type-ts.R b/tests/testthat/test-type-ts.R new file mode 100644 index 000000000..e5928823f --- /dev/null +++ b/tests/testthat/test-type-ts.R @@ -0,0 +1,58 @@ +test_that("can take the ptype of `ts` vectors", { + x <- ts(1:3) + expect_identical(vec_ptype(x), integer()) + + x <- ts(1.5) + expect_identical(vec_ptype(x), double()) + + x <- ts(matrix(1L, nrow = 2)) + expect_identical(vec_ptype(x), array(integer(), dim = c(0L, 1L))) +}) + +test_that("can slice `ts` vectors", { + # Slicing `ts` with `[` drops the `ts` class + x <- ts(1:3) + expect_identical(vec_slice(x, 2), x[2]) + expect_identical(vec_slice(x, 2), 2L) +}) + +test_that("can't do a self cast", { + x <- ts(1:3) + y <- ts(4:7) + + # Not allowed, attributes can't be resolved + expect_snapshot(error = TRUE, { + vec_cast(x, y) + }) + + # This results in common type of integer, the underlying storage type + expect_identical( + vec_cast_common(x, y), + list(1:3, 4:7) + ) + + # This actually works because the `vec_ptype()` returns the underlying + # storage type, and the `vec_ptype()` of `.to` is taken + expect_identical( + vec_cast_common(x, y, .to = x), + list(1:3, 4:7) + ) +}) + +test_that("can concatenate `ts` vectors", { + # `c()` method demonstrates that the common type is the common type of the + # underlying storage type, and attributes are dropped + x <- ts(1:3) + expect_identical(vec_c(x, x), c(x, x)) + + df <- data_frame(x = x) + expect_identical(vec_rbind(df, df), data_frame(x = c(x, x))) + + y <- ts(c(4, 5, 6)) + expect_identical(vec_c(x, y), c(x, y)) + + # vctrs rules retain the shape here + z <- ts(matrix(c(4, 5, 6))) + expect_identical(vec_c(x, z), matrix(c(1, 2, 3, 4, 5, 6))) + expect_identical(c(x, z), c(1, 2, 3, 4, 5, 6)) +}) diff --git a/tests/testthat/test-type.R b/tests/testthat/test-type.R index e64010959..797d06819 100644 --- a/tests/testthat/test-type.R +++ b/tests/testthat/test-type.R @@ -269,12 +269,22 @@ test_that("vec_ptype_finalise() requires vector types", { expect_error(vec_ptype_finalise(foobar()), class = "vctrs_error_scalar_type") }) -# This might change in the future if we decide that prototypes don't -# have names -test_that("vec_ptype() preserves type of names and row names", { - expect_identical(vec_ptype(c(foo = 1)), named(dbl())) - expect_identical(vec_ptype(mtcars), mtcars[0, ]) - expect_identical(vec_ptype(foobar(mtcars)), foobar(mtcars[0, ])) +test_that("vec_ptype() does not preserve names, dim names, or row names (#2025)", { + expect_identical(vec_ptype(c(foo = 1)), dbl()) + + expect_identical( + vec_ptype(array(1, dim = c(1, 1, 1), dimnames = list("a", "b", "c"))), + array(dbl(), dim = c(0, 1, 1)) + ) + + expect_identical( + vec_ptype(mtcars), + set_rownames_dispatch(mtcars[0, ], NULL) + ) + expect_identical( + vec_ptype(foobar(mtcars)), + foobar(set_rownames_dispatch(mtcars[0, ], NULL)) + ) }) test_that("vec_ptype_common() handles spliced names consistently (#1570)", { diff --git a/tests/testthat/test-type2.R b/tests/testthat/test-type2.R index 34363f5ae..3783af5bc 100644 --- a/tests/testthat/test-type2.R +++ b/tests/testthat/test-type2.R @@ -368,8 +368,8 @@ test_that("vec_ptype2() methods get prototypes", { expect_identical(y, foobar(chr())) vec_ptype2(foobar(mtcars), foobar(iris)) - expect_identical(x, foobar(mtcars[0, , drop = FALSE])) - expect_identical(y, foobar(iris[0, , drop = FALSE])) + expect_identical(x, vec_ptype(foobar(mtcars))) + expect_identical(y, vec_ptype(foobar(iris))) }) test_that("vec_ptype2() allows vec_ptype() to return another type", {