From e7262afb7b028dbc2067e60875bb9a8169d5f15e Mon Sep 17 00:00:00 2001
From: Tomasz Kalinowski <>
Date: Mon, 22 Jan 2018 19:00:45 -0500
Subject: [PATCH] alternative solution to #89; #91

With this implementation, the symbolic units are preserved so they can be printed correctly, and assigning a unit like "mg/kg" no longer changes the numeric value of the target vector
---
 R/make_units.R                   | 17 +++++++++++++----
 tests/testthat/test_conversion.R | 16 ++++++++++++----
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/R/make_units.R b/R/make_units.R
index 66e030d8..ab35e457 100644
--- a/R/make_units.R
+++ b/R/make_units.R
@@ -283,6 +283,15 @@ units_eval_env$ln <- function(x) base::log(x)
 units_eval_env$lg <- function(x) base::log(x, base = 10)
 units_eval_env$lb <- function(x) base::log(x, base = 2)
 
+units_eval_env$`/` <- function(x, y) {
+  # don't cancel units, just combine them
+  units <- structure(list(
+    numerator = c(units(x)$numerator, units(y)$denominator), 
+    denominator = c(units(y)$numerator, units(x)$denominator)), 
+    class = "symbolic_units")
+  structure(1, units = units, class = "units")
+}
+
 
 #' @export
 #' @rdname as_units
@@ -338,10 +347,10 @@ See ?as_units for usage examples.")
           "Did you try to supply a value in a context where a bare expression was expected?"
         ), call. = FALSE ))
   
-#  if(as.numeric(unit) %not_in% c(1, 0)) # 0 if log() used. 
-#    stop(call. = FALSE,
-#"In ", sQuote(deparse(x)), " the numeric multiplier ", sQuote(as.numeric(unit)), " is invalid. 
-#Use `install_conversion_constant()` to define a new unit that is a multiple of another unit.")
+ if(as.numeric(unit) %not_in% c(1, 0)) # 0 if log() used.
+   stop(call. = FALSE,
+"In ", sQuote(deparse(x)), " the numeric multiplier ", sQuote(as.numeric(unit)), " is invalid.
+Use `install_conversion_constant()` to define a new unit that is a multiple of another unit.")
   
   structure(as.numeric(unit), units = units(unit), class = "units")
 }
diff --git a/tests/testthat/test_conversion.R b/tests/testthat/test_conversion.R
index 2fe37df4..049e919f 100644
--- a/tests/testthat/test_conversion.R
+++ b/tests/testthat/test_conversion.R
@@ -139,11 +139,19 @@ test_that("dim propagates", {
 test_that("conversion to dimensionless with prefix works (g/kg)", {
 	a_orig <- a <- 1:10
 	units(a) = as_units("mg/kg")
-	expect_equal(as.numeric(a), a_orig/1e6)
+	# why should assigning units change the numeric value?
+	expect_equal(as.numeric(a), a_orig)
+	
 	units(a) = as_units("kg/mg")
-	expect_equal(a, a_orig)
+	expect_equal(a, a_orig/1e12)
+	
 	units(a) = as_units("g/g")
-	expect_equal(a, a_orig)
+	expect_equal(a, a_orig/1e6)
+	
 	units(a) = as_units("kg/g")
-	expect_equal(a, a_orig * 1000)
+	expect_equal(a, a_orig / 1e9)
+	
+	# back to original
+	units(a) <- as_units("mg/kg")
+	expect_equal(as.numeric(a), a_orig)
 })