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

Handling negative timestamps #63

Closed
jpmarindiaz opened this Issue May 7, 2017 · 12 comments

Comments

Projects
None yet
3 participants
@jpmarindiaz
> anytime(-863715600)
[1] "-2362803-01-03"
> anydate(-863715600)
[1] "-2362803-01-03"
@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 7, 2017

Owner

How is that not user error? In which context do you see these?

Let's remember where anytime() comes from -- variants of ISO dates and times. Accepting numeric-only input is a fringe case. I have a hard time seeing this as valid, sorry.

Owner

eddelbuettel commented May 7, 2017

How is that not user error? In which context do you see these?

Let's remember where anytime() comes from -- variants of ISO dates and times. Accepting numeric-only input is a fringe case. I have a hard time seeing this as valid, sorry.

@russellpierce

This comment has been minimized.

Show comment
Hide comment
@russellpierce

russellpierce May 7, 2017

Contributor

Would it be suitable to directly throw an error in the face of clearly invalid input?

Contributor

russellpierce commented May 7, 2017

Would it be suitable to directly throw an error in the face of clearly invalid input?

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 7, 2017

Owner

That though crossed my mind too. What's holding me back is that each additional test adds an extra layer...

At some point you just got to treat your users as adults. A negative time point (not talking durations here) is simply absurd.

Owner

eddelbuettel commented May 7, 2017

That though crossed my mind too. What's holding me back is that each additional test adds an extra layer...

At some point you just got to treat your users as adults. A negative time point (not talking durations here) is simply absurd.

@jpmarindiaz

This comment has been minimized.

Show comment
Hide comment
@jpmarindiaz

jpmarindiaz May 8, 2017

Hi @eddelbuettel

So the background is this: I was working with data in unix timestamp format (I think).
I figured anydate would do it to convert a date from a unix timestamp, as I had used it before for dates that happened to be after 1970-01-01.
Unix timestamps are quite common, maybe it is my lack of experience but I don't see why this might be considered wrong user input.

Anyway, I managed to get the expected result with this (from your stackoverflow answer)

 as.POSIXct(-863715600, origin="1970-01-01") #  "1942-08-19 PWT"

I am not sure if this background helps or was rather a silly question/issue.

Hi @eddelbuettel

So the background is this: I was working with data in unix timestamp format (I think).
I figured anydate would do it to convert a date from a unix timestamp, as I had used it before for dates that happened to be after 1970-01-01.
Unix timestamps are quite common, maybe it is my lack of experience but I don't see why this might be considered wrong user input.

Anyway, I managed to get the expected result with this (from your stackoverflow answer)

 as.POSIXct(-863715600, origin="1970-01-01") #  "1942-08-19 PWT"

I am not sure if this background helps or was rather a silly question/issue.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 8, 2017

Owner

True, true, time before the epoch is expressed as negative offset.

But where is your data coming from? "Raw numbers" like this mostly come from current/live measurement. "Old" datatimes before 1970 I have only ever seen expressed as textual.

Owner

eddelbuettel commented May 8, 2017

True, true, time before the epoch is expressed as negative offset.

But where is your data coming from? "Raw numbers" like this mostly come from current/live measurement. "Old" datatimes before 1970 I have only ever seen expressed as textual.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 8, 2017

Owner

Actually, crap. I exposes a logic bug. For 'small numbers' I switch to dates. Can't do that.

Because

R> utctime(0)
[1] "1970-01-01"
R>

it should also work for, say, utctime(3600) and show us an hour off. But that is interpreted as date now. I need to look into that.

Owner

eddelbuettel commented May 8, 2017

Actually, crap. I exposes a logic bug. For 'small numbers' I switch to dates. Can't do that.

Because

R> utctime(0)
[1] "1970-01-01"
R>

it should also work for, say, utctime(3600) and show us an hour off. But that is interpreted as date now. I need to look into that.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 8, 2017

Owner

As for the bug report, I need to at least document better what works or doesn't work.

Owner

eddelbuettel commented May 8, 2017

As for the bug report, I need to at least document better what works or doesn't work.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 8, 2017

Owner

It works for explicit date requests, at least here:

R> utcdate( 0 )
[1] "1970-01-01"
R> utcdate( -1 )
[1] "1969-12-31"
R> utcdate( -365 )
[1] "1969-01-01"
R> utcdate( -365 * 5 )
[1] "1965-01-02"
R> utcdate( -365 * 50 )
[1] "1920-01-14"
R> 
Owner

eddelbuettel commented May 8, 2017

It works for explicit date requests, at least here:

R> utcdate( 0 )
[1] "1970-01-01"
R> utcdate( -1 )
[1] "1969-12-31"
R> utcdate( -365 )
[1] "1969-01-01"
R> utcdate( -365 * 5 )
[1] "1965-01-02"
R> utcdate( -365 * 50 )
[1] "1920-01-14"
R> 
@jpmarindiaz

This comment has been minimized.

Show comment
Hide comment
@jpmarindiaz

jpmarindiaz May 8, 2017

I got the data from dbpedia sparql, birthDates of people.

I got the data from dbpedia sparql, birthDates of people.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 8, 2017

Owner

I found a possible solution on the commute in, but it needs testing. We have a problem because we clearly want to recognise '20170508' as well. But maybe differentiating between anytime() and anydate() does the trick.

As a general rule, for numeric data you can just load as a numeric vector and then do the convert you found, which essentially just sets the class attribute.

Owner

eddelbuettel commented May 8, 2017

I found a possible solution on the commute in, but it needs testing. We have a problem because we clearly want to recognise '20170508' as well. But maybe differentiating between anytime() and anydate() does the trick.

As a general rule, for numeric data you can just load as a numeric vector and then do the convert you found, which essentially just sets the class attribute.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 8, 2017

Owner

I wasn't thinking this through earlier, and you are quite frankly correct. This should always have worked.

I was a little too accomodating in the becoming allowing anytime(20170508). This will be interpreted as seconds past epoch. If you want a date, use anydate(20170508). While an interface change, it seems sensible.

I will add some more documentation to clarify. Thanks for filing this bug report. I think anytime will be a better package when I have this fully implemented (ie docs, tests, ...) and released.

Owner

eddelbuettel commented May 8, 2017

I wasn't thinking this through earlier, and you are quite frankly correct. This should always have worked.

I was a little too accomodating in the becoming allowing anytime(20170508). This will be interpreted as seconds past epoch. If you want a date, use anydate(20170508). While an interface change, it seems sensible.

I will add some more documentation to clarify. Thanks for filing this bug report. I think anytime will be a better package when I have this fully implemented (ie docs, tests, ...) and released.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel May 29, 2017

Owner

Addressed in #65 which I just merged.

Owner

eddelbuettel commented May 29, 2017

Addressed in #65 which I just merged.

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