Fix for "segmented" attributes with NaNs

[yha]

Fixes indexing into attributes that vary by series or data point, so that it is not affected by NaN values.
Some examples that previously produced either a BoundsError or silently wrong output:

plot(plot([2 3; 3 4; 1 NaN; 2 1; 4 2], c=:plasma, fill_z=[1 3], fillrange=0, fillalpha=0.5),
   plot([2 3; 3 4; 1 NaN; 2 1; 4 2], c=:plasma, fill_z=1:5, fillrange=0, fillalpha=0.5),
   scatter([1 2; NaN 3; 4 5], marker_z = [1 2]),
   scatter([1 2; NaN 3; 4 5], marker_z =[1 1; 2 2; 3 3]),
   plot([1 2; 2 3; 4 5], line_z=[1 1; 3 4; 5 6]),
   plot([[1,2,NaN,4,5] 2:6], line_z=[1 2]), 
   legend=false)

Fixes #3258 (maybe also #2959?).
Breaks the current reference image 48, but I don't think the current behavior is documented other than in this reference image. As I mentioned in #3258 (comment), I think this fix is important enough to break it.

[daschw]

In general, I'd also find this more consistent. However, I'm not sure if this "just" is a bugfix or breaking.
Consider e.g.

x = [1, 3, 2, NaN, 4, 6, 5, NaN, 0, 1, 1, 0]
y = [2, 2, 3, NaN, 0, 1, 0, NaN, 0, 0, 1, 1]

plot(x, y, st=:shape, fillcolor=[:red, :blue, :green])
plot(x, y, st=:shape, fill_z=1:3)

This PR changes

to

and

to

same results for

xs = [[1, 3, 2], [4, 6, 5], [0, 1, 1, 0]]
ys = [[2, 2, 3], [0, 1, 0], [0, 0, 1, 1]]
shapes = Shape.(xs, ys)

plot(shapes, fillcolor=[:red, :blue, :green])
plot(shapes, fill_z=1:3)

I wonder if we can keep the old behavior somehow with the changes in this PR (and still be consistent). I can be quite handy. I used it sometimes for plotting shapefile maps and coloring countries according to some values.

[mkborregaard]

I agree @daschw , this is a (not necessarily desirable) behaviour change rather than a fix.

[yha]

@daschw, I would say that the "before" plots in your first example are definitely a bug. I think that it's extremely surprising that the way indexes in attributes correspond to indexes in data depends in any way on the contents rather than types and shapes. Real data is eventually going to contain NaNs which are "real", and not there intentionally to separate segments.
(incidentally, this is more insidious than the cycling behavior of attributes because a) it can crop up after shapes have been debugged, and b) output are less likely to seem obviously wrong: cycled attributes create repetitive patterns in the plots that usually stand out)

The second example is more compelling, and not something I've tested. It's not consistent with the behavior of a plain plot(xs, ys,...) (with or without st=:shape), but also doesn't seem worth changing, so I'll fix the PR to preserve the behavior for this case.

To deal with the breakage, perhaps we can add a "soft deprecation", i.e. a warning when sizes of "segmenting" attributes don't match, and an extra warning when NaNs are present?

[yha]

I fixed the AVec{Shape} recipe to give the same output as before.
I also added a warning on attribute length mismatch + a hint when the data contains NaNs (which is where the output changes with this PR).

x = [1, 3, 2, NaN, 4, 6, 5, NaN, 0, 1, 1, 0]
y = [2, 2, 3, NaN, 0, 1, 0, NaN, 0, 0, 1, 1]

plot(x, y, st=:shape, fillcolor=[:red, :blue, :green])

gives

┌ Warning: Indices Base.OneTo(3) of attribute `fillcolor` does not match data indices 1:12.
└ @ Plots c:\Users\sternlab\.julia\dev\Plots\src\utils.jl:106
┌ Info: Data contains NaNs or missing values, and indices of `fillcolor` vector do not match data indices.
│ If you intend elements of `fillcolor` to apply to individual NaN-separated segements in the data,
│ pass each segment in a separate vector instead, and use a row vector for `fillcolor`. Legend entries 
│ may be suppressed by passing an empty label.
│ For example,
└     plot([1:2,1:3], [[4,5],[3,4,5]], label=["y" ""], fillcolor=[1 2])

An alternative would be to merge these message and show them only when the data has NaNs.
My rationale for choosing to display the warning on any dimension mismatch (even when data does not have NaNs) was that the current behavior in this case is very messy anyway: e.g. on GR plot(1:3,1:5) and scatter(1:5, marker_z=1:2) error, scatter(1:3, 1:5) truncates y, and scatter(1:5, c=["blue","red"]) cycles.

[mkborregaard]

Hmm, I'm being swayed by the good arguments here, sorry for making noise.

[daschw]

Nice, I'm for these changes. Note that this also fixes #3000 and #3117

[codecov]

Codecov Report

Merging #3320 (5cac0e9) into master (2220dbc) will decrease coverage by 2.17%.
The diff coverage is 78.72%.

@@            Coverage Diff             @@
##           master    #3320      +/-   ##
==========================================
- Coverage   65.17%   63.00%   -2.18%     
==========================================
  Files          25       25              
  Lines        6456     6098     -358     
==========================================
- Hits         4208     3842     -366     
- Misses       2248     2256       +8     

Impacted Files	Coverage Δ
src/backends.jl	67.59% <ø> (-1.90%)	⬇️
src/components.jl	60.83% <0.00%> (-0.71%)	⬇️
src/examples.jl	55.22% <ø> (-6.55%)	⬇️
src/recipes.jl	56.36% <0.00%> (-1.33%)	⬇️
src/utils.jl	52.57% <90.90%> (-3.71%)	⬇️
src/backends/gr.jl	89.29% <100.00%> (-0.70%)	⬇️
src/backends/pgfplotsx.jl	58.68% <100.00%> (-2.21%)	⬇️
src/layouts.jl	60.53% <0.00%> (-3.56%)	⬇️
src/subplots.jl	66.66% <0.00%> (-3.34%)	⬇️
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2220dbc...c8b882d. Read the comment docs.