"Some attributes are incompatible" error

[Enchufa2]

I has been notified that the {quantities} package has a broken vignette on CRAN. This is the error:

library(quantities)
#> Loading required package: units
#> udunits system database from /usr/share/udunits
#> Loading required package: errors
iris.q <- iris
for (i in 1:4)
  quantities(iris.q[,i]) <- list("cm", iris.q[,i] * 0.05)

library(dplyr, warn.conflicts=FALSE)
iris.q %>%
  group_by(Species) %>%
  summarise_all(mean)
#> Error: Problem with `summarise()` input `Sepal.Length`.
#> x Input `Sepal.Length` must return compatible vectors across groups
#> ℹ Input `Sepal.Length` is `(function (x, ...) ...`.
#> ℹ Result type for group 1 (Species = "setosa"): <quantities>.
#> ℹ Result type for group 2 (Species = "versicolor"): <quantities>.

How are groups with the same class incompatible? I tracked down the issue a little bit. A {quantities} object has two parts: {units} and {errors}. Just with {units}, it works ok:

iris.q %>%
  mutate_at(1:4, drop_errors) %>%
  group_by(Species) %>%
  summarise_all(mean)
#> # A tibble: 3 x 5
#>   Species    Sepal.Length Sepal.Width Petal.Length Petal.Width
#>   <fct>              [cm]        [cm]         [cm]        [cm]
#> 1 setosa            5.006       3.428        1.462       0.246
#> 2 versicolor        5.936       2.770        4.260       1.326
#> 3 virginica         6.588       2.974        5.552       2.026

But not with {errors}:

iris.q %>%
  mutate_at(1:4, drop_units) %>%
  group_by(Species) %>%
  summarise_all(mean)
#> Error: Problem with `summarise()` input `Sepal.Length`.
#> x Input `Sepal.Length` must return compatible vectors across groups
#> ℹ Input `Sepal.Length` is `(function (x, ...) ...`.
#> ℹ Result type for group 1 (Species = "setosa"): <errors>.
#> ℹ Result type for group 2 (Species = "versicolor"): <errors>.

I continued tracking the issue down to vec_ptype_common, which works for {units}:

c(set_units(1, m), set_units(3, m))
#> Units: [m]
#> [1] 1 3
vctrs::vec_ptype_common(set_units(1, m), set_units(3, m))
#>  [m]

but not for {errors}:

c(set_errors(1, 0.1), set_errors(3, 0.2))
#> Errors: 0.1 0.2
#> [1] 1 3
vctrs::vec_ptype_common(set_errors(1, 0.1), set_errors(3, 0.2))
#> Error: Can't combine `..1` <errors> and `..2` <errors>.
#> x Some attributes are incompatible.
#> ℹ The author of the class should implement vctrs methods.
#> ℹ See <https://vctrs.r-lib.org/reference/faq-error-incompatible-attributes.html>.

We are going to implement glue for {vctrs}, as we discussed in r-quantities/units#239, but shouldn't this work? And if so, it would be great if the fix makes it into the planned patch release, so that I don't have to apply a quick workaround to retain {quantities} on CRAN.

[Enchufa2]

Actually, it doesn't work for {units} either with different but compatible units:

c(set_units(1, km), set_units(3, m))
#> Units: [km]
#> [1] 1.000 0.003
vctrs::vec_ptype_common(set_units(1, km), set_units(3, m))
#> Error: Can't combine `..1` <units> and `..2` <units>.
#> x Some attributes are incompatible.
#> ℹ The author of the class should implement vctrs methods.
#> ℹ See <https://vctrs.r-lib.org/reference/faq-error-incompatible-attributes.html>.

[hadley]

(@Enchufa2 apologies for the hassle this has cause — somehow we didn't see this in our revdep checks, and then CRAN accepted the package surprisingly quickly. We're working hard to resolve as many of the problems as possible from our end)

[lionel-]

@Enchufa2 I think we'll need to implement vctrs methods for units, errors, and perhaps quantities. I'm now studying your packages and will start working on PRs tomorrow.

[Enchufa2]

