compare units via ut_compare, fixes r-quantities#339

.Rbuildignore

@@ -1,7 +1,8 @@
 ^.*\.Rproj$
 ^\.Rproj\.user$
-.travis.yml
+^.travis.yml$
 ^appveyor\.yml$
+^_pkgdown\.yml$
 ^\.github$
 README.md
 ^quantities$

DESCRIPTION

@@ -1,5 +1,5 @@
 Package: units
-Version: 0.8-1.1
+Version: 0.8-1.2
 Title: Measurement Units for R Vectors
 Authors@R: c(person("Edzer", "Pebesma", role = c("aut", "cre"), email = "[email protected]", comment = c(ORCID = "0000-0001-8049-7069")),
     person("Thomas", "Mailund", role = "aut", email = "[email protected]"),

NEWS.md

@@ -5,6 +5,9 @@
 * Identical units will always divide (`/`) and allow integer division (`%/%`).
   And, inverse units will always be able to multiply; #310 @billdenney
 
+* Compare units via `ut_compare()`, fixing inconsistent results for aliases
+  and symbols; #339
+
 # version 0.8-1
 
 * fix `%/%` and `%%` if arguments have different units; #313

R/RcppExports.R

@@ -13,6 +13,10 @@ ud_set_encoding <- function(enc_str) {
     invisible(.Call('_units_ud_set_encoding', PACKAGE = 'units', enc_str))
 }
 
+ud_compare <- function(x, y, xn, yn) {
+    .Call('_units_ud_compare', PACKAGE = 'units', x, y, xn, yn)
+}
+
 ud_convert <- function(val, from, to) {
     .Call('_units_ud_convert', PACKAGE = 'units', val, from, to)
 }

R/arith.R

@@ -45,24 +45,40 @@ Ops.units <- function(e1, e2) {
   if (missing(e2))
     return(NextMethod())
 
-  eq  <- .Generic %in% c("+", "-", "==", "!=", "<", ">", "<=", ">=") # requiring identical units
-  div <- .Generic %in% c("/", "%/%")                                 # division-type
-  mul <- .Generic %in% "*"                                           # multiplication-only
-  prd <- .Generic %in% c("*", "/", "%/%", "%%")                      # product-type
-  pw  <- .Generic %in% c("**", "^")                                  # power-type
-  mod <- .Generic %in% c("%/%", "%%")                                # modulo-type
-  pm  <- .Generic %in% c("+", "-")                                   # addition-type
-
-  if (! any(eq, prd, pw, mod))
+  cmp <- .Generic %in% c("==", "!=", "<", ">", "<=", ">=") # comparison-type
+  div <- .Generic %in% c("/", "%/%")                       # division-type
+  mul <- .Generic %in% "*"                                 # multiplication-only
+  prd <- .Generic %in% c("*", "/", "%/%", "%%")            # product-type
+  pw  <- .Generic %in% c("**", "^")                        # power-type
+  mod <- .Generic %in% c("%/%", "%%")                      # modulo-type
+  pm  <- .Generic %in% c("+", "-")                         # addition-type
+
+  if (! any(cmp, pm, prd, pw, mod))
     stop(paste("operation", .Generic, "not allowed"))
 
   e1_inherits_units <- inherits(e1, "units")
   e2_inherits_units <- inherits(e2, "units")
   both_inherit_units <- e1_inherits_units && e2_inherits_units
-  if (eq) {
-    if (!(both_inherit_units))
-      stop("both operands of the expression should be \"units\" objects") # nocov
-    units(e2) <- units(e1) # convert before we can compare; errors if unconvertible
+
+  if ((pm || cmp) && !both_inherit_units)
+    stop("both operands of the expression should be \"units\" objects") # nocov
+
+  if (cmp) {
+    if (!ud_are_convertible(units(e1), units(e2)))
+      stop("cannot compare non-convertible units")
+
+    if (length(e1) >= length(e2)) {
+      e1 <- ud_compare(e1, e2, as.character(units(e1)), as.character(units(e2)))
+      attr <- attributes(e2)
+      e2 <- rep(0, length(e2))
+      attributes(e2) <- attr
+    } else {
+      e2 <- ud_compare(e2, e1, as.character(units(e2)), as.character(units(e1)))
+      attr <- attributes(e1)
+      e1 <- rep(0, length(e1))
+      attributes(e1) <- attr
+    }
+    return(NextMethod())
   }
 
   identical_units <-
@@ -172,14 +188,12 @@ Ops.units <- function(e1, e2) {
           rep(unique(units(e1)$denominator),table(units(e1)$denominator)*abs(e2)),
           rep(unique(units(e1)$numerator),table(units(e1)$numerator)*abs(e2)))
     }
-  } else # eq, plus/minus:
+  } else { # pm:
+    units(e2) <- units(e1)
     u <- units(e1)
+  }
 
-  if (eq && !pm) {
-    dimension = dim(structure(as.numeric(e1), dim = dim(e1)) == structure(as.numeric(e2), dim = dim(e2)))
-    structure(as.logical(NextMethod()), dim = dimension)
-  } else
-    .as.units(NextMethod(), u)
+  .as.units(NextMethod(), u)
 }
 
 #' #' matrix multiplication

src/RcppExports.cpp

@@ -39,6 +39,20 @@ BEGIN_RCPP
     return R_NilValue;
 END_RCPP
 }
