r-lib / vctrs

Generic programming with typed R vectors
https://vctrs.r-lib.org
Other
287 stars 66 forks source link

make vec_restore(<AsIs>) apply AsIs to the restored object, not its proxy #1903

Open mjskay opened 9 months ago

mjskay commented 9 months ago

I ran into a weird ggplot2 bug when using I(...) with some aesthetics that I eventually traced back to a bug in the implementation of vec_restore.AsIs(). It currently applies the "AsIs" class to the proxy of the wrapped type without actually restoring the underlying type. I believe this is a simple fix.

Before:

x = posterior::rvar(1)

vctrs::vec_slice(x, 1)
#> rvar<1>[1] mean ± sd:
#> [1] 1 ± NA

vctrs::vec_slice(I(x), 1)
#> [[1]]
#> [[1]]$index
#> [1] 1
#> 
#> [[1]]$nchains
#> [1] 1
#> 
#> [[1]]$draws
#>   [,1]
#> 1    1

Created on 2023-12-21 with reprex v2.0.2

After:

x = posterior::rvar(1)

vctrs::vec_slice(x, 1)
#> rvar<1>[1] mean ± sd:
#> [1] 1 ± NA

vctrs::vec_slice(I(x), 1)
#> rvar<1>[1] mean ± sd:
#> [1] 1 ± NA

Created on 2023-12-21 with reprex v2.0.2

mjskay commented 9 months ago

Ah, I also had to adjust the definition of asis_restore() to pass some tests --- it now prevents "AsIs" from being added multiple times. Although it's worth noting that its implementation is now exactly the same as I(), so it could perhaps be replaced with that function if desired.