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

move Dates to stdlib #24459

Merged
merged 3 commits into from
Nov 22, 2017
Merged

move Dates to stdlib #24459

merged 3 commits into from
Nov 22, 2017

Conversation

KristofferC
Copy link
Sponsor Member

@KristofferC KristofferC commented Nov 3, 2017

Not completely done yet.

(Also removed 0.6 Dates deprecations, 0.7 deprecations moved to stdlib).

Fix #23302

@KristofferC KristofferC added domain:dates Dates, times, and the Dates stdlib module kind:excision Removal of code from Base or the repository labels Nov 3, 2017
@fredrikekre fredrikekre added the stdlib Julia's standard library label Nov 3, 2017
@KristofferC
Copy link
Sponsor Member Author

KristofferC commented Nov 4, 2017

LibGit2 uses Dates for printing a LibGit2.Signature. What's the best way to deal with that?

@@ -473,6 +469,7 @@ unshift!(Base._included_files, (@__MODULE__, joinpath(@__DIR__, "sysimg.jl")))
# load some stdlib packages but don't put their names in Main
Base.require(:DelimitedFiles)
Base.require(:Test)
Base.require(:Dates)
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

What decides if an stdlib package should be required here or not?

Copy link
Member

Choose a reason for hiding this comment

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

Anyone know the answer here? @JeffBezanson?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

These are to reduce startup/load time.

@KristofferC KristofferC force-pushed the kc/move_dates branch 5 times, most recently from 6ee33bd to 9d466fe Compare November 6, 2017 12:29
@KristofferC KristofferC force-pushed the kc/move_dates branch 3 times, most recently from a819a7d to 3830f66 Compare November 20, 2017 10:31
@KristofferC
Copy link
Sponsor Member Author

KristofferC commented Nov 20, 2017

I worked around the "LibGit2 using Dates"-problem in either, depending on your inclination, a brilliant or horrible way.

@KristofferC KristofferC changed the title WIP: move Dates to stdlib move Dates to stdlib Nov 20, 2017
function Base.show(io::IO, sig::Signature)
print(io, "Name: ", sig.name, ", ")
print(io, "Email: ", sig.email, ", ")
print(io, "Time: ", Dates.unix2datetime(sig.time + 60*sig.time_offset))
print(io, "Time: ", unix2datetime[](sig.time + 60*sig.time_offset))
Copy link
Member

Choose a reason for hiding this comment

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

how about

function yearmonthday(days)
    z = days + 306; h = 100z - 25; a = fld(h, 3652425); b = a - fld(a, 4)
    y = fld(100b + h, 36525); c = b + z - 365y - fld(y, 4); m = div(5c + 456, 153)
    d = c - div(153m - 457, 5); return m > 12 ? (y + 1, m - 12, d) : (y, m, d)
end
lpad0(x) = lpad(x, 2, '0')

function printunix(io, t)
    UNIXEPOCH = 62135683200000
    rata = UNIXEPOCH + round(Int64, Int64(1000) * t)
    year, month, day = yearmonthday(fld(rata, 86400000))
    secs = t % (24 * 60 * 60)
    mins = div(secs, 60)
    m, d = lpad0(month), lpad0(day)
    h, mi, s = lpad0(div(mins, 60)), lpad0(mins % 60), lpad0(secs % 60)
    print(io, "$year-$m-$d $h:$mi:$s")
end

@KristofferC
Copy link
Sponsor Member Author

Ok, I added the functions @quinnj with some modifications (truncating stuff to Integers).

If tests pass and this looks OK to you, this is ready to merge from my pov.

@quinnj
Copy link
Member

quinnj commented Nov 21, 2017

Travis OSX failure is spawn, I'm guessing appveyor linalg/bunchkaufman failures are unrelated?

@@ -198,6 +198,7 @@ for N in [0,10]
end

# Testing timedwait on multiple channels
using Dates
Copy link
Member

Choose a reason for hiding this comment

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

Will this cause problems if someone builds julia w/o the Dates module? I guess these tests would fail?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

Yes, but it is sort of the same if someone is building without Base64 though, right? I think for now, we don't have the requirement that arbitrary stdlib modules can be excluded.

@@ -3306,6 +3306,7 @@ end
@test Tuple{Int} === Tuple{Int, Vararg{Integer, 0}}

# issue #12003
using Dates
Copy link
Member

Choose a reason for hiding this comment

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

same here; I think it'd be generally preferable to not have Core/Base tests depend on stdlib modules, but I guess this is ok for now.

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

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

How about the Test stdlib module? :trollface:

@KristofferC KristofferC merged commit 0c0d6cc into master Nov 22, 2017
@StefanKarpinski StefanKarpinski deleted the kc/move_dates branch November 22, 2017 15:11
include("types.jl")
include("io.jl")
include("arithmetic.jl")
include("conversions.jl")
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we have a @testset "Dates" begin wrapping all of this?

Copy link
Member

Choose a reason for hiding this comment

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

Sure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dates Dates, times, and the Dates stdlib module kind:excision Removal of code from Base or the repository stdlib Julia's standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants