Should vec_proxy_equal.vctrs_rcrd() exist and be recursive?

[DavisVaughan]

We currently have a vec_proxy.vctrs_rcrd() method that just turns the rcrd into a data frame (with no recursion of proxying the types). This is consistent with our data frame proxy method, which also doesn't recurse right now.

vctrs/R/type-rcrd.R

Lines 32 to 35 in f9afbff

	#' @export
	vec_proxy.vctrs_rcrd <- function(x, ...) {
	new_data_frame(unclass(x))
	}

But this is what gets used for vec_proxy_equal() too, and the fact that it doesn't recurse is a big problem. Consider what happens if you nest a rcrd in a rcrd. The lack of recursion can actually bomb R in some cases.

library(vctrs)

inner <- new_rcrd(list(x = 1:5, y = 1:5), class = "inner")
outer <- new_rcrd(list(a = inner, b = inner), class = "outer")

format.inner <- function(x, ...) {
  paste0(field(x, "x"), "/", field(x, "y"))
}
format.outer <- function(x, ...) {
  paste0("[", format(field(x, "a")), ", ", format(field(x, "b")), "]")
}

inner
#> <inner[5]>
#> [1] 1/1 2/2 3/3 4/4 5/5
outer
#> <outer[5]>
#> [1] [1/1, 1/1] [2/2, 2/2] [3/3, 3/3] [4/4, 4/4] [5/5, 5/5]

# i think this is ok, data frame proxy isn't recursive right now
vec_proxy(outer)
#>     a   b
#> 1 1/1 1/1
#> 2 2/2 2/2
#> 3 3/3 3/3
#> 4 4/4 4/4
#> 5 5/5 5/5

# this is a problem
vec_proxy_equal(outer)
#>     a   b
#> 1 1/1 1/1
#> 2 2/2 2/2
#> 3 3/3 3/3
#> 4 4/4 4/4
#> 5 5/5 5/5

# it results in things like this bombing R!
# vec_equal(outer, outer)

vec_proxy_equal.vctrs_rcrd <- function(x, ...) {
  vec_proxy_equal(new_data_frame(unclass(x)))
}

# that looks better
vec_proxy_equal(outer)
#>   x y x y
#> 1 1 1 1 1
#> 2 2 2 2 2
#> 3 3 3 3 3
#> 4 4 4 4 4
#> 5 5 5 5 5

vec_equal(outer, outer)
#> [1] TRUE TRUE TRUE TRUE TRUE

I think we need a vec_proxy_equal() method defined like the one above for this to work right.

Note: we also have this weird vec_proxy_compare.vctrs_rcrd method, which looks like it is very similar to the vec_proxy() method, so maybe this can be removed? It also isn't recursive so it has the same issues.

vctrs/R/type-rcrd.R

Lines 142 to 147 in f9afbff

	# FIXME
	#' @export
	vec_proxy_compare.vctrs_rcrd <- function(x, ...) {
	new_data_frame(vec_data(x), n = length(x))
	}

[lionel-]

I vaguely remember that proxying recursively caused conceptual issues for restoration. Maybe that was with column operations though (which are now abandoned), or with df proxies that have more columns than the actual data, I don't remember.

[DavisVaughan]

proxying recursively caused conceptual issues for restoration

Yea but that is about vec_proxy() + vec_restore()

I'm talking about vec_proxy_equal() / vec_proxy_compare(), which you never restore from afterwards.

If you look at data frames right now, we don't recursively perform vec_proxy() but we do recursively perform vec_proxy_equal(). I think I'm just suggesting we do the same with rcrds.

[lionel-]

makes sense