Repository: JuliaPlots/Makie.jl (julia)

Issue:
Bad error when not passing named tuple
The error raised when forgetting the comma needed to make a named tuple is must unfelpful:
```
julia> plot(1:10, 1:10, figure=(title="Test"))
ERROR: BoundsError: attempt to access Char at index [2]
Stacktrace:
 [1] indexed_iterate(I::Char, i::Int64, state::Bool)
   @ Base .\tuple.jl:98
 [2] merge(a::NamedTuple{(), Tuple{}}, itr::String)
   @ Base .\namedtuple.jl:292
 [3] plot(::Type{Any}, ::UnitRange{Int64}, ::Vararg{UnitRange{Int64}}; axis::NamedTuple{(), Tuple{}}, figure::String, kw_attributes::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Makie C:\Users\Dennis Bal\.julia\packages\Makie\fEZv2\src\figureplotting.jl:16
 [4] #plot#13
   @ C:\Users\Dennis Bal\.julia\packages\MakieCore\oBlaS\src\recipes.jl:33 [inlined]
 [5] top-level scope
   @ REPL[15]:1
```

While this works
```
julia> plot(1:10, 1:10, figure=(title="Test",))
```

The same problem exists when using AlgebraOfGraphics's `draw` function, but perhaps it is a downstring issue from `Makie`, so I am raising it here first.

Can this be remedied by some sort of type-checking, or something else?

Pull request:
# Description

Fixes #1988

This should add a more helpful error when users pass incorrectly specified one-element 
`NamedTuple`s to the `axis` or `figure` keyword arguments. It's simply checked if the 
argument type is a `NamedTuple`, an `AbstractDict{Symbol}`, or an `Attributes`. This should 
cover everything that users are likely to pass. Of course the error message will only 
appear if the value of the incorrect one-element NamedTuple doesn't happen to be one of 
those types, but that can't be helped.

For example:

```julia
lines(1:10, axis = (aspect = DataAspect()))
```

<img width="692" alt="image" src="https://user-images.githubusercontent.com/22495855/177105614-7173f5b1-b551-4689-aafe-4cc246353091.png">


## Type of change

Delete options that do not apply:

- [x] Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] Added unit tests for new algorithms, conversion methods, etc.

Patch:
diff --git a/src/figureplotting.jl b/src/figureplotting.jl
index bf21ff16d34..5d3185a2871 100644
--- a/src/figureplotting.jl
+++ b/src/figureplotting.jl
@@ -11,8 +11,22 @@ Base.iterate(ap::AxisPlot, args...) = iterate((ap.axis, ap.plot), args...)
 
 get_scene(ap::AxisPlot) = get_scene(ap.axis.scene)
 
+function _validate_nt_like_keyword(@nospecialize(kw), name)
+    if !(kw isa NamedTuple || kw isa AbstractDict{Symbol} || kw isa Attributes)
+        throw(ArgumentError("""
+            The $name keyword argument received an unexpected value $(repr(kw)).
+            The $name keyword expects a collection of Symbol => value pairs, such as NamedTuple, Attributes, or AbstractDict{Symbol}.
+            The most common cause of this error is trying to create a one-element NamedTuple like (key = value) which instead creates a variable `key` with value `value`.
+            Write (key = value,) or (; key = value) instead."""
+        ))
+    end
+end
+
 function plot(P::PlotFunc, args...; axis = NamedTuple(), figure = NamedTuple(), kw_attributes...)
-    # scene_attributes = extract_scene_attributes!(attributes)
+    
+    _validate_nt_like_keyword(axis, "axis")
+    _validate_nt_like_keyword(figure, "figure")
+
     fig = Figure(; figure...)
 
     axis = Dict(pairs(axis))
@@ -48,6 +62,8 @@ end
 
 function plot(P::PlotFunc, gp::GridPosition, args...; axis = NamedTuple(), kwargs...)
 
+    _validate_nt_like_keyword(axis, "axis")
+    
     f = get_top_parent(gp)
 
     c = contents(gp, exact = true)
@@ -95,6 +111,8 @@ end
 
 function plot(P::PlotFunc, gsp::GridSubposition, args...; axis = NamedTuple(), kwargs...)
 
+    _validate_nt_like_keyword(axis, "axis")
+    
     layout = GridLayoutBase.get_layout_at!(gsp.parent, createmissing = true)
     c = contents(gsp, exact = true)
     if !isempty(c)


Test patch:
diff --git a/test/figures.jl b/test/figures.jl
index 5f0e43081b5..4d844d115af 100644
--- a/test/figures.jl
+++ b/test/figures.jl
@@ -153,3 +153,18 @@ end
 @testset "Not implemented error" begin
     @test_throws ErrorException("Not implemented for scatter. You might want to put:  `using Makie` into your code!") scatter()
 end
+
+@testset "Figure and axis kwargs validation" begin
+    @test_throws ArgumentError lines(1:10, axis = (aspect = DataAspect()), figure = (resolution = (100, 100)))
+    @test_throws ArgumentError lines(1:10, figure = (resolution = (100, 100)))
+    @test_throws ArgumentError lines(1:10, axis = (aspect = DataAspect()))
+    
+    # these just shouldn't error
+    lines(1:10, axis = (aspect = DataAspect(),))
+    lines(1:10, axis = Attributes(aspect = DataAspect()))
+    lines(1:10, axis = Dict(:aspect => DataAspect()))
+
+    f = Figure()
+    @test_throws ArgumentError lines(f[1, 1], 1:10, axis = (aspect = DataAspect()))
+    @test_throws ArgumentError lines(f[1, 1][2, 2], 1:10, axis = (aspect = DataAspect()))
+end
\ No newline at end of file