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

Implementation of S3 method for zoo class #70

Merged
merged 3 commits into from Jun 1, 2014

Conversation

RomanTsegelskyi
Copy link
Contributor

Implementation of S3 method for zoo class
I'm not sure if to format dates with panderOption date

colnames(c.tab) <- trunc(time(x))

or

colnames(c.tab) <- format(trunc(time(x)), panderOptions('date'))

Basic examples:

x.Date <- as.Date("2003-02-01") + c(1, 3, 7, 9, 14) - 1
x <- zoo(rnorm(5), x.Date)
pander(x)
z <- zoo(cbind(foo = rnorm(5), bar = rnorm(5)))
pander(z)

@daroczig
Copy link
Member

Great, thanks! IMHO we should use the date from panderOptions, so that's pretty cool, what I am not sure about is the layout. 5 dates can fit on the screen horizontally, but if the time series also includes time, not just date, or we have more values, it's pretty screwed up. What do you think about a vertical layout?

@RomanTsegelskyi
Copy link
Contributor Author

I've changed that for vertical alignment

@daroczig
Copy link
Member

Great, thank you @RomanTsegelskyi!

Two things came to my mind while running your second example:

  1. It fails, probably because time(z) is not a Date object, so not format.Date but format.default is called.
  2. And I think we should not call format in this S3 method, but please implement pander.Date method instead, just to make this solution more robust. That could be reused later.

@RomanTsegelskyi
Copy link
Contributor Author

I fixed that error. Specifying format= will only influence rendering for dates, but will not change anything for other classes.
I don't clearly understand how can implementation of pander.Date be used. I thought that pander methods do printing. So should I implement pandoc.Date and pandoc.Date.return methods then? And how to use them in that case? Apply function to a vector?
And just out of curiosity, why is it not a good idea to use format?

@daroczig
Copy link
Member

daroczig commented Jun 1, 2014

Thanks!

About pander.Date: I though that it might be a good idea to implement it first, then using it in pander.zoo due to DRY principles. This way you do not have to format(..., format = panderOptions('date')) even in other S3 methods, but simply call pander.return(...).

About this later: we already have pander.POSIXct and pander.POSIXlt, so pander.Date might be the very same implementation. Even the code could be cleaned up at https://github.com/Rapporter/pander/blob/master/R/S3.R#L373 like writing the function only once, then simply pander.POSIXlt <- pander.POSIXct and pander.Date <- pander.POSIXct with the usual Roxygen tags above those.

But please note that I have just realized that the already existing implementation for POSIXct and POSIXlt is wrong :( Try e.g. pander(as.POSIXct(Sys.Date() - 1:10)).

So probably we should merge this pull request, then let's decide how to deal with dates properly.

@daroczig daroczig merged commit 9658828 into Rapporter:master Jun 1, 2014
@RomanTsegelskyi RomanTsegelskyi deleted the zoo branch July 11, 2014 03:10
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.

None yet

2 participants