+// ud_compare
+IntegerVector ud_compare(NumericVector x, NumericVector y, CharacterVector xn, CharacterVector yn);
+RcppExport SEXP _units_ud_compare(SEXP xSEXP, SEXP ySEXP, SEXP xnSEXP, SEXP ynSEXP) {
+BEGIN_RCPP
+    Rcpp::RObject rcpp_result_gen;
+    Rcpp::RNGScope rcpp_rngScope_gen;
+    Rcpp::traits::input_parameter< NumericVector >::type x(xSEXP);
+    Rcpp::traits::input_parameter< NumericVector >::type y(ySEXP);
+    Rcpp::traits::input_parameter< CharacterVector >::type xn(xnSEXP);
+    Rcpp::traits::input_parameter< CharacterVector >::type yn(ynSEXP);
+    rcpp_result_gen = Rcpp::wrap(ud_compare(x, y, xn, yn));
+    return rcpp_result_gen;
+END_RCPP
+}
 // ud_convert
 NumericVector ud_convert(NumericVector val, CharacterVector from, CharacterVector to);
 RcppExport SEXP _units_ud_convert(SEXP valSEXP, SEXP fromSEXP, SEXP toSEXP) {
@@ -283,6 +297,7 @@ static const R_CallMethodDef CallEntries[] = {
     {"_units_ud_exit", (DL_FUNC) &_units_ud_exit, 0},
     {"_units_ud_init", (DL_FUNC) &_units_ud_init, 1},
     {"_units_ud_set_encoding", (DL_FUNC) &_units_ud_set_encoding, 1},
+    {"_units_ud_compare", (DL_FUNC) &_units_ud_compare, 4},
     {"_units_ud_convert", (DL_FUNC) &_units_ud_convert, 3},
     {"_units_ud_map_names", (DL_FUNC) &_units_ud_map_names, 2},
     {"_units_ud_unmap_names", (DL_FUNC) &_units_ud_unmap_names, 1},

src/udunits.cpp

@@ -78,6 +78,38 @@ void ud_set_encoding(std::string enc_str) {
     stop("Valid encoding string parameters are ('utf8'|'ascii'|'iso-8859-1','latin1')");
 }
 
+// [[Rcpp::export]]
+IntegerVector ud_compare(NumericVector x, NumericVector y,
+                         CharacterVector xn, CharacterVector yn)
+{
+  bool swapped = false;
+
+  if (y.size() > x.size()) {
+    std::swap(x, y);
+    std::swap(xn, yn);
+    swapped = true;
+  }
+
+  IntegerVector out(x.size());
+  for (std::string &attr : x.attributeNames())
+    out.attr(attr) = x.attr(attr);
+
+  ut_unit *ux = ut_parse(sys, ut_trim(xn[0], enc), enc);
+  ut_unit *uy = ut_parse(sys, ut_trim(yn[0], enc), enc);
+
+  for (int i=0, j=0; i < x.size(); i++, j++) {
+    if (j == y.size())
+      j = 0;
+    out[i] = ut_compare(ut_scale(x[i], ux), ut_scale(y[j], uy));
+  }
+
+  ut_free(ux);
+  ut_free(uy);
+  if (swapped)
+    out = -out;
+  return out;
+}
+
 // [[Rcpp::export]]
 NumericVector ud_convert(NumericVector val, CharacterVector from, CharacterVector to) {
   ut_unit *u_from = ut_parse(sys, ut_trim(from[0], enc), enc);

tests/testthat/test_arith.R

@@ -15,6 +15,26 @@ test_that("we can compare vectors with equal units", {
   expect_false(any(x > y))
   expect_false(any(x != y))
   expect_true(all(x != z))
+
+  expect_error(x == set_units(1, "kg"))
+})
+
+test_that("vectors are correctly recycled in comparisons", {
+  x <- 1:4 * as_units("m")
+  y <- 1:2 * as_units("m")
+  res <- drop_units(x) == drop_units(y)
+  expect_equal(x == y, res)
+  expect_equal(y == x, res)
+
+  y <- 1:3 * as_units("m")
+  expect_warning(res <- drop_units(x) == drop_units(y))
+  expect_warning(expect_equal(x == y, res))
+  expect_warning(expect_equal(y == x, res))
+})
+
+test_that("aliases are correctly handled in comparisons (#339)", {
+  expect_true(as_units("foot") == as_units("feet"))
+  expect_true(as_units("foot") == as_units("ft"))
 })
 
 test_that("we can scale units with scalars", {

tests/testthat/test_misc.R

@@ -101,7 +101,7 @@ test_that("all.equal works", {
   expect_true(all.equal(set_units(1, m/s), set_units(3.6, km/h)))
   expect_true(set_units(1, m/s) == set_units(3.6, km/h))
   expect_true(all.equal(set_units(3.6, km/h), set_units(1, m/s)))
-  expect_false(set_units(3.6, km/h) == set_units(1, m/s))
+  expect_true(set_units(3.6, km/h) == set_units(1, m/s))
   expect_false(isTRUE(all.equal(set_units(1, m), 1)))
 })
 

tests/testthat/test_user_conversion.R

@@ -22,7 +22,7 @@ test_that("we can convert between units with a user-defined function", {
   expect_equal(bananas + 3 * apples, (6 + 3 * 2 / 3) * as_units("banana"))
 
   # check dimensionless
-  install_unit("person", "unitless")
+  install_unit("person", "1")
   persons <- set_units(3, person)
   expect_true(persons == set_units(3, 1))