From e07715654992229d46d7871638985c4c0a0afc8d Mon Sep 17 00:00:00 2001 From: lbm364dl Date: Sat, 12 Jul 2025 15:07:44 +0200 Subject: [PATCH 1/2] Fix small typo --- R/type-rcrd.R | 2 +- man/new_rcrd.Rd | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/type-rcrd.R b/R/type-rcrd.R index c45728549..77ac8e43d 100644 --- a/R/type-rcrd.R +++ b/R/type-rcrd.R @@ -3,7 +3,7 @@ #' rcrd (record) S3 class #' #' The rcrd class extends [vctr]. A rcrd is composed of 1 or more [field]s, -#' which must be vectors of the same length. Is designed specifically for +#' which must be vectors of the same length. It's designed specifically for #' classes that can naturally be decomposed into multiple vectors of the same #' length, like [POSIXlt], but where the organisation should be considered #' an implementation detail invisible to the user (unlike a [data.frame]). diff --git a/man/new_rcrd.Rd b/man/new_rcrd.Rd index 05705629d..48781b03e 100644 --- a/man/new_rcrd.Rd +++ b/man/new_rcrd.Rd @@ -20,7 +20,7 @@ named vectors.} } \description{ The rcrd class extends \link{vctr}. A rcrd is composed of 1 or more \link{field}s, -which must be vectors of the same length. Is designed specifically for +which must be vectors of the same length. It's designed specifically for classes that can naturally be decomposed into multiple vectors of the same length, like \link{POSIXlt}, but where the organisation should be considered an implementation detail invisible to the user (unlike a \link{data.frame}). From b7ace581a7c1e350fac069b0cff0a90b090fb9b4 Mon Sep 17 00:00:00 2001 From: lbm364dl Date: Sat, 12 Jul 2025 15:08:02 +0200 Subject: [PATCH 2/2] Fix misleading example and clarify --- vignettes/s3-vector.Rmd | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/vignettes/s3-vector.Rmd b/vignettes/s3-vector.Rmd index dce4b1620..0134e4ba5 100644 --- a/vignettes/s3-vector.Rmd +++ b/vignettes/s3-vector.Rmd @@ -546,7 +546,7 @@ vctrs makes it easy to create new record-style classes using `new_rcrd()`, which ### Rational class -A fraction, or rational number, can be represented by a pair of integer vectors representing the numerator (the number on top) and the denominator (the number on bottom), where the length of each vector must be the same. +A vector of fractions, or rational numbers, can be represented by a pair of integer vectors representing the numerator (the number on top) and the denominator (the number on bottom), where the length of each vector must be the same. To represent such a data structure we turn to a new base data type: the record (or rcrd for short). As usual we start with low-level and user-friendly constructors. @@ -771,11 +771,18 @@ x == rational(1, 1) unique(x) ``` -We now need to fix the comparison operations similarly, since comparison currently happens lexicographically by `n`, then by `d`: +We now need to fix the comparison operations similarly, since comparison currently happens lexicographically by `n`, then by `d`. +The default implementation of `vec_proxy_compare()` calls `vec_proxy_equal()`. +Since we implemented `vec_proxy_equal()` previously, the default order is +more specifically happening by `n/gcd`, then by `d/gcd`. ```{r} +# The expected behaviour rational(1, 2) < rational(2, 3) +# True but misleading, actually doing rational(1, 2) < rational(2, 3) rational(2, 4) < rational(2, 3) +# Wrong because of lexicographical order +rational(2, 5) < rational(2, 3) ``` The easiest fix is to convert the fraction to a floating point number and use this as a proxy: @@ -785,7 +792,7 @@ vec_proxy_compare.vctrs_rational <- function(x, ...) { field(x, "n") / field(x, "d") } -rational(2, 4) < rational(2, 3) +rational(2, 5) < rational(2, 3) ``` This also fixes `sort()`, because the default implementation of `vec_proxy_order()` calls `vec_proxy_compare()`. @@ -794,7 +801,14 @@ This also fixes `sort()`, because the default implementation of `vec_proxy_order sort(x) ``` -(We could have used the same approach in `vec_proxy_equal()`, but when working with floating point numbers it's not necessarily true that `x == y` implies that `d * x == d * y`.) +We could have used the same approach in `vec_proxy_equal()`, but when working with floating point numbers it's not necessarily true that `x == y` implies that `d * x == d * y`. + +These floating point precision issues are something to keep in mind if using +our rational example seriously. Our original `vec_proxy_equal()` implementation +using the greatest common divisor is completely accurate, but our floating point +implementation of `vec_proxy_compare()` is not. A correct comparison check +between two rationals `a/b < c/d` would be `a*d < c*b`, but this is not possible +with our proxy values approach. Always be cautious with floating point numbers. ### Polynomial class