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

Initial work to add a Time type to Base.Dates #12274

Merged
merged 7 commits into from
Jan 24, 2017
+467 −94
Merged

Initial work to add a Time type to Base.Dates #12274

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b8b33f9
Add a Time type to the Base.Dates module
quinnj Jan 5, 2017
73eafe9
Time cleanup
quinnj Jan 6, 2017
58338da
Fix Time doc
quinnj Jan 19, 2017
8881760
Docs cleanup
quinnj Jan 19, 2017
673d2b8
Fix whitespace
quinnj Jan 19, 2017
e383f20
Fix Time docs
quinnj Jan 23, 2017
6cd4ea5
Add isfinite tests for TimeTypes
quinnj Jan 23, 2017
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
Prev Previous commit
Next Next commit
Time cleanup
  • Loading branch information
@quinnj
quinnj committed Jan 19, 2017
commit 73eafe901ddc6aa11419a0b7279d71a140458543
12 changes: 6 additions & 6 deletions base/dates/accessors.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ hour(dt::DateTime) = mod(fld(value(dt),3600000),24)
minute(dt::DateTime) = mod(fld(value(dt),60000),60)
second(dt::DateTime) = mod(fld(value(dt),1000),60)
millisecond(dt::DateTime) = mod(value(dt),1000)
hour(t::Time) = mod(fld(value(t),3600000000000),24)
minute(t::Time) = mod(fld(value(t),60000000000),60)
second(t::Time) = mod(fld(value(t),1000000000),60)
millisecond(t::Time) = mod(fld(value(t),1000000),1000)
microsecond(t::Time) = mod(fld(value(t),1000),1000)
nanosecond(t::Time) = mod(value(t),1000)
hour(t::Time) = mod(fld(value(t),3600000000000),Int64(24))
minute(t::Time) = mod(fld(value(t),60000000000),Int64(60))
second(t::Time) = mod(fld(value(t),1000000000),Int64(60))
millisecond(t::Time) = mod(fld(value(t),Int64(1000000)),Int64(1000))
microsecond(t::Time) = mod(fld(value(t),Int64(1000)),Int64(1000))
nanosecond(t::Time) = mod(value(t),Int64(1000))

dayofmonth(dt::TimeType) = day(dt)

Expand Down
8 changes: 4 additions & 4 deletions base/dates/arithmetic.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ end
(-)(x::Date, y::Week) = return Date(UTD(value(x) - 7*value(y)))
(+)(x::Date, y::Day) = return Date(UTD(value(x) + value(y)))
(-)(x::Date, y::Day) = return Date(UTD(value(x) - value(y)))
(+)(x::DateTime, y::Period) = return DateTime(UTM(value(x) + toms(y)))
(-)(x::DateTime, y::Period) = return DateTime(UTM(value(x) - toms(y)))
(+)(x::Time, y::TimePeriod) = return Time(Nanosecond(value(x) + tons(y)))
(-)(x::Time, y::TimePeriod) = return Time(Nanosecond(value(x) - tons(y)))
(+)(x::DateTime, y::Period) = return DateTime(UTM(value(x) + toms(y)))
(-)(x::DateTime, y::Period) = return DateTime(UTM(value(x) - toms(y)))
(+)(x::Time, y::TimePeriod) = return Time(Nanosecond(value(x) + tons(y)))
(-)(x::Time, y::TimePeriod) = return Time(Nanosecond(value(x) - tons(y)))
(+)(y::Period, x::TimeType) = x + y
(-)(y::Period, x::TimeType) = x - y
Copy link
Contributor

Choose a reason for hiding this comment

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

are these magnitudes then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I understand your comment. These are generic fallbacks that will end up dispatching to the definitions above. This is just for the switched order of operators.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks wrong that y - x = x - y

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, got it. Yes, these are magnitudes. Order doesn't matter.

Copy link
Contributor

Choose a reason for hiding this comment

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

worth a comment then, in case anyone else gets confused looking at this

Copy link
Member Author

Choose a reason for hiding this comment

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

Because we already do

julia> Dates.Day(1) - Date(2016, 1, 1)
2015-12-31

Copy link
Contributor

Choose a reason for hiding this comment

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

wat? why allow that?

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 don't remember if it was an explicit decision to allow or not. We can open another issue to remove.

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 in favor of removing this.

Copy link
Contributor

Choose a reason for hiding this comment

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

opened #20205


Expand Down
25 changes: 14 additions & 11 deletions base/dates/periods.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,26 @@ for period in (:Year, :Month, :Week, :Day, :Hour, :Minute, :Second, :Millisecond
# AbstractString parsing (mainly for IO code)
@eval $period(x::AbstractString) = $period(Base.parse(Int64,x))
# Period accessors
typs = period in (:Microsecond, :Nanosecond) ? ["Time"] : (period in (:Hour, :Minute, :Second, :Millisecond) ? ["DateTime","Time"] : ["TimeType"])
description = typs == ["TimeType"] ? "`Date` or `DateTime`" : (typs == ["Time"] ? "`Time`" : "`Time` or `DateTime`")
reference = period == :Week ? " For details see [`$accessor_str(::TimeType)`](:func:`$accessor_str`)." : ""
typs = period in (:Microsecond, :Nanosecond) ? ["Time"] :
period in (:Hour, :Minute, :Second, :Millisecond) ? ["Time", "DateTime"] : ["Date","DateTime"]
reference = period == :Week ? " For details see [`$accessor_str(::Union{Date,DateTime})`](:func:`$accessor_str`)." : ""
Copy link
Contributor

Choose a reason for hiding this comment

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

old-style cross-reference format

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the new style?

Copy link
Contributor

Choose a reason for hiding this comment

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

(@ref), the way the line used to be

for typ_str in typs
@eval begin
@doc """
$($period_str)(dt::$($typ_str)) -> $($period_str)

The $($accessor_str) part of a $($description) as a `$($period_str)`.$($reference)
The $($accessor_str) part of a $($typ_str) as a `$($period_str)`.$($reference)
""" $period(dt::$(Symbol(typ_str))) = $period($(Symbol(accessor_str))(dt))

@doc """
$($period_str)(v)

Construct a `$($period_str)` object with the given `v` value. Input must be
losslessly convertible to an `Int64`.
""" $period(v)
end
Copy link
Member

Choose a reason for hiding this comment

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

Docstrings seem a little strange here as the docstring specifically for Hour(::DateTime) references Time.

Hour(dt::DateTime) -> Hour

The hour part of a Time or DateTime as a Hour.

Hour(dt::Time) -> Hour

The hour part of a Time or DateTime as a Hour.

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 I've got them better sorted out now.

end
@eval begin
@doc """
$($period_str)(v)

Construct a `$($period_str)` object with the given `v` value. Input must be
losslessly convertible to an `Int64`.
""" $period(v)
end
end
# Now we're safe to define Period-Number conversions
# Anything an Int64 can convert to, a Period can convert to
Expand Down Expand Up @@ -449,6 +450,8 @@ Base.promote_rule(::Type{Year}, ::Type{Month}) = Month
Base.isless{T<:OtherPeriod,S<:OtherPeriod}(x::T,y::S) = isless(promote(x,y)...)

# truncating conversions to milliseconds and days:
toms(c::Nanosecond) = div(value(c), 1000000)
toms(c::Microsecond) = div(value(c), 1000)
toms(c::Millisecond) = value(c)
toms(c::Second) = 1000*value(c)
toms(c::Minute) = 60000*value(c)
Expand Down