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

invalid kwargs specification should throw an error now #99

Merged
merged 7 commits into from
Nov 14, 2023
Merged

invalid kwargs specification should throw an error now #99

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
781ec58
invalid kwargs specification should throw an error now
behinger Nov 13, 2023
ee53bc2
bug fix and better error message
vladdez Nov 14, 2023
b9e592a
small renamings
vladdez Nov 14, 2023
7d491d5
error fix
vladdez Nov 14, 2023
c9a6fa9
changed color in topoplot in wiki
vladdez Nov 14, 2023
b6e3341
formatting
vladdez Nov 14, 2023
79638b5
fail in test
vladdez Nov 14, 2023
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
15 changes: 15 additions & 0 deletions src/plotconfig.jl
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would propose the following message:

Keyword argument specification (kwargs...) Specified config groups must be NamedTuples', but $(keys(kwargs)[.!is_namedtuple]) was not.
Maybe you forgot the semicolon (;) at the beginning of your specification? Compare these strings:

plot_example(...; layout = (; showColorbar=true))

plot_example(...; layout = (showColorbar=true))
 
 The first is correct and creates a NamedTuple as required. The second is wrong and is ignored.`

Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@ end
Takes a kwargs named tuple of Key => NamedTuple and merges the fields with the defaults
"""
function config_kwargs!(cfg::PlotConfig; kwargs...)
is_namedtuple = [isa(t,NamedTuple) for t in values(kwargs)]
@debug is_namedtuple
@assert(all(is_namedtuple),
""" Keyword-Argument specification (kwargs...)

All your specified config-groups need to be `NamedTuples`, but: `$(keys(kwargs)[.!is_namedtuple])` was not.
Maybe you forgot the semicolon (;) in your specification? It should be:

plot_example(...;layout = (;showColorbar=true))

and not:

plot_example(...;layout = (showColorbar=true))

The former creates a NamedTuple as required, the later doesn't do something helpful in this case.""")
list = fieldnames(PlotConfig)#[:layout,:visual,:mapping,:legend,:colorbar,:axis]
keyList = collect(keys(kwargs))
:extra ∈ keyList ? @warn("Extra is deprecated in 0.4 and extra-keyword args have to be used directly as key-word arguments") : ""
Expand Down
16 changes: 10 additions & 6 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,30 @@ using UnfoldMakie
include("setup.jl")
#include("../src/UnfoldMakie.jl")

@testset "UnfoldMakie.jl" begin

@testset "Test Config" begin
include("test_config.jl")
end
@testset "CircularEEGTopoPlot" begin
include("test_plot_circulareegtopoplot.jl")
end

@testset "UnfoldMakie.jl" begin
@testset "TopoSeries" begin
include("test_toposeries.jl")
end

@testset "UnfoldMakie.jl" begin
@testset "ERPImage" begin
include("test_erpimage.jl")
end

@testset "UnfoldMakie.jl" begin
@testset "TopoPlot" begin
include("test_topoplot.jl")
end

@testset "UnfoldMakie.jl" begin
@testset "Butterfly" begin
include("test_butterfly.jl")
end

@testset "UnfoldMakie.jl" begin
@testset "AllPlots" begin
include("test_all.jl")
end
9 changes: 9 additions & 0 deletions test/test_config.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
@testset "config kwargs" begin
cfg = PlotConfig()
UnfoldMakie.config_kwargs!(cfg;visual=(;bla=:blub))
@test cfg.visual.bla == :blub

# now test that you cannot forget the ; - that is, cant forget to specify a NamedTuple
@test_throws AssertionError UnfoldMakie.config_kwargs!(cfg;visual=(bla=:blub))

end
Loading