Skip to content

Dates.Time compatibility with StepRangeLen by adding converting constructors #58846

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

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
49 changes: 43 additions & 6 deletions stdlib/Dates/docs/src/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
DocTestSetup = :(using Dates)
```

The `Dates` module provides two types for working with dates: [`Date`](@ref) and [`DateTime`](@ref),
representing day and millisecond precision, respectively; both are subtypes of the abstract [`TimeType`](@ref).
The `Dates` module provides three types for representing dates and times:
[`Date`](@ref), [`DateTime`](@ref), and [`Time`](@ref) measured with
day, millisecond and nanosecond precision, respectively;
all are subtypes of the abstract [`TimeType`](@ref).
The motivation for distinct types is simple: some operations are much simpler, both in terms of
code and mental reasoning, when the complexities of greater precision don't have to be dealt with.
For example, since the [`Date`](@ref) type only resolves to the precision of a single date (i.e.
Expand All @@ -14,7 +16,7 @@ time, and leap seconds are unnecessary and avoided.

Both [`Date`](@ref) and [`DateTime`](@ref) are basically immutable [`Int64`](@ref) wrappers.
The single `instant` field of either type is actually a `UTInstant{P}` type, which
represents a continuously increasing machine timeline based on the UT second [^1]. The
represents a monotonically increasing machine timeline based on the UT second [^1]. The
[`DateTime`](@ref) type is not aware of time zones (*naive*, in Python parlance),
analogous to a *LocalDateTime* in Java 8. Additional time zone functionality
can be added through the [TimeZones.jl package](https://github.com/JuliaTime/TimeZones.jl/), which
Expand All @@ -26,6 +28,12 @@ The ISO standard, however, states that 1 BC/BCE is year zero, so `0000-12-31` is
`0001-01-01`, and year `-0001` (yes, negative one for the year) is 2 BC/BCE, year `-0002` is 3
BC/BCE, etc.

The [`Time`](@ref) is also an immutable [`Int64`](@ref) wrapper, also based on the UT second [^1],
and represents the time of day according to the conventional 24-hour clock, starting at midnight,
and ending the instant one nanosecond prior to midnight. Time is periodic, and wraps around at
midnight (see [TimeType-Period arithmetic](#TimeType-Period-Arithmetic)).


[^1]:
The notion of the UT second is actually quite fundamental. There are basically two different notions
of time generally accepted, one based on the physical rotation of the earth (one full rotation
Expand All @@ -39,7 +47,7 @@ BC/BCE, etc.

## Constructors

[`Date`](@ref) and [`DateTime`](@ref) types can be constructed by integer or [`Period`](@ref)
[`Date`](@ref), [`DateTime`](@ref) and [`Time`](@ref) types can be constructed by integer or [`Period`](@ref)
types, by parsing, or through adjusters (more on those later):

```jldoctest
Expand Down Expand Up @@ -78,6 +86,16 @@ julia> Date(Dates.Year(2013),Dates.Month(7),Dates.Day(1))

julia> Date(Dates.Month(7),Dates.Year(2013))
2013-07-01

julia> Time(12)
12:00:00

julia> Time(12, 30, 59, 1, 0, 2)
12:30:59.001000002

julia> Time(Hour(12), Minute(30), Second(59), Millisecond(1), Nanosecond(2))
12:30:59.001000002

```

[`Date`](@ref) or [`DateTime`](@ref) parsing is accomplished by the use of format strings. Format
Expand Down Expand Up @@ -459,6 +477,25 @@ julia> collect(dr)
2014-07-29
```

Time is periodic, and wraps around at midnight:

