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

Confusing behavior when converting StepRange{<:Dates.TimeType,<:Dates.Period} to numeric range #10451

Closed
jiahao opened this issue Mar 8, 2015 · 6 comments
Labels
dates Dates, times, and the Dates stdlib module

Comments

@jiahao
Copy link
Member

jiahao commented Mar 8, 2015

Conversions of ranges involving dates and times do not always preserve length.

julia> D1 = Dates.Date(2015,1,1):Dates.Date(2015,3,31)
2015-01-01:1 day:2015-03-31

julia> length(D1)
90

julia> length(int(D1))
90

julia> length(float(D1))
90

julia> D2 = Dates.DateTime(2015,1,1,0,0,0):Dates.DateTime(2015,3,31,0,0,0)
2015-01-01T00:00:00:1 day:2015-03-31T00:00:00

julia> length(D2)
90

julia> length(int(D2))
7689600001

julia> length(float(D2))
7689600001

The problem appears to lie in the conversion of the step, int(Dates.Day(1)) == float(Dates.Day(1)) == int(Dates.Millisecond(1)) == 1, which results in an incommensurate conversion of the entire range. (7689600001 is the correct answer if the step is 1 millisecond.)

@quinnj
Copy link
Member

quinnj commented Mar 8, 2015

I'm not sure what we could do differently here, apart from defining custom conversion methods for StepRange{<:TimeType}s, which would have their own, not totally intuitive conversion semantics. Right now, when you call int(D2), I find it pretty natural that it's going to call int on the start, step, and stop, so the result 63555753600000:1:63563443200000 seems natural.

I'm not sure if it would totally solve the problem here, but I've thought of extending UnitRange to not just Integers which would mean Date(2015,1,1):Date(2015,1,31) would be a UnitRange{Date} instead of the representation as a StepRange that we do currently. Like I said, not sure if that would really help the case here, but something I've thought about before.

@jiahao
Copy link
Member Author

jiahao commented Mar 8, 2015

The problem here is that int(D2) is no longer a convenient vectorized syntax for map(int, D2). This could be yet another reason to get rid of the former syntax altogether (c.f. #1470).

cc: @JeffBezanson - this could be another good example of why vectorization is bad.

@nalimilan
Copy link
Member

If ranges are to be taken seriously as AbstractArrays, then changing their element type must either 1) return an array equal to map(Type, D2), or even better when possible 2) return a range which when collect()ed gives the same result as 1. Both possibilities preserve the length of the original range.

As you say, getting rid of vectorized conversion functions like int() would solve this problem by forcing you to call explicitly map(Int, D2) (equivalent to 1), or convert to an integer range manually.

@ivarne
Copy link
Member

ivarne commented Mar 8, 2015

The problem seems to be that we're inconsistent on what int() means when applied to a Date and period type. Maybe all conversions should use milliseconds as basis?

@jiahao
Copy link
Member Author

jiahao commented Mar 14, 2015

As discussed with @quinnj offline, the problem is that int here is used for two incompatible meanings:
a. int(::DateTime) returns the Int64 internal representation of a DateTime, and
b. int(Hour(1)) returns the numeric part of the unitful quantity Hour(1).

One possible solution is to disallow both meanings in favor of another function, possibly reinterpret:
a. Use instead reinterpret(Int64, ::DateTime) to return the Int64 internal representation of a DateTime.
b. Use reinterpret(Int64, ::TimePeriod) to return the Int64 internal representation of a TimePeriod, which I believe has units of milliseconds.

@quinnj quinnj added the dates Dates, times, and the Dates stdlib module label Aug 18, 2015
@TotalVerb
Copy link
Contributor

This is fixed by #19920.

@quinnj quinnj closed this as completed Jan 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dates Dates, times, and the Dates stdlib module
Projects
None yet
Development

No branches or pull requests

5 participants