Fix handling of vector attributes in plotly code (#5175)

* fix segment handlin in plotly code

* remove show

* change handling of segments to fix plotly bar plots

* formatting + show

* more shows

* fix histogram case

* one example testset per backend

* fix pythonplot code

* runic formatting

Author: BeastyBlacksmith

src/backends/plotly.jl

@@ -335,7 +335,8 @@ function plotly_layout(plt::Plot)
         plotattributes_out[:hovermode] = "none"
     end
 
-    return plotattributes_out = recursive_merge(plotattributes_out, plt.attr[:extra_plot_kwargs])
+    return plotattributes_out =
+        recursive_merge(plotattributes_out, plt.attr[:extra_plot_kwargs])
 end
 
 function plotly_add_legend!(plotattributes_out::KW, sp::Subplot)
@@ -758,12 +759,8 @@ function plotly_colorbar(sp::Subplot)
 end
 
 function plotly_series_shapes(plt::Plot, series::Series, clims)
-    segments = series_segments(series; check = true)
-    plotattributes_outs = map(i -> KW(), 1:length(segments))
-
-    # TODO: create a plotattributes_out for each polygon
-    # x, y = series[:x], series[:y]
-
+    plotattributes_outs = KW[]
+    i = 1
     # these are the axes that the series should be mapped to
     x_idx, y_idx = plotly_link_indicies(plt, series[:subplot])
     plotattributes_base = KW(
@@ -773,23 +770,16 @@ function plotly_series_shapes(plt::Plot, series::Series, clims)
         :legendgroup => series[:label],
     )
 
-    x, y = (
-        plotly_data(series, letter, data) for
-            (letter, data) in zip((:x, :y), shape_data(series, 100))
-    )
-
-    for (k, segment) in enumerate(segments)
-        i, rng = segment.attr_index, segment.range
-        length(rng) < 2 && continue
+    for (xs, ys) in zip(nansplit(series[:x]), nansplit(series[:y]))
 
         # to draw polygons, we actually draw lines with fill
         plotattributes_out = merge(
             plotattributes_base,
             KW(
                 :type => "scatter",
                 :mode => "lines",
-                :x => vcat(x[rng], x[rng[1]]),
-                :y => vcat(y[rng], y[rng[1]]),
+                :x => xs,
+                :y => ys,
                 :fill => "tozeroy",
                 :fillcolor => rgba_string(
                     plot_color(get_fillcolor(series, clims, i), get_fillalpha(series, i)),
@@ -805,10 +795,11 @@ function plotly_series_shapes(plt::Plot, series::Series, clims)
                 :dash => string(get_linestyle(series, i)),
             )
         end
-        plotattributes_out[:showlegend] = k == 1 ? should_add_to_legend(series) : false
+        plotattributes_out[:showlegend] = i == 1 ? should_add_to_legend(series) : false
         plotly_polar!(plotattributes_out, series)
         plotly_adjust_hover_label!(plotattributes_out, _cycle(series[:hover], i))
-        plotattributes_outs[k] = merge(plotattributes_out, series[:extra_kwargs])
+        push!(plotattributes_outs, merge(plotattributes_out, series[:extra_kwargs]))
+        i += 1
     end
     if series[:fill_z] !== nothing
         push!(plotattributes_outs, plotly_colorbar_hack(series, plotattributes_base, :fill))

src/backends/pythonplot.jl

@@ -451,7 +451,7 @@ function _py_add_series(plt::Plot{PythonPlotBackend}, series::Series)
     end
 
     if st === :shape
-        for segment in series_segments(series)
+        for segment in series_segments(series, st)
             i, rng = segment.attr_index, segment.range
             if length(rng) > 1
                 lc = get_linecolor(series, clims, i, cbar_scale)
@@ -463,8 +463,6 @@ function _py_add_series(plt::Plot{PythonPlotBackend}, series::Series)
                 has_fs = !isnothing(fs)
 
                 path = mpl.path.Path(hcat(x[rng], y[rng]))
-                # FIXME: path can be un-filled e.g. ex 56,
-                # where rectangles are created using 4 paths instead of a single one
 
                 # shape outline (and potentially solid fill)
                 mpl.patches.PathPatch(

src/recipes.jl

@@ -482,15 +482,15 @@ end
         y := yseg.pts
         # expand attributes to match indices in new series data
         for k in _segmenting_vector_attributes ∪ _segmenting_array_attributes
-            if (v = get(plotattributes, k, nothing)) isa AVec
-                if eachindex(v) != eachindex(y)
-                    @warn "Indices $(eachindex(v)) of attribute `$k` do not match data indices $(eachindex(y))."
-                end
-                # Each segment is 6 elements long, including the NaN separator.
-                # One segment is created for each non-NaN element of `procy`.
-                # There is no trailing NaN, so the last repetition is dropped.
-                plotattributes[k] = @views repeat(v[valid_i]; inner = 6)[1:(end - 1)]
-            end
+            # if (v = get(plotattributes, k, nothing)) isa AVec
+            #     if eachindex(v) != eachindex(y)
+            #         @warn "Indices $(eachindex(v)) of attribute `$k` do not match data indices $(eachindex(y))."
+            #     end
+            #     # Each segment is 6 elements long, including the NaN separator.
+            #     # One segment is created for each non-NaN element of `procy`.
+            #     # There is no trailing NaN, so the last repetition is dropped.
+            #     plotattributes[k] = @views repeat(v[valid_i]; inner = 6)[1:(end - 1)]
+            # end
         end
         ()
     end

src/utils.jl

@@ -1,10 +1,11 @@
 # ---------------------------------------------------------------
-bool_env(x, default)::Bool =
-try
-    return parse(Bool, get(ENV, x, default))
-catch e
-    @warn e
-    return false
+function bool_env(x, default)::Bool
+    try
+        return parse(Bool, get(ENV, x, default))
+    catch e
+        @warn e
+        return false
+    end
 end
 
 treats_y_as_x(seriestype) =
@@ -36,10 +37,8 @@ to_nan(::Type{NTuple{2, Float64}}) = (NaN, NaN)
 to_nan(::Type{NTuple{3, Float64}}) = (NaN, NaN, NaN)
 
 coords(segs::Segments{Float64}) = segs.pts
-coords(segs::Segments{NTuple{2, Float64}}) =
-    (map(p -> p[1], segs.pts), map(p -> p[2], segs.pts))
-coords(segs::Segments{NTuple{3, Float64}}) =
-    (map(p -> p[1], segs.pts), map(p -> p[2], segs.pts), map(p -> p[3], segs.pts))
+coords(segs::Segments{NTuple{2, Float64}}) = (map(p -> p[1], segs.pts), map(p -> p[2], segs.pts))
+coords(segs::Segments{NTuple{3, Float64}}) = (map(p -> p[1], segs.pts), map(p -> p[2], segs.pts), map(p -> p[3], segs.pts))
 
 function Base.push!(segments::Segments{T}, vs...) where {T}
     isempty(segments.pts) || push!(segments.pts, to_nan(T))
@@ -112,21 +111,21 @@ function series_segments(series::Series, seriestype::Symbol = :path; check = fal
     end
 
     segments = if has_attribute_segments(series)
-        map(nan_segments) do r
+        (
             if seriestype === :shape
-                warn_on_inconsistent_shape_attr(series, x, y, z, r)
-                (SeriesSegment(r, first(r)),)
+                    # warn_on_inconsistent_shape_attr(series, x, y, z, r)
+                    (SeriesSegment(segment, j),)
             elseif seriestype in (:scatter, :scatter3d)
-                (SeriesSegment(i:i, i) for i in r)
+                    (SeriesSegment(i:i, i) for i in segment)
             else
-                (SeriesSegment(i:(i + 1), i) for i in first(r):(last(r) - 1))
-            end
-        end |> Iterators.flatten
+                    (SeriesSegment(i:(i + 1), i) for i in first(segment):(last(segment) - 1))
+            end for (j, segment) in enumerate(nan_segments)
+        ) |> Iterators.flatten
     else
         (SeriesSegment(r, 1) for r in nan_segments)
     end
 
-    warn_on_attr_dim_mismatch(series, x, y, z, segments)
+    # warn_on_attr_dim_mismatch(series, x, y, z, segments)
     return segments
 end
 
@@ -198,7 +197,8 @@ _cycle(wrapper::InputWrapper, idx::Int) = wrapper.obj
 _cycle(wrapper::InputWrapper, idx::AVec{Int}) = wrapper.obj
 
 _cycle(v::AVec, idx::Int) = v[mod(idx, axes(v, 1))]
-_cycle(v::AMat, idx::Int) = size(v, 1) == 1 ? v[end, mod(idx, axes(v, 2))] : v[:, mod(idx, axes(v, 2))]
+_cycle(v::AMat, idx::Int) =
+    size(v, 1) == 1 ? v[end, mod(idx, axes(v, 2))] : v[:, mod(idx, axes(v, 2))]
 _cycle(v, idx::Int) = v
 
 _cycle(v::AVec, indices::AVec{Int}) = map(i -> _cycle(v, i), indices)
@@ -376,20 +376,22 @@ function nanvcat(vs::AVec)
     return v_out
 end
 
-sort_3d_axes(x, y, z, letter) =
-if letter === :x
-    x, y, z
-elseif letter === :y
-    y, x, z
-else
-    z, y, x
+function sort_3d_axes(x, y, z, letter)
+    return if letter === :x
+        x, y, z
+    elseif letter === :y
+        y, x, z
+    else
+        z, y, x
+    end
 end
 
-axes_letters(sp, letter) =
-if RecipesPipeline.is3d(sp)
-    sort_3d_axes(:x, :y, :z, letter)
-else
-    letter === :x ? (:x, :y) : (:y, :x)
+function axes_letters(sp, letter)
+    return if RecipesPipeline.is3d(sp)
+        sort_3d_axes(:x, :y, :z, letter)
+    else
+        letter === :x ? (:x, :y) : (:y, :x)
+    end
 end
 
 handle_surface(z) = z
@@ -1009,19 +1011,20 @@ function ___straightline_data(xl, yl, x, y, exp_fact)
     )
 end
 
-__straightline_data(xl, yl, x, y, exp_fact) =
-if (n = length(x)) == 2
-    ___straightline_data(xl, yl, x, y, exp_fact)
-else
-    k, r = divrem(n, 3)
-    @assert r == 0 "Misformed data. `straightline_data` either accepts vectors of length 2 or 3k. The provided series has length $n"
-    xdata, ydata = fill(NaN, n), fill(NaN, n)
-    for i in 1:k
-        inds = (3i - 2):(3i - 1)
-        xdata[inds], ydata[inds] =
-            ___straightline_data(xl, yl, x[inds], y[inds], exp_fact)
+function __straightline_data(xl, yl, x, y, exp_fact)
+    return if (n = length(x)) == 2
+        ___straightline_data(xl, yl, x, y, exp_fact)
+    else
+        k, r = divrem(n, 3)
+        @assert r == 0 "Misformed data. `straightline_data` either accepts vectors of length 2 or 3k. The provided series has length $n"
+        xdata, ydata = fill(NaN, n), fill(NaN, n)
+        for i in 1:k
+            inds = (3i - 2):(3i - 1)
+            xdata[inds], ydata[inds] =
+                ___straightline_data(xl, yl, x[inds], y[inds], exp_fact)
+        end
+        xdata, ydata
     end
-    xdata, ydata
 end
 
 _straightline_data(::Val{true}, ::Function, ::Function, ::Function, ::Function, args...) =
@@ -1282,12 +1285,13 @@ function protectedstring(s)
         To suppress all axis labels, pass an empty string to `xlabel`, etc.
         To suppress units in axis labels pass `unitformat = :nounit` or `unitformat=(l,u)->l`
         (equivalently for `xunitformat`, `yunitformat`, etc.).
-            """, :protectedstring, force = true
+            """,
+        :protectedstring,
+        force = true,
     )
     return ProtectedString(s)
 end
 
-
 """
     P_str(s)
 

test/test_backends.jl

@@ -213,7 +213,7 @@ is_pkgeval() || @testset "PlotlyJS" begin
     end
 end
 
-is_pkgeval() || @testset "Examples" begin
+is_pkgeval() || @testset "Examples $be" for be in TEST_BACKENDS
     callback(m, pkgname, i) = begin
         pl = m.Plots.current()
         save_func = (; pgfplotsx = m.Plots.pdf, unicodeplots = m.Plots.txt)  # fastest `savefig` for each backend
@@ -224,9 +224,8 @@ is_pkgeval() || @testset "Examples" begin
         )
         @test filesize(fn) > 1_000
     end
-    Sys.islinux() && for be in TEST_BACKENDS
-        skip = vcat(Plots._backend_skips[be], blacklist)
-        Plots.test_examples(be; skip, callback, disp = is_ci(), strict = true)  # `ci` display for coverage
-        closeall()
-    end
+    Sys.islinux() || continue
+    skip = vcat(Plots._backend_skips[be], blacklist)
+    Plots.test_examples(be; skip, callback, disp = is_ci(), strict = true)  # `ci` display for coverage
+    closeall()
 end