```jldoctest
julia> Time(23) + Hour(1)
00:00:00

julia> r = range(Time(0), step = Hour(9), length = 5)
Time(0):Hour(9):Time(12)
Copy link
Member

Choose a reason for hiding this comment

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

This seems... odd.

It violates extrema(range) == minmax(first(range), last(range))

But the definition of extrema assumes this identity, and therefore gets the wrong answer.

julia> r = range(Time(0), step = Hour(9), length = 5)
Time(0):Hour(9):Time(12)

julia> extrema(r)
(Time(0), Time(12))

Copy link
Author

Choose a reason for hiding this comment

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

Ok that's nasty odd, but it looks to me like faulty short-cutting minimum and maximum methods for StepRangeLen. We could fix those to actually iterate the sequence. For comparison, this gives the expected result:

julia> extrema(identity, r)
(Time(0), Time(18))

I suppose another unhappy oddity that initially drew your attention is that the returned range violates the implied expectation (see ?show - The representation used by show ... should be parseable Julia code when possible) that its repr could be used to construct an equivalent range, but in this case it stops early (length 2) rather than wrapping around to the full length of 5:

julia> length(eval(Meta.parse(repr(r)))), length(r)
(2, 5)

We could fix this too - there is already special handing in the show method to show ranges such StepRangeLen(0, 0, 0) with constructor syntax.

Are ranges expected to be monotonic? Both the above oddities are due to that unsatisfied expectation. But nothing normative in the docs for range, AbstractRange, or StrepRangeLen (or the types OrdinalRange, StepRange ) firmly specifies monotonicity. Extracting key references:

  • range - ... a specialized array with evenly spaced elements and optimized storage
  • AbstractRange - Supertype for linear ranges ...
  • StepRangeLen - a range r where r[i] produces values of type T ...
  • OrdinalRange - ... with spacings of type S. The steps should be always-exact multiples of oneunit
  • StepRange - the step between each element is constant. Neither T nor S should be floating point types.

If so, ideally that expectation should be codified somehow, or at least documented. Here is an example from another package that already behaves similarly to the proposed behavior for Time:

julia> using Mods

julia> r2 = range(Mod{24}(0), step=9, length=5)
Mod{24}(0):9:Mod{24}(12)

julia> collect(r2)
5-element Vector{Mod{24}}:
  Mod{24}(0)
  Mod{24}(9)
 Mod{24}(18)
  Mod{24}(3)
 Mod{24}(12)

julia> maximum(r2)
ERROR: MethodError: no method matching isless(::Mod{24}, ::Mod{24})

It already works because, unlike Dates.Time, Mods.Mod already has a copy constructor:

julia> Mod{24}(Mod{24}(0))
Mod{24}(0)

Now, that expectation of a copy constructor for T in StepRangeLen (ie that T(t::T) = T) is the original problem that I solved by adding copy constructor for Time. But, in retrospect, I now think a better solution could be to relieve StepRangeLen of this undocumented expectation, by changing to use the available parametric self-conversion (and then document the expectation of a conversion from R to T where they are not identical):

--- a/base/range.jl
+++ b/base/range.jl
@@ -974,7 +974,7 @@ unsafe_getindex(v::OneTo{T}, i::Integer) where T = convert(T, i)
 unsafe_getindex(v::AbstractRange{T}, i::Integer) where T = convert(T, first(v) + (i - oneunit(i))*step_hp(v))
 function unsafe_getindex(r::StepRangeLen{T}, i::Integer) where T
     u = oftype(r.offset, i) - r.offset
-    T(r.ref + u*r.step)
+    convert(T, (r.ref + u*r.step))
 end

Sorry for the long reply, but I think it's worth digging into the details to help decide what is the best way forward.


julia> collect(r)
5-element Vector{Time}:
00:00:00
09:00:00
18:00:00
03:00:00
12:00:00

```

## Adjuster Functions

As convenient as date-period arithmetic is, often the kinds of calculations needed on dates
Expand Down Expand Up @@ -707,7 +744,7 @@ Dates.UTC
Dates.DateTime(::Int64, ::Int64, ::Int64, ::Int64, ::Int64, ::Int64, ::Int64)
Dates.DateTime(::Dates.Period)
Dates.DateTime(::Function, ::Any...)
Dates.DateTime(::Dates.TimeType)
Dates.DateTime(::Dates.Date)
Dates.DateTime(::AbstractString, ::AbstractString)
Dates.format(::Dates.TimeType, ::AbstractString)
Dates.DateFormat
Expand All @@ -716,7 +753,7 @@ Dates.DateTime(::AbstractString, ::Dates.DateFormat)
Dates.Date(::Int64, ::Int64, ::Int64)
Dates.Date(::Dates.Period)
Dates.Date(::Function, ::Any, ::Any, ::Any)
Dates.Date(::Dates.TimeType)
Dates.Date(::Dates.DateTime)
Dates.Date(::AbstractString, ::AbstractString)
Dates.Date(::AbstractString, ::Dates.DateFormat)
Dates.Time(::Int64::Int64, ::Int64, ::Int64, ::Int64, ::Int64)
Expand Down
6 changes: 3 additions & 3 deletions stdlib/Dates/src/conversions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ Convert a `DateTime` to a `Date`. The hour, minute, second, and millisecond part
the `DateTime` are truncated, so only the year, month and day parts are used in
construction.
"""
Date(dt::TimeType) = convert(Date, dt)
Date(dt::DateTime) = convert(Date, dt)

"""
DateTime(dt::Date)
DateTime(d::Date)

