.S3 doesn't work correctly if class contains a dot (".")

[bspalat]

Hello there and thanks for an awesome package!

I stumbled upon the fact that import(..., .S3 = TRUE) doesn't handle S3 methods for classes like data.frame. Apparently, the class doesn't get processed completely which is even hinted at with a warning. Short example:

# File: module.R
f <- function(x) UseMethod("f")

f.integer <- function(x) TRUE
f.data.frame <- function(x) FALSE

# works here
# f(1L)
# f(data.frame())

but doesn't work when imported:

# File: main.R
import::from(module.R, f, .S3 = TRUE)
# > Warning messages:
# > 1: In assign(paste(genname, class, sep = "."), method, envir = table) :
# >  only the first element is used as variable name
# > 2: In (function (..., deparse.level = 1)  :
# >   number of columns of result is not a multiple of vector length (arg 1)

f(1L)
f(data.frame())
# > Error in UseMethod("f") : 
# > no applicable method for 'f' applied to an object of class "data.frame"

methods(f)
# > [1] f.data*    f.integer*

The feature is labeled as experimental and the above isn't difficult to circumvent, but I suppose the behaviour isn't intentional.

Once again, thanks for all the work :)

[torfason]

Thanks for the report. This clearly has to do with how R resolves ambiguous (more than one period) names into <method>.<class>. Definitely not intentional behavior, and if someone were to contribute a fix in a PR, along with unit tests, I would try to merge it and possibly roll a bugfix release (say 1.3.2). But I personally don't know exactly how to fix this and would probably have to spend some time to figure that out, so I'm unlikely to find the time myself, unfortunately.

[J-Moravec]

Ah, I was wondering why the S3 support failed on me recently and went back to registering manually.

I will research it and fix it.

But note that there might be no way to distinguish between the hypothetical cases mentioned in https://github.com/rticulate/import/pull/65#issue-1213228948

A hypothetical t.test.data.frame might then consist of t generic for the test.data.frame class, t.test generic for the data.frame class and t.test.data generic for the frame class and there is no easy way to resolve this ambiguity.

[J-Moravec]

I was able to find a bug in my original implementation.

Note that we are still assuming that the generics doesn't have a dot in name. This is probably a reasonable assumption for new user-generated generics, but some historical generics might suffer. Users can always identify and register these issues manually.

[torfason]

Awesome, thanks for the fix, we should see this on CRAN within a day or two, se the PR (#87) for detail.