Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setclass() added to data.table.r #844

Closed
wants to merge 1 commit into from
Closed

setclass() added to data.table.r #844

wants to merge 1 commit into from

Conversation

rsaporta
Copy link
Contributor

Motivated by the question raised in #839, it might be useful to have a quick wrapper for

  setattr(x, "class", c(newclass, class(x))

@rsaporta
Copy link
Contributor Author

If others agree it's useful, I'll go ahead and write up news/doc

@gsee
Copy link

gsee commented Sep 30, 2014

I love it, but I think append should be FALSE by default.

@rsaporta
Copy link
Contributor Author

@gsee, you are probably right.

In my Rprofile, I have this function named as appendClass_( ) But I personally use the convention of ending function names with _ when modifying by reference. In this package, the convetion is to name the function set*.

Would it make sense to use an alternate name with append=TRUE by default?
Or perhaps just leave it as setclass and change default behavior

@gsee
Copy link

gsee commented Sep 30, 2014

agreed. With the current implementation, if I call setclass(DT, "data.table") it will give a warning and then call setDT(DT). setDT has this code: if (is.data.table(x)) return(invisible(x)). Therefore, if you set the class of an object to "data.table" (instead of c("data.table", "data.frame")), then calling setclass(DT, "data.table") or setDT(DT) will return an object that is not a data.frame. I think setDT should actually return a c("data.table", "data.frame") even if it is given a "data.table"

@rsaporta
Copy link
Contributor Author

Ah, I think I see where the confusion is:

then calling setclass(DT, "data.table") or setDT(DT) will return an object that is not a data.frame.

I do not think that is the case.
Anytime is.data.table(x) == TRUE then is.data.frame(x) must be TRUE -- unless of course the user specifically removed the data.frame class.

so in the case of if (is.data.table(x)) return(invisible(x)) , if we look at is(x) I am sure we will see something like c(data.table, data.frame, list, etc)

@rsaporta
Copy link
Contributor Author

Separately, perhaps the warning in setclass(x, "data.table") is unnecessary. We can simply call setDT() quietly

@gsee
Copy link

gsee commented Sep 30, 2014

> DF <- data.frame(A=1:5)
> setattr(DF, "class", "data.table")
> class(DF)
[1] "data.table"
> is(DF)
[1] "data.table" "data.frame" "list"       "oldClass"   "vector"  

But...

> is.data.table(DF)
[1] TRUE
> is.data.frame(DF)
[1] FALSE

and

> setDT(DF)
> class(DF)
[1] "data.table"
> is.data.frame(DF)
[1] FALSE

@gsee
Copy link

gsee commented Sep 30, 2014

So I think it'd be important to guard against doing this accidentally

I think I misunderstood. You're saying the user should guard against making the accident, not that setDT() and/or setclass() should try to fix it?

@rsaporta
Copy link
Contributor Author

no, @gsee, you had understood correctly. I had misunderstood. Specifically, I didn't realize that setDT(DF); is.data.frame(DF) returned FALSE.

Personally, I think it should return TRUE and that setDT() might need to be modified, unless there may be some reason not to do this, of which I am unaware.

@mattdowle and/or @arunsrinivasan , thoughts regarding output of setDT()

@arunsrinivasan
Copy link
Member

@rsaporta right, setDT could fix the class for the case you and Garrett mention.

But I'm not sure why we need setclass. setnames is specifically for data.frames and data.tables only. And to convert a data.frame to data.table, we've setDT now, which is easier than doing setclass(DF, c("data.table", "data.frame")).

@rsaporta
Copy link
Contributor Author

rsaporta commented Oct 4, 2014

@Arun, I use a function like setclass often. Usually, for inheriting from data.table. (Eg, I may use something like setclass(DT.qres, "query.results") and then print(DT.qres) deploys the appropriate print function.

It's just a handy wrapper to save a few keystrokes.

@arunsrinivasan
Copy link
Member

So setclass works only with data.tables, and would append classes provided as input before the already existing classes by default, and errors when input is not a data.table pointing to setattr?

@jangorecki
Copy link
Member

I've run into an error produced (most probably) by altered class of data.table.
It occurs when rendering Rmd file, the chunk in Rmd contains the following:

ticker <- market.api.process("kraken",c("BTC","EUR"),"ticker") # Rbitcoin 0.9.3 function returns DT with additional class
pander::pandoc.table(ticker)

and it results following R Markdown error:

Error in dimnames.data.table(x) : data.table inherits from data.frame (from v1.5) but this data.table does not. Has it been created manually (e.g. by using structure' rather than 'data.table') or saved to disk using a prior version of data.table? The correct class is c('data.table','data.frame'). Calls: <Anonymous> ... has.rownames -> rownames -> dimnames -> dimnames.data.table Execution halted

and the class of ticker:

class(ticker)
# [1] "btc.ticker" "data.table" "data.frame"

Using the following solve the problem, so it must be related to additional class in DT:

pander::pandoc.table(setDT(setDF(ticker)))

also related to already closed #839

@arunsrinivasan
Copy link
Member

Closing this, as it's stale at this point. Have added an issue for setclass() function, #1412.

@arunsrinivasan arunsrinivasan deleted the setclass branch March 2, 2016 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants