Merge pull request #17 from invenia/mz/deprecate

deprecate old checkpointing syntax

Project.toml

@@ -1,7 +1,7 @@
 name = "Checkpoints"
 uuid = "b4a3413d-e481-5afc-88ff-bdfbd6a50dce"
 authors = "Invenia Technical Computing Corporation"
-version = "0.3.5"
+version = "0.3.6"
 
 [deps]
 AWSS3 = "1c724243-ef5b-51ab-93f4-b0a88ac62a95"

src/Checkpoints.jl

@@ -28,6 +28,7 @@ const CHECKPOINTS = Dict{String, Union{Nothing, Handler}}()
 @contextvar CONTEXT_TAGS::Tuple{Vararg{Pair{Symbol, Any}}} = Tuple{}()
 
 include("session.jl")
+include("deprecated.jl")
 
 """
     with_checkpoint_tags(f::Function, context_tags::Pair...)
@@ -62,29 +63,45 @@ available() = collect(keys(CHECKPOINTS))
 
 """
     checkpoint([prefix], name, data)
-    checkpoint([prefix], name, data::Pair...; tags...)
-    checkpoint([prefix], name, data::Dict; tags...)
+    checkpoint([prefix], name, data::Pair...)
+    checkpoint([prefix], name, data::Dict)
 
 Defines a data checkpoint with a specified `label` and values `data`.
 By default checkpoints are no-ops and need to be explicitly configured.
 
-    checkpoint(session, data; tags...)
-    checkpoint(handler, name, data::Dict; tags...)
+    checkpoint(session, data)
+    checkpoint(handler, name, data::Dict)
 
 Alternatively, you can also checkpoint with to a session which stages the data to be
 commited later by `commit!(session)`.
 Explicitly calling checkpoint on a handler is generally not advised, but is an option.
 """
 function checkpoint(name::String, data::Dict{Symbol}; tags...)
-    checkpoint(CHECKPOINTS[name], name, data; tags...)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        checkpoint(CHECKPOINTS[name], name, data)
+    end
 end
 
-checkpoint(name::String, data::Pair...; tags...) = checkpoint(name, Dict(data...); tags...)
+function checkpoint(name::String, data::Pair...; tags...)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        checkpoint(name, Dict(data...))
+    end
+end
 
-checkpoint(name::String, data; tags...) = checkpoint(name, Dict(:data => data); tags...)
+function checkpoint(name::String, data; tags...)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        checkpoint(name, Dict(:data => data))
+    end
+end
 
-function checkpoint(prefix::Union{Module, String}, name::String, args...; kwargs...)
-    checkpoint("$prefix.$name", args...; kwargs...)
+function checkpoint(prefix::Union{Module, String}, name::String, args...; tags...)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        checkpoint("$prefix.$name", args...)
+    end
 end
 
 """

src/deprecated.jl

@@ -0,0 +1,13 @@
+function checkpoint_deprecation(tags...)
+    kwargs = join(["$(first(tag))=\"$(last(tag))\"" for tag in tags], ", ")
+    pairs = join([":$(first(tag)) => \"$(last(tag))\"" for tag in tags], ", ")
+
+    isempty(tags) || Base.depwarn(
+        "checkpoint(args...; $(kwargs)) is deprecated, use\n" *
+        "with_checkpoint_tags($(pairs)) do\n" *
+        "    checkpoint(args...)\n" *
+        "end\n" *
+        "instead. Note the use of `Pair`s instead of keyword arguments.",
+        :checkpoint
+    )
+end

src/handler.jl

@@ -15,24 +15,21 @@ Handler(path::String; kwargs...) = Handler(Path(path), kwargs)
 Handler(bucket::String, prefix::String; kwargs...) = Handler(S3Path("s3://$bucket/$prefix"), kwargs)
 
 """
-    path(handler, name; tags...)
+    path(handler, name)
 
-Determines the path to save to based on the handlers path prefix, name and tags.
+Determines the path to save to based on the handlers path prefix, name, and context.
 Tags are used to dynamically prefix the named file with the handler's path.
 Names with a '.' separators will be used to form subdirectories
 (e.g., "Foo.bar.x" will be saved to "\$prefix/Foo/bar/x.jlso").
 """
-function path(handler::Handler{P}, name::String; tags...) where P
-    with_checkpoint_tags(tags...) do
-        # Build up a path prefix based on the tags passed in.
-        prefix = ["$key=$val" for (key,val) in CONTEXT_TAGS[]]
+function path(handler::Handler{P}, name::String) where P
+    prefix = ["$key=$val" for (key,val) in CONTEXT_TAGS[]]
 
-        # Split up the name by '.' and add the jlso extension
-        parts = split(name, '.')
-        parts[end] = string(parts[end], ".jlso")
+    # Split up the name by '.' and add the jlso extension
+    parts = split(name, '.')
+    parts[end] = string(parts[end], ".jlso")
 
