Fix colormap keyword argument handling (#358)

JuliaGraphics · Sep 27, 2019 · 43d93ff · 43d93ff
1 parent d64fb1a
commit 43d93ff
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 8 deletions.
diff --git a/src/colormaps.jl b/src/colormaps.jl
@@ -284,31 +284,35 @@ function colormap(cname::AbstractString, N::Int=100; mid=0.5, logscale=false, kv
 
     cname = lowercase(cname)
     if haskey(colormaps_sequential, cname)
+        allowedkeys = [:h, :w, :d, :c, :s, :b, :wcolor, :dcolor]
         p = copy(colormaps_sequential[cname][1:8])
 
-        allowedkeys = [:h, :w, :d, :c, :s, :b, :wcolor, :dcolor]
         for (k,v) in kvs
-            k in allowedkeys || throw(ArgumentError("Unknown keyword argument $k"))
-            ind = findfirst(allowedkeys, k)
+            ind = findfirst(e->e==k, allowedkeys)
+            ind == nothing && throw(ArgumentError("Unknown keyword argument $k"))
             if ind > 0
                 p[ind] = v
             end
         end
 
-        return sequential_palette(p[1], N, w=p[2], d=p[3], c=p[4], s=p[5], b=p[6], wcolor=p[7], dcolor=p[8], logscale=logscale)
+        return sequential_palette(p[1], N, w=p[2], d=p[3], c=p[4], s=p[5], b=p[6], wcolor=p[7],
+                                  dcolor=p[8], logscale=logscale)
 
     elseif haskey(colormaps_diverging, cname)
+        allowedkeys = [:h1, :h2, :w, :d1, :d2, :c, :s, :b, :wcolor, :dcolor1, :dcolor2]
         p = copy(colormaps_diverging[cname][1:11])
 
         for (k,v) in kvs
-            k in allowedkeys || throw(ArgumentError("Unknown keyword argument $k"))
-            ind = findfirst(allowedkeys, k)
+            ind = findfirst(e->e==k, allowedkeys)
+            ind == nothing && throw(ArgumentError("Unknown keyword argument $k"))
             if ind > 0
                 p[ind] = v
             end
         end
 
-        return diverging_palette(p[1], p[2], N, w=p[3], d1=p[4], d2=p[5], c=p[6], s=p[7], b=p[8], wcolor=p[9], dcolor1=p[10], dcolor2=p[11], mid=mid, logscale=logscale)
+        return diverging_palette(p[1], p[2], N, w=p[3], d1=p[4], d2=p[5], c=p[6], s=p[7],
+                                 b=p[8], wcolor=p[9], dcolor1=p[10], dcolor2=p[11], mid=mid,
+                                 logscale=logscale)
 
     else
         throw(ArgumentError(string("Unknown colormap: ", cname)))

diff --git a/test/colormaps.jl b/test/colormaps.jl
@@ -13,13 +13,46 @@
     cols = distinguishable_colors(1)
     @test colordiff(distinguishable_colors(1, cols; dropseed=true)[1], cols[1]) > 50
 
-    
+
     @test length(colormap("RdBu", 100)) == 100
 
     @test isconcretetype(eltype(colormap("Grays")))
 
     @test_throws ArgumentError colormap("Grays", N=10) # optional arguments, not keyword
 
+    # not return values to check here, just checking that keywords can be used
+    # Sequential
+    default_blues = colormap("Blues", 10)
+    @test colormap("Blues", 10; mid=0.9) == default_blues # `mid` is for diverging colormaps only (issue #300)
+    @test colormap("Blues", 10; logscale=true) != default_blues
+    @test colormap("Blues", 10; h=0.5) != default_blues
+    @test colormap("Blues", 10; w=0.5) != default_blues
+    @test colormap("Blues", 10; d=0.5) != default_blues
+    @test colormap("Blues", 10; c=0.5) != default_blues
+    @test colormap("Blues", 10; s=0.5) != default_blues
+    @test colormap("Blues", 10; b=0.5) != default_blues
+    @test colormap("Blues", 10; wcolor=colorant"white") != default_blues
+    @test colormap("Blues", 10; dcolor=colorant"black") != default_blues
+    @test_throws ArgumentError colormap("Blues", 10; h1=0.5)
+
+    # Diverging
+    default_rdbu = colormap("RdBu", 10)
+    @test colormap("RdBu", 10; mid=0.9) != default_rdbu
+    @test colormap("RdBu", 10; logscale=true) != default_rdbu
+    @test colormap("RdBu", 10; h1=0.5) != default_rdbu
+    @test colormap("RdBu", 10; h2=0.5) != default_rdbu
+    @test colormap("RdBu", 10; w=0.5) != default_rdbu
+    @test colormap("RdBu", 10; d1=0.5) != default_rdbu
+    @test colormap("RdBu", 10; d2=0.5) != default_rdbu
+    @test colormap("RdBu", 10; c=0.5) != default_rdbu
+    @test colormap("RdBu", 10; s=0.5) != default_rdbu
+    @test colormap("RdBu", 10; b=0.5) != default_rdbu
+    @test colormap("RdBu", 10; wcolor=colorant"white") != default_rdbu
+    @test colormap("RdBu", 10; dcolor1=colorant"black") != default_rdbu
+    # `dcolor2` is disabled by the default `d2`(=0)
+    @test colormap("RdBu", 10; dcolor2=colorant"black", d2=1) != default_rdbu
+    @test_throws ArgumentError colormap("RdBu", 10; h=0.5)
+
     # The outputs of `colormap()` were slightly affected by the bug fix of
     # `MSC(h)` (issue #349).
     # The following were generated by `colormap()` in Colors.jl v0.9.6.