Convert a `Date` to a `DateTime`. The hour, minute, second, and millisecond parts of
the new `DateTime` are assumed to be zero.
"""
DateTime(dt::TimeType) = convert(DateTime, dt)
DateTime(d::Date) = convert(DateTime, d)

"""
Time(dt::DateTime)
Expand Down
7 changes: 6 additions & 1 deletion stdlib/Dates/src/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ struct UTC <: TimeZone end
"""
TimeType

`TimeType` types wrap `Instant` machine instances to provide human representations of the
`TimeType` types wrap `Instant` machine instants to provide human representations of the
machine instant. `Time`, `DateTime` and `Date` are subtypes of `TimeType`.
"""
abstract type TimeType <: AbstractTime end
Expand Down Expand Up @@ -417,6 +417,11 @@ function DateTime(dt::Date, t::Time)
return DateTime(y, m, d, hour(t), minute(t), second(t), millisecond(t))
end

# explicit copy constructors
# (to avoid the Fallbacks which attempt to convert single param to Int64)
DateTime(dt::DateTime) = dt
Date(d::Date) = d
Time(t::Time) = t
# Fallback constructors
DateTime(y, m=1, d=1, h=0, mi=0, s=0, ms=0, ampm::AMPM=TWENTYFOURHOUR) = DateTime(Int64(y), Int64(m), Int64(d), Int64(h), Int64(mi), Int64(s), Int64(ms), ampm)
Date(y, m=1, d=1) = Date(Int64(y), Int64(m), Int64(d))
Expand Down
14 changes: 14 additions & 0 deletions stdlib/Dates/test/conversions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,18 @@ end
@test Dates.nanosecond(t) == 0
end

@testset "idempotency of conversion for TimeType subtypes" begin
for T in [DateTime, Date, Time]
@test T(T(0)) == T(0)
end
end

@testset "idempotency of conversion for Period subtypes" begin
for T in [Nanosecond, Millisecond,
Second, Minute, Hour,
Day, Week, Month, Quarter, Year]
@test T(T(0)) == T(0)
end
end

end
80 changes: 50 additions & 30 deletions stdlib/Dates/test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -534,41 +534,54 @@ let d = Dates.Day(1)
@test (Dates.Date(2000):d:Dates.Date(2001)) - d == (Dates.Date(2000) - d:d:Dates.Date(2001) - d)
end

# Time ranges
dr = Dates.Time(23, 1, 1):Dates.Second(1):Dates.Time(23, 2, 1)
dr1 = Dates.Time(23, 1, 1):Dates.Second(1):Dates.Time(23, 1, 1)
dr2 = Dates.Time(23, 1, 1):Dates.Second(1):Dates.Time(22, 2, 1) # empty range
dr3 = Dates.Time(23, 1, 1):Dates.Minute(-1):Dates.Time(22, 1, 1) # negative step
# Big ranges
dr8 = typemin(Dates.Time):Dates.Second(1):typemax(Dates.Time)
dr9 = typemin(Dates.Time):Dates.Nanosecond(1):typemax(Dates.Time)
# Other steps
dr10 = typemax(Dates.Time):Dates.Microsecond(-1):typemin(Dates.Time)
dr11 = typemin(Dates.Time):Dates.Millisecond(1):typemax(Dates.Time)
dr12 = typemin(Dates.Time):Dates.Minute(1):typemax(Dates.Time)
dr13 = typemin(Dates.Time):Dates.Hour(1):typemax(Dates.Time)
dr14 = typemin(Dates.Time):Dates.Millisecond(10):typemax(Dates.Time)
dr15 = typemin(Dates.Time):Dates.Minute(100):typemax(Dates.Time)
dr16 = typemin(Dates.Time):Dates.Hour(1000):typemax(Dates.Time)
dr17 = typemax(Dates.Time):Dates.Millisecond(-10000):typemin(Dates.Time)
dr18 = typemax(Dates.Time):Dates.Minute(-100):typemin(Dates.Time)
dr19 = typemax(Dates.Time):Dates.Hour(-10):typemin(Dates.Time)
dr20 = typemin(Dates.Time):Dates.Microsecond(2):typemax(Dates.Time)