-        return join(handler.path, prefix..., parts...)
-    end
+    return join(handler.path, prefix..., parts...)
 end
 
 """
@@ -65,17 +62,23 @@ function commit!(handler::Handler{P}, path::P, jlso::JLSO.JLSOFile) where P <: A
 end
 
 function checkpoint(handler::Handler, name::String, data::Dict{Symbol}; tags...)
-    debug(LOGGER, "Checkpoint $name triggered, with tags: $(join(tags, ", ")).")
-    jlso = JLSO.JLSOFile(Dict{Symbol, Vector{UInt8}}(); handler.settings...)
-    p = path(handler, name; tags...)
-    stage!(handler, jlso, data)
-    commit!(handler, p, jlso)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        debug(LOGGER, "Checkpoint $name triggered, with context: $(join(CONTEXT_TAGS[], ", ")).")
+        jlso = JLSO.JLSOFile(Dict{Symbol, Vector{UInt8}}(); handler.settings...)
+        p = path(handler, name)
+        stage!(handler, jlso, data)
+        commit!(handler, p, jlso)
+    end
 end
 
 #=
 Define our no-op conditions just to be safe
 =#
 function checkpoint(handler::Nothing, name::String, data::Dict{Symbol}; tags...)
-    debug(LOGGER, "Checkpoint $name triggered, but no handler has been set.")
-    nothing
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        debug(LOGGER, "Checkpoint $name triggered, but no handler has been set.")
+        nothing
+    end
 end

src/session.jl

@@ -49,14 +49,27 @@ function commit!(session::Session)
 end
 
 function checkpoint(session::Session, data::Dict{Symbol}; tags...)
-    # No-ops skip when handler is nothing
-    session.handler === nothing && return nothing
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        # No-ops skip when handler is nothing
+        session.handler === nothing && return nothing
 
-    p = path(session.handler, session.name; tags...)
-    jlso = session.objects[p]
-    session.objects[p] = stage!(session.handler, jlso, data)
+        p = path(session.handler, session.name)
+        jlso = session.objects[p]
+        session.objects[p] = stage!(session.handler, jlso, data)
+    end
 end
 
-checkpoint(s::Session, data::Pair...; tags...) = checkpoint(s, Dict(data...); tags...)
+function checkpoint(s::Session, data::Pair...; tags...)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        checkpoint(s, Dict(data...))
+    end
+end
 
-checkpoint(s::Session, data; tags...) = checkpoint(s, Dict(:data => data); tags...)
+function checkpoint(s::Session, data; tags...)
+    checkpoint_deprecation(tags...)
+    with_checkpoint_tags(tags...) do
+        checkpoint(s, Dict(:data => data))
+    end
+end

test/runtests.jl

@@ -40,6 +40,14 @@ Distributed.addprocs(5)
         end
     end
 
+    @testset "deprecated tags syntax" begin
+        mktempdir() do path
+            Checkpoints.config("TestPkg.deprecated", path)
+            TestPkg.deprecated_checkpoint_syntax()
+            @test isfile(joinpath(path, "date=2017-01-01", "TestPkg", "deprecated.jlso"))
+        end
+    end
+
     @testset "Context tags" begin
         mktempdir() do path
             @everywhere begin

test/testpkg.jl

@@ -1,11 +1,11 @@
 module TestPkg
 
-using Checkpoints: register, checkpoint, Session
+using Checkpoints: register, checkpoint, with_checkpoint_tags, Session
 
 # We aren't using `@__MODULE__` because that would return TestPkg on 0.6 and Main.TestPkg on 0.7
 const MODULE = "TestPkg"
 
-__init__() = register(MODULE, ["foo", "bar", "baz", "qux_a", "qux_b"])
+__init__() = register(MODULE, ["foo", "bar", "baz", "qux_a", "qux_b", "deprecated"])
 
 function foo(x::Matrix, y::Matrix)
     # Save multiple variables to 1 foo.jlso file by passing in pairs of variables
@@ -15,8 +15,10 @@ end
 
 function bar(a::Vector)
     # Save a single value for bar.jlso. The object name in that file defaults to :data.
-    checkpoint(MODULE, "bar", a; date="2017-01-01")
-    return a * a'
+    with_checkpoint_tags(:date=>"2017-01-01") do
+        checkpoint(MODULE, "bar", a)
+        return a * a'
+    end
 end
 
 function baz(data::Dict)
@@ -39,4 +41,8 @@ function qux(a::Dict, b::Vector)
     end
 end
 
+function deprecated_checkpoint_syntax()
+    checkpoint(MODULE, "deprecated", [1, 2, 3]; date="2017-01-01")
+end
+
 end