clearer error messages and bugfix for nested figuresubpositions (#662)

* clearer error messages and bugfix for nested figuresubpositions

* fix test

* add tests

src/figureplotting.jl

@@ -41,7 +41,17 @@ end
 
 function plot(P::PlotFunc, fp::FigurePosition, args...; axis = NamedTuple(), kwargs...)
 
-    @assert isempty(contents(fp.gp, exact = true))
+    c = contents(fp.gp, exact = true)
+    if !isempty(c)
+        error("""
+        You have used the non-mutating plotting syntax with a FigurePosition, which requires an empty GridLayout slot to create an axis in, but there are already the following objects at this layout position:
+
+        $(c)
+
+        If you meant to plot into an axis at this position, use the plotting function with `!` (e.g. `func!` instead of `func`).
+        If you really want to place an axis on top of other layoutables, make your intention clear and create it manually.
+        """)
+    end
 
     proxyscene = Scene()
     plot!(proxyscene, P, Attributes(kwargs), args...)
@@ -69,6 +79,18 @@ end
 
 function plot(P::PlotFunc, fsp::FigureSubposition, args...; axis = NamedTuple(), kwargs...)
 
+    c = contents(fsp, exact = true)
+    if !isempty(c)
+        error("""
+        You have used the non-mutating plotting syntax with a FigureSubposition, which requires an empty GridLayout slot to create an axis in, but there are already the following objects at this layout position:
+
+        $(c)
+
+        If you meant to plot into an axis at this position, use the plotting function with `!` (e.g. `func!` instead of `func`).
+        If you really want to place an axis on top of other layoutables, make your intention clear and create it manually.
+        """)
+    end
+
     fig = get_figure(fsp)
 
     proxyscene = Scene()

src/figures.jl

@@ -119,6 +119,7 @@ end
 function Base.setindex!(parent::Union{FigurePosition,FigureSubposition}, obj,
     rows, cols, side = GridLayoutBase.Inner())
     layout = get_layout_at!(parent, createmissing = true)
+    isnothing(layout) && error("No single GridLayout could be found or created at this FigureSubposition. This means that there are at least two GridLayouts at this position already and it's unclear which one is meant.")
     layout[rows, cols, side] = obj
     obj
 end
@@ -130,6 +131,7 @@ end
 
 function Base.setindex!(parent::FigureSubposition, obj)
     layout = get_layout_at!(parent.parent, createmissing = true)
+    isnothing(layout) && error("No single GridLayout could be found or created at this FigureSubposition. This means that there are at least two GridLayouts at this position already and it's unclear which one is meant.")
     layout[parent.rows, parent.cols, parent.side] = obj
     obj
 end
@@ -142,23 +144,33 @@ end
 function get_layout_at!(fp::FigurePosition; createmissing = false)
     c = contents(fp.gp, exact = true)
     layouts = filter(x -> x isa GridLayoutBase.GridLayout, c)
-    if isempty(layouts)
-        if createmissing
-            return fp.gp[] = GridLayoutBase.GridLayout()
-        else
-            error("No layout found but `createmissing` is false.")
-        end
+    if isempty(layouts) && createmissing
+        fp.gp[] = GridLayoutBase.GridLayout()
     elseif length(layouts) == 1
-        return only(layouts)
+        only(layouts)::GridLayoutBase.GridLayout
     else
-        error("Found more than zero or one GridLayouts at $(fp.gp)")
+        nothing
     end
 end
 
 function get_layout_at!(fsp::FigureSubposition; createmissing = false)
     layout = get_layout_at!(fsp.parent; createmissing = createmissing)
+
+    if isnothing(layout)
+        return nothing
+    end
+
     gp = layout[fsp.rows, fsp.cols, fsp.side]
-    get_layout_at!(gp; createmissing = createmissing)
+
+    c = contents(gp, exact = true)
+    layouts = filter(x -> x isa GridLayoutBase.GridLayout, c)
+    if isempty(layouts) && createmissing
+        gp[] = GridLayoutBase.GridLayout()
+    elseif length(layouts) == 1
+        only(layouts)::GridLayoutBase.GridLayout
+    else
+        nothing
+    end
 end
 
 get_figure(fsp::FigureSubposition) = get_figure(fsp.parent)
@@ -170,6 +182,7 @@ end
 
 function GridLayoutBase.contents(f::FigureSubposition; exact = false)
     layout = get_layout_at!(f.parent, createmissing = false)
+    isnothing(layout) && return []
     GridLayoutBase.contents(layout[f.rows, f.cols, f.side], exact = exact)
 end
 

test/unit_tests/figures.jl

@@ -100,7 +100,7 @@ end
     @test contents(fig[1, 2][1, 1], exact = false) == [ax2]
     @test contents(fig[1, 2][1:2, 1:2], exact = true) == []
     @test contents(fig[1, 2][1:2, 1:2], exact = false) == [ax2]
-    @test_throws ErrorException contents(fig[1:2, 1:2][1, 1])
+    @test contents(fig[1:2, 1:2][1, 1]) == []
 
     label2 = fig[1, 2][1, 1] = Label(fig)
     @test contents(fig[1, 2][1, 1], exact = true) == [ax2, label2]
@@ -109,4 +109,23 @@ end
     @test contents(fig[1, 2][1:2, 1:2], exact = false) == [ax2, label2]
 
     @test_throws ErrorException content(fig[1, 2][1, 1])
+end
+
+@testset "Nested axis assignment" begin
+    fig = Figure()
+    @test Axis(fig[1, 1]) isa Axis
+    @test Axis(fig[1, 1][2, 3]) isa Axis
+    @test Axis(fig[1, 1][2, 3][4, 5]) isa Axis
+    @test_throws ErrorException scatter(fig[1, 1])
+    @test_throws ErrorException scatter(fig[1, 1][2, 3])
+    @test_throws ErrorException scatter(fig[1, 1][2, 3][4, 5])
+    @test scatter(fig[1, 2], 1:10) isa AbstractPlotting.AxisPlot
+    @test scatter(fig[1, 1][1, 1], 1:10) isa AbstractPlotting.AxisPlot
+    @test scatter(fig[1, 1][1, 1][1, 1], 1:10) isa AbstractPlotting.AxisPlot
+
+    fig = Figure()
+    fig[1, 1] = GridLayout()
+    @test Axis(fig[1, 1][1, 1]) isa Axis
+    fig[1, 1] = GridLayout()
+    @test_throws ErrorException Axis(fig[1, 1][1, 1])
 end