JuliaPlots / Plots.jl

Powerful convenience for Julia visualizations and data analysis
https://docs.juliaplots.org
Other
1.84k stars 355 forks source link

[BUG] grouping Measurement data leads to wrong errorbars in plot #4917

Closed lukas-weber closed 1 month ago

lukas-weber commented 7 months ago

Details

using Measurements
using Plots

data = range(0, 1, 10) .± range(0, 1, 10)
ids = mod.(1:10, 2)

plot(
   plot(data, title="expected errorbars"),
   plot(data, group=ids, title="wrong errorbars")
)

Quite scary. It seems like in the group version, the ungrouped list of uncertainties is accessed using the intra-group index.

Backends

This bug occurs on ( insert x below )

Backend yes no untested
gr (default) x
pythonplot
plotlyjs
pgfplotsx
unicodeplots x
inspectdr
gaston

Versions

Plots.jl version: v1.40.3 Backend version (]st -m <backend(s)>): GR v0.73.3, UnicodePlots v3.6.4 Output of versioninfo():

Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 32 × Intel(R) Xeon(R) Gold 6244 CPU @ 3.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, cascadelake)
Threads: 1 default, 0 interactive, 1 GC (on 32 virtual cores)
lukas-weber commented 3 months ago

I did some digging. First I thought the problem was downstream with the Measurements.jl recipe, but I do not know what could be wrong with it.

Instead, looking at how the GroupBy recipe gets applied in Plots.jl, I think the issue is that in this case, it will attach the idxfilter attribute to the series rather than actually filtering the data. Later, there is a call to

# RecipesPipeline/src/group.jl
function filter_data!(plotattributes::AKW, idxfilter)
    for s in (:x, :y, :z)
        plotattributes[s] = filter_data(get(plotattributes, s, nothing), idxfilter)
end

Which will filter the data but not :xerror, :yerror, or :zerror, leading to the result from the minimal example above. I am pretty sure adding them to filter_data! would fix it and would have made a PR, however I noticed you are in the middle of restructuring for Plots v2. Should this go to the v2 branch or master?

isentropic commented 3 months ago

It should go to.v2 thanks