KristofferC / PGFPlotsX.jl

Plots in Julia using the PGFPlots LaTeX package
Other
301 stars 40 forks source link

PGFPlotsX 1.2.7 broke the Plots backend #230

Closed BeastyBlacksmith closed 4 years ago

BeastyBlacksmith commented 4 years ago

Ref: https://github.com/JuliaPlots/Plots.jl/issues/2792

The offending code is

            cstr = plot_color(bgc)
            a = alpha(cstr)
            push!(
                the_plot.options,
                "/tikz/background rectangle/.style" => PGFPlotsX.Options(
                    "fill" => cstr,
                    "draw opacity" => a,
                ),
                "show background rectangle" => nothing,
            )

Where the_plot is a PGFPlotsX.TikzDocument.

tpapp commented 4 years ago

I think this is coming from #228, with the two-argument default constructor taking over.

That said, I don't think that technically using the Options constructor is part of the API we expose so I am not sure this is a bug. I would recommend something like

@pgf { fill => cstr, draw_opacity => a }
BeastyBlacksmith commented 4 years ago

Well, when writing the backend @KristofferC advised to not use the @pgf macro.

Ref: https://github.com/JuliaPlots/Plots.jl/pull/2252#discussion_r346410991

KristofferC commented 4 years ago

Yes, and I stand by that. :). We should just fix and test the constructor.

fredrikekre commented 4 years ago
**diff --git a/src/options.jl b/src/options.jl
index 61f240e..b04af61 100644
--- a/src/options.jl
+++ b/src/options.jl
@@ -14,7 +14,7 @@ see the [`@pgf`](@ref) convenience macro.
 When `print_empty = false` (the default), empty options are not printed. Use
 `print_empty = true` to force printing a `[]` in this case.
 """
-function Options(args...; print_empty::Bool = false)
+function Options(args::Union{Pair,MergeEntry}...; print_empty::Bool = false)
     d = OrderedDict()
     for arg in args
         if arg isa Pair

should fix the dispatch I think.

tpapp commented 4 years ago

Fair enough; we should also document that

  1. Options and MergeEntry are part of the API, and mention that packages building on PGFPlotsX are encouraged to use it,
  2. also the "Options" section of the docs should mention this,
  3. update the docstring of at least Options to reflect #228, possibly with examples.
KristofferC commented 4 years ago

MergeEntry doesn't need to be a part of the official API I think.