HenrikBengtsson / R.oo

R package: R.oo - R Object-Oriented Programming with or without References
https://cran.r-project.org/package=R.oo
20 stars 1 forks source link

`$<-.Object` breaks on custom objects with class `Object` #21

Closed arunsrinivasan closed 4 years ago

arunsrinivasan commented 4 years ago
require(R.oo)
x <- list(b=1L)
class(x) <- "Object"
x$c <- "bbb"
# Error in exists(name, envir = envir, inherits = FALSE) : 
#   use of NULL environment is defunct

packageVersion("R.oo")
# [1] ‘1.23.0’
getRversion()
# [1] ‘3.6.1’

When writing a data.frame to disk using arrow::write_parquet, it fails if R.oo is loaded because internally it tries to convert it to arrow format, and it results in an object of class Table, Object, R6 which later on calls $<-.Object from R.oo instead of whatever method it should call if R.oo wasn't loaded and ends up with the same error.

The only way I could think of to avoid this is to overwrite $<-.Object in my package. But it seems like something that could be fixed upstream instead of having all users to write a package.

CC: @romainfrancois

For reference, the code that'd fail with R.oo loaded:

require(R.oo)
require(arrow)
df <- data.frame(x=1)
write_parquet(df, tempfile())
# Error in exists(name, envir = envir, inherits = FALSE) : 
#   use of NULL environment is defunct

If you remove the first line and rerun in a new session, it would work just fine.

HenrikBengtsson commented 4 years ago

I can reproduce:

> library(arrow)
> df <- data.frame(x=1)
> write_parquet(df, tempfile())
>
> library(R.oo)
> write_parquet(df, tempfile())
Error in exists(name, envir = envir, inherits = FALSE) : 
  use of NULL environment is defunct

Quick workaround

> registerS3method("$<-", "Object", function(...) NextMethod())
> write_parquet(df, tempfile())
>

What R.oo could do to help

I've already got a special case for $.Object in R.oo;

> R.oo::`$.Object`
structure(function (this, name) 
{
    if (!.isRoo(this)) 
        return(NextMethod())
    {
        .subset2Internal(this, name = name, exact = TRUE)
    }
}, S3class = "Object")
<environment: namespace:R.oo>

with

> R.oo:::.isRoo
function (x) 
is.environment(attr(x, ".env"))
<environment: namespace:R.oo>

It exists to work around similar clashes that might occur with rJava, cf. https://github.com/HenrikBengtsson/R.oo/blob/31f9a557aebcb189c8b1fe4aa6111779c1db291a/R/zzz.rJava-tweaks.R. I could probably add something similar for $<-.Object.

Misc.

I noticed that arrow doesn't register all of its S3 methods, e.g. to_arrow() is only an internal S3 generic. I'm not sure if this is intentional by design or a mistake.

PS. <ocd warning but ...>Pls try to use library() instead of require(); there are enough beginners who find examples online that use require() where they shouldn't, resulting in lots of confusion and unnecessary back and forth communication</ocd warning but ...>

HenrikBengtsson commented 4 years ago

Fixed in commit f804e2c;

remotes::install_github("HenrikBengtsson/R.oo@develop")
> library(R.oo)
> library(arrow)
> df <- data.frame(x=1)
> write_parquet(df, nullfile())
> 
HenrikBengtsson commented 4 years ago

FYI, R.oo 1.24.0 fixing this just hit CRAN