%% doesn't work correctly?

[edzer]

This doesn't seem right:

> set_units(1.98, m) %% set_units(1, foot)
0.98 [m]
> set_units(1.98, m) %% set_units(set_units(1, foot), m)
0.1512 [m]

[Enchufa2]

These three should be the same, right?

library(units)
#> udunits database from /usr/share/udunits/udunits2.xml
x <- set_units(1.98, m)
y <- set_units(1, foot)
x %% y
#> 0.98 [m]
x %% set_units(y, m)
#> 0.1512 [m]
drop_units(x) %% drop_units(set_units(y, m))
#> [1] 0.1512

as well as these

set_units(x, foot) %% y
#> 0.496063 [foot]
set_units(x, foot) %% set_units(y, m)
#> 0.09526299 [foot]
drop_units(set_units(x, foot)) %% drop_units(y)
#> [1] 0.496063

[edzer]

Yes, I think so.

[Enchufa2]

But should the result have units? Or unitless?

[edzer]

units. I tried to compute my length in feet and inches:

> library(units)
udunits database from /usr/share/xml/udunits/udunits2.xml
> set_units(1.98, m) %/% set_units(set_units(1, foot), m)
6 [1]
> rem = set_units(1.98, m) %% set_units(set_units(1, foot), m)
> set_units(rem, `in`)
5.952756 [in]

however this also seems wrong:

> set_units(1.98, m) %/% set_units(1, foot)
3.28084 [1]

[Enchufa2]

Forget about my previous comment, deleted now. You are right. This should be always true:

all.equal(x, y * (x %/% y) + x %% y)

[Enchufa2]

This doesn't work properly either:

x <- set_units(3, m)
y <- set_units(1.4, foot)
x %/% y
#> 6.56168 [1]

[edzer]

library(units)
# udunits database from /usr/share/xml/udunits/udunits2.xml
x <- set_units(3, m)
y <- set_units(1.4, foot)
x %/% y
# 7 [1]
ym = set_units(y, m)
x %/% y
# 7 [1]
x
# 3 [m]
y
# 1.4 [foot]
x %/% ym
# 7 [1]
x
# 3 [m]
ym
# 0.42672 [m]
ym * (x %/% ym) + (x %% ym)
# 3 [m]
set_units(y * (x %/% y) + (x %% y), m)
# 3 [m]

[edzer]

This now breaks on the tests, with

  > a = set_units(1:5, m)
  > a %/% a
  Units: [1]
  [1] 1 1 1 1 1
  > a %/% set_units(2)
  Error: cannot convert 1 into m

but my feeling is that that is correct, and needs removed from the tests.

[Enchufa2]

Hmmm... That example may not be the best one, but it should work. The commit above improves things a little bit for a particular case (convertible units), but these operations should work, in principle, with any combination of units.

I worked a bit on this problem some weeks ago, but I entered a rabbit hole at some point and undid the work. What I have is a small test script of things that should work:

library(units)
x <- set_units(3, m^2)
y <- set_units(1.4, foot)
x %/% y

drop_units(x) %% drop_units(set_units(y, m))
x %% y
x %% set_units(y, m)
x %% drop_units(set_units(y, m))
x %% set_units(drop_units(set_units(y, m)), 1)
x %% set_units(drop_units(set_units(y, m)), 1)
drop_units(x) %% set_units(y, m) # error

drop_units(set_units(x, foot^2)) %% drop_units(y)
set_units(x, foot^2) %% y
set_units(x, foot^2) %% set_units(y, m)
set_units(x, foot^2) %% drop_units(y)
set_units(x, foot^2) %% set_units(drop_units(y), 1)
drop_units(set_units(x, foot^2)) %% y # error

all.equal(x, y * (x %/% y) + x %% y)

We could:

1. roll an update with aa387a1, but then I would revert 7622f33, and we would need to remove that example to avoid the check error for now;
2. or revert both aa387a1, 7622f33 and aim at a complete solution to this bug for a future release.

[edzer]

these operations should work, in principle, with any combination of units.

I don't think that %/% or %% should work for incompatible units. "How often can we put one foot in one square metre?" doesn't make sense.

[Enchufa2]

You can put

set_units(1, foot) / set_units(1, m^2)
#> 0.3048 [1/m]

So the same for %/% and %%. They're just the integer part and remainder of the operation above.

And from the fact that this equality should hold,

all.equal(x, y * (x %/% y) + x %% y)

• x %/% y must have the same units as x / y.
• x %% y must have the same units as x.

[edzer]

Feels odd, but please go ahead (fixing this or reverting my change).

[Enchufa2]

My previous attempt at this tried to make conversions and forward with NextMethod, but I think that the most sensible thing to do is to perform a regular division and extract the proper part afterwards. Let me try.

[Enchufa2]

Admittedly not the most efficient implementation, but it works. :) I've added the test cases above to the test suite.

[Enchufa2]

Package quantities fail with this change a couple of tests. I'll take a look, they probably need to be removed. In such a case, I need to send an update to CRAN too. I'll run full revdep checks too just in case.

[Enchufa2]

It required the same fix in package errors. Let me know when you plan to send the update to CRAN, and I'll send an update for errors and quantities too.

[edzer]

I'm ready to go. Shall I inform CRAN that breakage is expected in pkg errors and quantities, or do you want to submit them first?

[Enchufa2]

Yes, please, submit informing about the expected breakage. I'll submit in a few hours when I come back to my laptop. Thanks!