drs = Any[dr, dr1, dr2, dr3, dr8, dr9, dr10,
dr11, dr12, dr13, dr14, dr15, dr16, dr17, dr18, dr19, dr20]
# small Time ranges
small_ranges_with = Any[
Dates.Time(23, 1, 1):Dates.Second(1):Dates.Time(23, 2, 1),
Dates.Time(23, 1, 1):Dates.Second(1):Dates.Time(23, 1, 1),
Dates.Time(23, 1, 1):Dates.Minute(-1):Dates.Time(22, 1, 1) # negative step
]
empty_range = Dates.Time(23, 1, 1):Dates.Second(1):Dates.Time(22, 2, 1)
small_ranges_without = Any[
empty_range,
range(Dates.Time(0), step=Dates.Hour(1), length=23) # by step, length
]
small_ranges = Any[small_ranges_with; small_ranges_without]

# Big ranges of various steps, increasing and decreasing
big_ranges = Any[
[typemin(Dates.Time):step:typemax(Dates.Time) for step in [
Dates.Second(1)
Dates.Nanosecond(1)
Dates.Microsecond(2)
Dates.Millisecond(1)
Dates.Minute(1)
Dates.Hour(1)
Dates.Millisecond(10)
Dates.Minute(100)
Dates.Hour(1000)
]]
[typemax(Dates.Time):step:typemin(Dates.Time) for step in [
Dates.Microsecond(-1)
Dates.Millisecond(-10000)
Dates.Minute(-100)
Dates.Hour(-10)
]]
]


drs = Any[small_ranges; big_ranges]

@test map(length, drs) == map(x->size(x)[1], drs)
@test all(x->findall(in(x), x) == [1:length(x);], drs[1:4])
@test isempty(dr2)
@test all(x->findall(in(x), x) == [1:length(x);], small_ranges)
@test isempty(empty_range)
@test all(x->reverse(x) == last(x): - step(x):first(x), drs)
@test all(x->minimum(x) == (step(x) < zero(step(x)) ? last(x) : first(x)), drs[4:end])
@test all(x->maximum(x) == (step(x) < zero(step(x)) ? first(x) : last(x)), drs[4:end])
@test_throws MethodError dr .+ 1
@test all(x->minimum(x) == (step(x) < zero(step(x)) ? last(x) : first(x)), big_ranges)
@test all(x->maximum(x) == (step(x) < zero(step(x)) ? first(x) : last(x)), big_ranges)
@test_throws MethodError drs[1] .+ 1

a = Dates.Time(23, 1, 1)
@test map(x->a in x, drs[1:4]) == [true, true, false, true]
@test a in dr
@test all(x->a in x, small_ranges_with)
@test all(x->!(a in x), small_ranges_without)

@test all(x->sort(x) == (step(x) < zero(step(x)) ? reverse(x) : x), drs)
@test all(x->step(x) < zero(step(x)) ? issorted(reverse(x)) : issorted(x), drs)
Expand Down Expand Up @@ -611,4 +624,11 @@ end
@test_throws OverflowError StepRange(dmin, Day(1), dmax)
end

@testset "StepRangeLen{Time} periodicity, indexing" begin
r = range(Time(0), step = Hour(9), length = 5)
@test length(r) == 5
@test r[begin:end] == [Time(0), Time(9), Time(18), Time(3), Time(12)]
end


end # RangesTest module