Should functions or the user be responsible for coercing an S4 object
argument containing the proper object (and thus should below be
considered a bug in the packages or not)?
The example is with RSQLite but the same thing happens with RMySQL, and
other DBI packages.
> library("RSQLite") Loading required package: DBI
> m <- dbDriver("SQLite")
> con <- dbConnect(m)
> setClass("SQLConPlus",
contains=c("SQLiteConnection","integer"))
[1] "SQLConPlus"
> conPlus <- new("SQLConPlus", con, 1)
> dbListTables(con)
character(0)
> dbListTables(conPlus)
Error in sqliteExecStatement(con, statement, bind.data) :
RS-DBI driver: (invalid dbManager handle)
> dbListTables(as(conPlus, "SQLiteConnection"))
character(0)
>
The problem is happening in sqliteExecStatement which does
conId <- as(con, "integer")
but con only *contains* an SQLiteConnection and the other integer
causes confusion. If the line were
conId <- as(as(con, "SQLiteConnection"), "integer")
everything works.
I can work around this, but I am curious where responsibility for this
coercion should be.
Paul Gilbert
===================================================================================
La version fran?aise suit le texte anglais.
------------------------------------------------------------------------------------
This email may contain privileged and/or confidential in...{{dropped:26}}
* On 2008-09-15 at 08:56 -0400 Paul Gilbert wrote:> Should functions or the user be responsible for coercing an S4 object > argument containing the proper object (and thus should below be > considered a bug in the packages or not)? > > The example is with RSQLite but the same thing happens with RMySQL, and > other DBI packages. > > > library("RSQLite") Loading required package: DBI > > m <- dbDriver("SQLite") > > con <- dbConnect(m) > > setClass("SQLConPlus", contains=c("SQLiteConnection","integer")) > [1] "SQLConPlus" > > conPlus <- new("SQLConPlus", con, 1) > > dbListTables(con) > character(0) > > dbListTables(conPlus) > Error in sqliteExecStatement(con, statement, bind.data) : > RS-DBI driver: (invalid dbManager handle) > > dbListTables(as(conPlus, "SQLiteConnection")) > character(0) > > > > The problem is happening in sqliteExecStatement which does > conId <- as(con, "integer") > but con only *contains* an SQLiteConnection and the other integer > causes confusion. If the line were > conId <- as(as(con, "SQLiteConnection"), "integer") > everything works. > > I can work around this, but I am curious where responsibility for this > coercion should be.Well, you've created a class that is-a SQLiteConnection *and* is-a integer. The fact that the as() method dispatch doesn't match that of SQLiteConnection should really be that surprising. I don't see how this could be the responsibility of the author of the class you've subclassed. I would also question why SQLConPlus is extending integer. That seems like a very strange choice. Why not extend SQLiteConnection and add extra slots as you like. The dispatch will in this case be much easier to reason about. + seth -- Seth Falcon | http://userprimary.net/user/
A couple more comments... * On 2008-09-15 at 10:07 -0700 Seth Falcon wrote:> > The example is with RSQLite but the same thing happens with > > RMySQL, and other DBI packages.The use of as() within the various DBI packages should be re-evaluated. I suspect some of that code was among the first to make heavy use of S4. As S4 has evolved and become better documented and understood, the DBI packages may not always have had a chance to keep up.> > > library("RSQLite") Loading required package: DBI > > > m <- dbDriver("SQLite") > > > con <- dbConnect(m) > > > setClass("SQLConPlus", contains=c("SQLiteConnection","integer")) > > [1] "SQLConPlus" > > > conPlus <- new("SQLConPlus", con, 1) > > > dbListTables(con) > > character(0) > > > dbListTables(conPlus)In the latest R-devel code (svn r46542), this behaves differently (and works as you were hoping). I get: library("RSQLite") setClass("SQLConPlus", contains=c("SQLiteConnection","integer")) dd = data.frame(a=1:3, b=letters[1:3]) con = new("SQLConPlus", dbConnect(SQLite(), ":memory:"), 11L) dbWriteTable(con, "t1", dd) dbListTables(con) dbDisconnect(con) I know that the methods package has been undergoing some improvements recently, so it is not entirely surprising that behavior has changed. I think the new behavior is desirable as it follows the rule that the order of the superclasses listed in contains is used to break ties when multiple methods match. Here, there are two coerce() methods (invoked via as()) one for SQLiteConnection and one, I believe auto-generated, for integer. Since SQLiteConnection comes first, it is chosen. Indeed, if you try the following, you get the error you were originally seeing: setClass("SQLConMinus", contains=c("integer", "SQLiteConnection")) con2 = new("SQLConMinus", dbConnect(SQLite(), ":memory:"), 11L) > as(con, "integer") [1] 15395 2 > as(con2, "integer") [1] 11 >> Why not extend SQLiteConnection and add extra slots as you like. > The dispatch will in this case be much easier to reason about.This is still appropriate advice. In general, inheritance should be used with care, and multiple inheritance should be used with multiple care. Using representation() to add additional slots likely makes more sense here. + seth -- Seth Falcon | http://userprimary.net/user/> sessionInfo()R version 2.8.0 Under development (unstable) (--) i386-apple-darwin9.4.0 locale: C attached base packages: [1] stats graphics grDevices datasets utils methods base other attached packages: [1] RSQLite_0.7-0 DBI_0.2-4