Very much appreciated. I would have started it myself, but I have my hands full with other duties.

[Enchufa2]

I read the FAQs and started something here, but clearly I don't know what I'm doing, and vec_ptype2 and vec_cast are not enough or I'm missing something.

library(errors)
library(dplyr, warn.conflicts=FALSE)

iris.q <- iris
for (i in 1:4)
  errors(iris.q[,i]) <- iris.q[,i] * 0.05

iris.q %>%
  group_by(Species) %>%
  summarise_all(mean)
#> # A tibble: 3 x 5
#>   Species    Sepal.Length Sepal.Width Petal.Length Petal.Width
#>   <fct>         <[(err)]>   <[(err)]>    <[(err)]>   <[(err)]>
#> 1 setosa        5.006(NA)   3.428(NA)    1.462(NA)   0.246(NA)
#> 2 versicolor    5.936(NA)    2.77(NA)     4.26(NA)   1.326(NA)
#> 3 virginica     6.588(NA)   2.974(NA)    5.552(NA)   2.026(NA)

[lionel-]

Right, this class also needs to implement vec_proxy() and vec_restore() for which there is currently less documentation. See https://vctrs.r-lib.org/reference/vec_proxy.html.

In the errors class, the errors are a vectorised attribute. vctrs supports these via data frames (which are treated by vctrs as vectors of rows rather than columns). So the errors class needs a data frame proxy.

A proxy is simply the internal data representation for your class, and with a data frame proxy you're saying that each element is defined by the data contained in multiple columns, row by row. The vec_restore() method is used to undo this transformation and restore the relevant attributes.

At the same time, the errors are not used for comparison and identity, and so they shouldn't be part of the equality proxy, which needs to be overidden (vec_equal_proxy() defaults to vec_proxy()).

Here is my work in progress: r-quantities/errors@master...lionel-:add-vctrs. I'm happy to finish this and send a PR, but let me know if you'd like to pick this up.

[Enchufa2]

Makes sense. I was afraid I had to basically duplicate every single base method implemented for the class into its vec_something counterpart. I suppose that for {units} it will be simpler, because the attribute is not vectorised. But then we have mixed_units, that is a list of heterogeneous units. And {quantities}, that combines and orchestrates {units} and {errors} (the class is c("quantities", "units", "errors") for that case).

But step by step. I think that, with a working reference implementation for {errors}, things will be much clearer for me, so I'd appreciate it very much if you can finish that PR. Could you please rebase your current work using the feature/vctrs branch? Basically, I moved stuff to the tidyverse.R file to implement the dynamic registration mechanism, as you suggested in r-quantities/errors#37. Also there's this ignominious attempt that you can of course kill with fire.

[lionel-]

Yes for quantities I think we'll be in maturing/experimental territory. It's linked to #945. We'd like to make this part of vctrs more robust for 0.4.0, and your classes seem like a good use case for us.

[Enchufa2]

Great. I bought some time by sending a quick patch for {quantities} to CRAN to prevent WARNs in vignettes.

[lionel-]

@Enchufa2 Would you like to have integration tests with dplyr in your packages? To do this, we can either add dplyr to Suggests, or use the same strategy I use for sf if you prefer to avoid the soft dependency:

vctrs/tests/testthat/test-type-sf.R

Lines 2 to 13 in 29baab9

	# Avoids adding `sf` to Suggests
	testthat_import_from("sf", c(
	"st_sf",
	"st_sfc",
	"st_point",
	"st_bbox",
	"st_precision",
	"st_crs",
	"st_linestring",
	"st_as_sf",
	"st_multipoint"
	))

In the latter case the tests are only run when the machine happens to have dplyr installed (usually, your local machine).

The main advantage of adding to Suggests is that you'll be included in our revdep checks. The disadvantage is that it might slow down your CI builds a bit. Though dplyr is now a much lighter dependency than it used to.

[Enchufa2]

@Enchufa2 Would you like to have integration tests with dplyr in your packages? To do this, we can either add dplyr to Suggests

Yes, and ok to add it to Suggests. But in that case there should be a skip_if_not_installed too.