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

IDate should not convert to Date when met with integers #1528

Closed
MichaelChirico opened this issue Feb 9, 2016 · 6 comments
Closed

IDate should not convert to Date when met with integers #1528

MichaelChirico opened this issue Feb 9, 2016 · 6 comments

Comments

@MichaelChirico
Copy link
Member

There's a discrepancy between Date and IDate w.r.t. interacting with non-IDate objects, namely:

D <- as.Date("2016-02-09")
class(D + 1:3)

I <- as.IDate(D)
class(I + 1:3) #forced to `Date`

Is this intentional?

For reference, here is the +.Date method:

function (e1, e2) 
{
    coerceTimeUnit <- function(x) as.vector(round(switch(attr(x, 
        "units"), secs = x/86400, mins = x/1440, hours = x/24, 
        days = x, weeks = 7 * x)))
    if (nargs() == 1) 
        return(e1)
    if (inherits(e1, "Date") && inherits(e2, "Date")) 
        stop("binary + is not defined for \"Date\" objects")
    if (inherits(e1, "difftime")) 
        e1 <- coerceTimeUnit(e1)
    if (inherits(e2, "difftime")) 
        e2 <- coerceTimeUnit(e2)
    structure(unclass(e1) + unclass(e2), class = "Date")
}
@MichaelChirico MichaelChirico changed the title IDate should not convert to date when met with integers IDate should not convert to Date when met with integers Feb 10, 2016
@MichaelChirico
Copy link
Member Author

MichaelChirico commented Apr 26, 2016

Made progress on this but I can't seem to get the method to export properly. See this branch.

Any suggestions? I'm at a loss.

@jangorecki
Copy link
Member

jangorecki commented Apr 26, 2016

Looks like extra backtick in namespace file, easy visible when compare to master

export(as.IDate,as.ITime,IDateTime`)

@MichaelChirico
Copy link
Member Author

oh, thanks! vestige from when I was trying to add it to exports.
On Apr 26, 2016 7:44 AM, "Jan Gorecki" [email protected] wrote:

extra backtick in namespace file, easy visible when compare to master

export(as.IDate,as.ITime,IDateTime`)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#1528 (comment)

arunsrinivasan added a commit that referenced this issue May 9, 2016
Closes #1528, adds S3 method for `+.IDate`
mattdowle added a commit that referenced this issue Sep 20, 2016
…eck.attributes=FALSE to ignore column name differences. #1528
@WetRobot
Copy link

Hi,

I'm having some difficulties with the current -.IDate:

> d <- "2015-01-01"
> as.IDate(d) - 1.5
Error in as.Date.numeric(e2) : 'origin' must be supplied

because of forced coercion to Date. At the same time

> str(as.Date(d) - 1.5)
Date[1:1], format: "2014-12-30"

Might I suggest modifying the forced coercion to Date

return(`-.Date`(as.Date(e1),as.Date(e2)))

to just

return(`-.Date`(as.Date(e1), e2))

?

Then,

> str(as.IDate(d) - as.IDate(d))
int 0

> str(as.IDate(d) - 1L)
IDate[1:1], format: "2014-12-31"

> str(as.IDate(d) - as.Date(d))
IDate[1:1], format: "1970-01-01"
Warning message:
In str(as.IDate(d) - as.Date(d)) :
Incompatible methods ("-.IDate", "-.Date") for "-"

> str(as.IDate(d) - 1.0)
Date[1:1], format: "2014-12-31"

Side note: the warning message should perhaps not be a problem, since e.g.

> str(as.Date(d) - as.IDate(d))
Date[1:1], format: "1970-01-01"
Warning message:
In str(as.Date(d) - as.IDate(d)) :
Incompatible methods ("-.Date", "-.IDate") for "-"

and I at least would expect as.Date(d) - as.IDate(d) and as.IDate(d) - as.Date(d) to produce the same result even if it is broken.

@MichaelChirico
Copy link
Member Author

Would you mind quickly trying it from my branch which has a different implementation of -.IDate?

@WetRobot
Copy link

You appear to have included difftime as a possible output and input of e2 - matter of taste perhaps, though of course more closely modeled after -.Date. Doubles work in your implementation as well, though should the output then be Date to conserve information?

> d <- "2015-01-01"

> str(as.IDate(d) - as.IDate(d))
Class 'difftime'  atomic [1:1] 0
  ..- attr(*, "units")= chr "days"

> str(as.IDate(d) - 1L)
 IDate[1:1], format: "2014-12-31"

> str(as.IDate(d) - as.Date(d)) 
 IDate[1:1], format: "1970-01-01"
Warning message:
In str(as.IDate(d) - as.Date(d)) :
  Incompatible methods ("-.IDate", "-.Date") for "-"

> str(as.IDate(d) - 1.0)
 IDate[1:1], format: "2014-12-31"

> str(as.IDate(d) - as.date(1))
 IDate[1:1], format: "2014-12-31"
Warning message:
In str(as.IDate(d) - as.date(1)) :
  Incompatible methods ("-.IDate", "Ops.date") for "-"

> diff <- as.Date(d) - (as.Date(d) - 10)
> str(as.IDate(d) - diff)
 IDate[1:1], format: "2014-12-22"
Warning message:
In str(as.IDate(d) - diff) :
  Incompatible methods ("-.IDate", "Ops.difftime") for "-"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants