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.

Sign up for GitHub

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

Remove problematic Dates conversions #19920

Merged
merged 1 commit into from
Jan 25, 2017
Merged

Remove problematic Dates conversions #19920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 4 additions & 24 deletions base/dates/conversions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,30 +31,10 @@ Base.convert(::Type{DateTime}, dt::Date) = DateTime(UTM(value(dt)*86400000))
Base.convert(::Type{Date}, dt::DateTime) = Date(UTD(days(dt)))
Base.convert(::Type{Time}, dt::DateTime) = Time(Nanosecond((value(dt) % 86400000) * 1000000))

"""
convert{T<:Real}(::Type{T}, dt::DateTime) -> T
Converts a DateTime value `dt` to a number of type `T`. The returned value corresponds to the number of Rata Die milliseconds since epoch.
See `convert(DateTime, x::Real)` for inverse.
"""
Base.convert{R<:Real}(::Type{R},x::DateTime) = convert(R,value(x))
"""
convert{T<:Real}(::Type{T}, dt::Date) -> T
Converts a Date value `dt` to a number of type `T`. The returned value corresponds to the number of Rata Die days since epoch.
See `convert(Date, x::Real)` for inverse.
"""
Base.convert{R<:Real}(::Type{R},x::Date) = convert(R,value(x))
"""
convert{T<:Real}(::Type{DateTime}, x::T) -> DateTime
Converts a number of type `T` to a DateTime. `x` should be the number of Rata Die milliseconds since epoch.
See `convert(Int64,dt::DateTime)` for inverse.
"""
Base.convert{R<:Real}(::Type{DateTime}, x::R) = DateTime(UTM(x))
"""
convert{T<:Real}(::Type{Date}, x::T) -> Date
Converts a number of type `T` to a Date. `x` should be the number of Rata Die days since epoch.
See `convert(Int64,dt::Date)` for inverse.
"""
Base.convert{R<:Real}(::Type{Date}, x::R) = Date(UTD(x))
Base.convert(::Type{DateTime},x::Millisecond) = DateTime(Dates.UTInstant(x)) # Converts Rata Die milliseconds to a DateTime
Base.convert(::Type{Millisecond},dt::DateTime) = Millisecond(value(dt)) # Converts DateTime to Rata Die milliseconds
Base.convert(::Type{Date},x::Day) = Date(Dates.UTInstant(x)) # Converts Rata Die days to a Date
Base.convert(::Type{Day},dt::Date) = Day(value(dt)) # Converts Date to Rata Die days

### External Conversions
const UNIXEPOCH = value(DateTime(1970)) #Rata Die milliseconds for 1970-01-01T00:00:00
Expand Down
14 changes: 5 additions & 9 deletions base/dates/periods.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,30 +37,26 @@ for period in (:Year, :Month, :Week, :Day, :Hour, :Minute, :Second, :Millisecond
""" $period(v)
end
end
# Now we're safe to define Period-Number conversions
# Anything an Int64 can convert to, a Period can convert to
Base.convert{T<:Number}(::Type{T},x::Period) = convert(T,value(x))
Base.convert{T<:Period}(::Type{T},x::Real) = T(x)

#Print/show/traits
Base.string{P<:Period}(x::P) = string(value(x),_units(x))
Base.show(io::IO,x::Period) = print(io,string(x))
Base.zero{P<:Period}(::Union{Type{P},P}) = P(0)
Base.one{P<:Period}(::Union{Type{P},P}) = P(1)
Base.one{P<:Period}(::Union{Type{P},P}) = 1 # see #16116
Base.typemin{P<:Period}(::Type{P}) = P(typemin(Int64))
Base.typemax{P<:Period}(::Type{P}) = P(typemax(Int64))

# Default values (as used by TimeTypes)
"""
default(p::Period) -> Period

Returns a sensible "default" value for the input Period by returning `one(p)` for Year,
Month, and Day, and `zero(p)` for Hour, Minute, Second, and Millisecond.
Returns a sensible "default" value for the input Period by returning `T(1)` for Year,
Month, and Day, and `T(0)` for Hour, Minute, Second, and Millisecond.
"""
function default end

default{T<:DatePeriod}(p::Union{T,Type{T}}) = one(p)
default{T<:TimePeriod}(p::Union{T,Type{T}}) = zero(p)
default{T<:DatePeriod}(p::Union{T,Type{T}}) = T(1)
default{T<:TimePeriod}(p::Union{T,Type{T}}) = T(0)

(-){P<:Period}(x::P) = P(-value(x))
Base.isless{P<:Period}(x::P,y::P) = isless(value(x),value(y))
Expand Down
15 changes: 11 additions & 4 deletions base/dates/ranges.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,19 @@

# Override default step; otherwise it would be Millisecond(1)
Base.colon{T<:DateTime}(start::T, stop::T) = StepRange(start, Day(1), stop)
Base.colon{T<:Time}(start::T, stop::T) = StepRange(start, Second(1), stop)
Base.colon{T<:Date}(start::T, stop::T) = StepRange(start, Day(1), stop)
Base.colon{T<:Time}(start::T, stop::T) = StepRange(start, Second(1), stop)

Base.range(start::DateTime, len::Integer) = range(start, Day(1), len)
Base.range(start::Date, len::Integer) = range(start, Day(1), len)

(::Type{StepRange{T,R}}){T<:Dates.DatePeriod,R<:Real}(start, step, stop) =
throw(ArgumentError("must specify step as a Period when constructing Dates ranges"))

# Given a start and end date, how many steps/periods are in between
guess(a::DateTime,b::DateTime,c) = floor(Int64,(Int128(b) - Int128(a)) / toms(c))
guess(a::Date,b::Date,c) = Int64(div(Int64(b - a), days(c)))
len(a::Time,b::Time,c) = Int64(div(Int64(b - a), tons(c)))
guess(a::DateTime,b::DateTime,c) = floor(Int64,(Int128(value(b)) - Int128(value(a)))/toms(c))
guess(a::Date,b::Date,c) = Int64(div(value(b - a),days(c)))
len(a::Time,b::Time,c) = Int64(div(value(b - a), tons(c)))
function len(a,b,c)
lo, hi, st = min(a,b), max(a,b), abs(c)
i = guess(a,b,c)-1
Expand Down
13 changes: 13 additions & 0 deletions base/deprecated.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1773,4 +1773,17 @@ end)

@deprecate EachLine(stream, ondone) EachLine(stream, ondone=ondone)

# These conversions should not be defined, see #19896
@deprecate convert{T<:Number}(::Type{T}, x::Dates.Period) convert(T, Dates.value(x))
@deprecate convert{T<:Dates.Period}(::Type{T}, x::Real) T(x)
@deprecate convert{R<:Real}(::Type{R}, x::Dates.DateTime) R(Dates.value(x))
@deprecate convert{R<:Real}(::Type{R}, x::Dates.Date) R(Dates.value(x))
@deprecate convert(::Type{Dates.DateTime}, x::Real) Dates.DateTime(Dates.Millisecond(x))
@deprecate convert(::Type{Dates.Date}, x::Real) Dates.Date(Dates.Day(x))

function colon{T<:Dates.Period}(start::T, stop::T)
depwarn("$start:$stop is deprecated, use $start:$T(1):$stop instead.", :colon)
colon(start, T(1), stop)
end

# End deprecations scheduled for 0.6
32 changes: 15 additions & 17 deletions test/dates/conversions.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,10 @@ let t = Dates.Period[Dates.Week(2), Dates.Day(14), Dates.Hour(14*24), Dates.Minu
Pi = typeof(t[i])
for j = 1:length(t)
@test t[i] == t[j]
@test Int(convert(Pi,t[j])) == Int(t[i])
end
for j = i+1:length(t)
Pj = typeof(t[j])
tj1 = t[j] + one(Pj)
tj1 = t[j] + Pj(1)
@test t[i] < tj1
@test_throws InexactError Pi(tj1)
@test_throws InexactError Pj(Pi(typemax(Int64)))
Expand All @@ -74,8 +73,7 @@ let t = Dates.Period[Dates.Week(2), Dates.Day(14), Dates.Hour(14*24), Dates.Minu
end
end
@test Dates.Year(3) == Dates.Month(36)
@test Int(convert(Dates.Month, Dates.Year(3))) == 36
@test Int(convert(Dates.Year, Dates.Month(36))) == 3
@test_throws ErrorException Int(Dates.Month(36)) # eventually change to MethodError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only warns, not throws - strangely it doesn't fail unless you run the tests with JULIA_CPU_CORES=1 though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably comment this out in #20226, but I wonder whether there's a test-system bug here.

@test Dates.Year(3) < Dates.Month(37)
@test_throws InexactError convert(Dates.Year, Dates.Month(37))
@test_throws InexactError Dates.Month(Dates.Year(typemax(Int64)))
Expand All @@ -89,20 +87,20 @@ let dt = DateTime(1915,1,1,12)
@test Dates.julian2datetime(julian) == dt
end

# Conversions to/from numbers
# "Conversions" to/from numbers
a = Dates.DateTime(2000)
b = Dates.Date(2000)
@test convert(Real,b) == 730120
@test convert(Float64,b) == 730120.0
@test convert(Int32,b) == 730120
@test convert(Real,a) == 63082368000000
@test convert(Float64,a) == 63082368000000.0
@test convert(Int64,a) == 63082368000000
@test convert(DateTime,63082368000000) == a
@test convert(DateTime,63082368000000.0) == a
@test convert(Date,730120) == b
@test convert(Date,730120.0) == b
@test convert(Date,Int32(730120)) == b
@test Dates.value(b) == 730120
@test Dates.value(a) == 63082368000000
@test convert(Dates.DateTime, Dates.Millisecond(63082368000000)) == a
@test convert(Dates.Millisecond, a) == Dates.Millisecond(63082368000000)
@test Dates.DateTime(Dates.UTM(63082368000000)) == a
@test Dates.DateTime(Dates.UTM(63082368000000.0)) == a
@test convert(Dates.Date, Dates.Day(730120)) == b
@test convert(Dates.Day, b) == Dates.Day(730120)
@test Dates.Date(Dates.UTD(730120)) == b
@test Dates.Date(Dates.UTD(730120.0)) == b
@test Dates.Date(Dates.UTD(Int32(730120))) == b

dt = Dates.DateTime(2000,1,1,23,59,59,50)
t = Dates.Time(dt)
Expand All @@ -111,4 +109,4 @@ t = Dates.Time(dt)
@test Dates.second(t) == 59
@test Dates.millisecond(t) == 50
@test Dates.microsecond(t) == 0
@test Dates.nanosecond(t) == 0
@test Dates.nanosecond(t) == 0
31 changes: 0 additions & 31 deletions test/dates/periods.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,6 @@ ns = Dates.Nanosecond(1)
@test Dates.Millisecond(ms) == ms
@test Dates.Microsecond(us) == us
@test Dates.Nanosecond(ns) == ns
@test typeof(Int8(y)) <: Int8
@test typeof(UInt8(y)) <: UInt8
@test typeof(Int16(y)) <: Int16
@test typeof(UInt16(y)) <: UInt16
@test typeof(Int32(y)) <: Int32
@test typeof(UInt32(y)) <: UInt32
@test typeof(Int64(y)) <: Int64
@test typeof(UInt64(y)) <: UInt64
@test typeof(Int128(y)) <: Int128
@test typeof(UInt128(y)) <: UInt128
@test typeof(convert(BigInt,y)) <: BigInt
@test typeof(convert(BigFloat,y)) <: BigFloat
@test typeof(convert(Complex,y)) <: Complex
@test typeof(convert(Rational,y)) <: Rational
@test typeof(Float16(y)) <: Float16
@test typeof(Float32(y)) <: Float32
@test typeof(Float64(y)) <: Float64
@test Dates.Year(convert(Int8,1)) == y
@test Dates.Year(convert(UInt8,1)) == y
@test Dates.Year(convert(Int16,1)) == y
Expand Down Expand Up @@ -227,20 +210,6 @@ test = ((((((((dt + y) - m) + w) - d) + h) - mi) + s) - ms)
@test zero(Dates.Second(10)) == Dates.Second(0)
@test zero(Dates.Millisecond) == Dates.Millisecond(0)
@test zero(Dates.Millisecond(10)) == Dates.Millisecond(0)
@test one(Dates.Year) == Dates.Year(1)
@test one(Dates.Year(10)) == Dates.Year(1)
@test one(Dates.Month) == Dates.Month(1)
@test one(Dates.Month(10)) == Dates.Month(1)
@test one(Dates.Day) == Dates.Day(1)
@test one(Dates.Day(10)) == Dates.Day(1)
@test one(Dates.Hour) == Dates.Hour(1)
@test one(Dates.Hour(10)) == Dates.Hour(1)
@test one(Dates.Minute) == Dates.Minute(1)
@test one(Dates.Minute(10)) == Dates.Minute(1)
@test one(Dates.Second) == Dates.Second(1)
@test one(Dates.Second(10)) == Dates.Second(1)
@test one(Dates.Millisecond) == Dates.Millisecond(1)
@test one(Dates.Millisecond(10)) == Dates.Millisecond(1)
@test Dates.Year(-1) < Dates.Year(1)
@test !(Dates.Year(-1) > Dates.Year(1))
@test Dates.Year(1) == Dates.Year(1)
Expand Down
4 changes: 2 additions & 2 deletions test/dates/ranges.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,10 +478,10 @@ let n=100000
return b
end

@test length(Dates.Year(1):Dates.Year(10)) == 10
@test length(Dates.Year(1):Dates.Year(1):Dates.Year(10)) == 10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to specify the step here? Can't we just define the following to get the old behaviour?

Base.colon{T<:Period}(start::T, stop::T) = StepRange(start, T(1), stop)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's dangerous to define ranges with unitful quantities if you don't explicitly specify the step, because with unitful quantities we tend to think that there's an underlying physical reality independent of the units you choose to express the quantities in. Why should 1m:10m give a range of length 10 whereas 1000mm:10000mm gives you a range of length 9001? Both of those ranges are defined with the same endpoints.

In my mind the only reasonable approach is to force the user to specify the step or the desired length of the range.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with requiring the user to specify the step, but this change should be documented.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle I agree that you should define step for unitful ranges. However since ranges involving periods currently require that the endpoints and the step are of the same unit I think this change is mostly just inconvenient. Overall though I'm happy with the change if it leads to less confusion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the sense in supporting it when you're typing the endpoints explicitly: I think it's pretty obvious what the user wants with Minute(1):Minute(60). But I worry about code like this:

function foo(a::Period, b::Period)
    ap, bp = promote(a, b)
    ap:bp
end

where now knowing what you'll get relies on a fair amount of mental gymnastics.

Either way, I agree completely with the perspective that we need a transitional strategy. I suspect a deprecation would suffice?

@test length(Dates.Year(10):Dates.Year(-1):Dates.Year(1)) == 10
@test length(Dates.Year(10):Dates.Year(-2):Dates.Year(1)) == 5
@test_throws OverflowError length(typemin(Dates.Year):typemax(Dates.Year))
@test_throws OverflowError length(typemin(Dates.Year):Dates.Year(1):typemax(Dates.Year))
@test_throws MethodError Dates.Date(0):Dates.DateTime(2000)
@test_throws MethodError Dates.Date(0):Dates.Year(10)
@test length(range(Dates.Date(2000),366)) == 366
Expand Down