search

MakieOrg / Makie.jl

Interactive data visualizations and plotting in Julia
https://docs.makie.org/stable
MIT License
2.4k stars 310 forks source link

Incorrect keyword arguments for `axis` argument don't show any suggestions #4391

Open DanielVandH opened 1 month ago

DanielVandH commented 1 month ago 
julia> using CairoMakie

(jl_Y5LoAd) pkg> st -m Makie
Status `C:\Users\danjv\AppData\Local\Temp\jl_Y5LoAd\Manifest.toml`
  [ee78f7c6] Makie v0.21.11

julia> scatter([1.0], [1.0], axis = (; blahblah = "1.0")) # should show suggestions
ERROR: MethodError: no method matching initialize_block!(::Axis; blahblah::String)

Closest candidates are:
  initialize_block!(::Axis; palette) got unsupported keyword argument "blahblah"
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\makielayout\blocks\axis.jl:150
  initialize_block!(::Textbox) got unsupported keyword argument "blahblah"
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\makielayout\blocks\textbox.jl:4
  initialize_block!(::Colorbar) got unsupported keyword argument "blahblah"
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\makielayout\blocks\colorbar.jl:115
  ...

Stacktrace:
 [1] kwerr(::@NamedTuple{blahblah::String}, ::Function, ::Axis)
   @ Base .\error.jl:165
 [2] _block(T::Type{…}, fig_or_scene::Figure, args::Vector{…}, kwdict::Dict{…}, bbox::Nothing; kwdict_complete::Bool)
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\makielayout\blocks.jl:371
 [3] _block(T::Type{Axis}, fig_or_scene::Figure, args::Vector{Any}, kwdict::Dict{Symbol, Any}, bbox::Nothing)
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\makielayout\blocks.jl:291
 [4] create_axis_for_plot(figure::Figure, plot::Scatter{Tuple{Vector{Point{2, Float64}}}}, attributes::Dict{Symbol, Any})
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\figureplotting.jl:107
 [5] create_axis_like(plot::Scatter{Tuple{Vector{Point{2, Float64}}}}, attributes::Dict{Symbol, Any}, ::Nothing)
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\figureplotting.jl:199
 [6] _create_plot(::Function, ::Dict{Symbol, Any}, ::Vector{Float64}, ::Vararg{Vector{Float64}})
   @ Makie C:\Users\danjv\.julia\packages\Makie\gG38B\src\figureplotting.jl:317
 [7] scatter(::Vector{Float64}, ::Vararg{Vector{Float64}}; kw::@Kwargs{axis::@NamedTuple{blahblah::String}})
   @ MakieCore C:\Users\danjv\.julia\packages\MakieCore\rTINf\src\recipes.jl:449
 [8] top-level scope
   @ REPL[20]:1
Some type information was truncated. Use `show(err)` to see complete types.

Is it possible to allow suggestions for allowed keyword arguments to be shown here?

asinghvi17 commented 1 month ago

This is generic to all Blocks I think, maybe we could inject a keyword check somewhere? But that seems incompatible with methods like initialize_block!(::Axis; palette) got unsupported keyword argument "blahblah", since you'd have to know that method takes the palette kwarg.

DanielVandH commented 1 month ago

Can it be tried in any of the earlier calls like create_axis_for_plot (just going off the stacktrace anyway)?

jkrumbiegel commented 1 month ago

I actually don't want the keywords in the initialize_block! thing anymore... I think I only did it because I didn't know how else to pipe that palette through, but at the time it wasn't clear anyway how one should do that

DanielVandH commented 1 month ago

One quick solution specific to Axis (that I'm sure there are problems with since I'm in unfamiliar territory here 😅) is to define

MakieCore._valid_attributes(::Type{<:Axis}) = keys(_attribute_docs(Axis))
MakieCore._is_block(::Type{<:Axis}) = true
function validate_axis_attributes(attributes::Dict)
    nameset = _attribute_docs(Axis)
    unknown = setdiff(keys(attributes), nameset)
    if !isempty(unknown)
        throw(MakieCore.InvalidAttributeError(Axis, unknown))
    end
    return 
end

and then put validate_axis_attributes at the top of create_axis_for_plot:

function create_axis_for_plot(figure::Figure, plot::AbstractPlot, attributes::Dict)
    axis_kw = extract_attributes(attributes, :axis)
    validate_axis_attributes(axis_kw)
    AxType = if haskey(axis_kw, :type)
        pop!(axis_kw, :type)
    else
        preferred_axis_type(plot)
    end
    if AxType == FigureOnly # For FigureSpec, which creates Axes dynamically
        return nothing
    end
    bbox = pop!(axis_kw, :bbox, nothing)
    set_axis_attributes!(AxType, axis_kw, plot)
    return _block(AxType, figure, [], axis_kw, bbox)
end

Reusing InvalidAttributeError like this meant I needed to adjust InvalidAttributeError a bit (further adjustments would be needed to not call Axis a plot type too)

_valid_attributes(p) = attribute_names(p)
_is_block(_) = false

function Base.showerror(io::IO, i::InvalidAttributeError)
    n = length(i.attributes)
    print(io, "Invalid attribute$(n > 1 ? "s" : "") ")
    for (j, att) in enumerate(i.attributes)
        j > 1 && print(io, j == length(i.attributes) ? " and " : ", ")
        printstyled(io, att; color = :red, bold = true)
    end
    print(io, " for plot type ")
    printstyled(io, i.plottype; color = :blue, bold = true)
    println(io, ".")
    nameset = sort(string.(collect(_valid_attributes(i.plottype))))
    println(io)
    println(io, "The available plot attributes for $(i.plottype) are:")
    println(io)
    print_columns(io, nameset; cols = displaysize(stderr)[2], rows_first = true)
    if !_is_block(i.plottype)
        allowlist = attribute_name_allowlist()
        println(io)
        println(io)
        println(io, "Generic attributes are:")
        println(io)
        print_columns(io, sort([string(a) for a in allowlist]); cols = displaysize(stderr)[2], rows_first = true)
        println(io)
    else 
        println(io)
        println(io)
    end
end

This shows

julia> scatter([1.0], [1.0], axis = (; blahblah = "1.0")) # should show suggestions
ERROR: Invalid attribute blahblah for plot type Axis.

The available plot attributes for Axis are:

alignmode           height             subtitlefont        titlefont         xaxisposition   xlabelsize         xminorticksvisible  xticklabelalign     xticksmirrored    ygridvisible     yminorgridstyle     ypanlock         yticklabelpad       ytrimspine  
aspect              leftspinecolor     subtitlegap         titlegap          xgridcolor      xlabelvisible      xminortickwidth     xticklabelcolor     xticksvisible     ygridwidth       yminorgridvisible   yrectzoom        yticklabelrotation  yzoomkey    
autolimitaspect     leftspinevisible   subtitlelineheight  titlelineheight   xgridstyle      xminorgridcolor    xpankey             xticklabelfont      xtickwidth        ylabel           yminorgridwidth     yreversed        yticklabelsize      yzoomlock   
backgroundcolor     limits             subtitlesize        titlesize         xgridvisible    xminorgridstyle    xpanlock            xticklabelpad       xtrimspine        ylabelcolor      yminortickalign     yscale           yticklabelspace     zoombutton  
bottomspinecolor    panbutton          subtitlevisible     titlevisible      xgridwidth      xminorgridvisible  xrectzoom           xticklabelrotation  xzoomkey          ylabelfont       yminortickcolor     ytickalign       yticklabelsvisible
bottomspinevisible  rightspinecolor    tellheight          topspinecolor     xlabel          xminorgridwidth    xreversed           xticklabelsize      xzoomlock         ylabelpadding    yminorticks         ytickcolor       yticks
dim1_conversion     rightspinevisible  tellwidth           topspinevisible   xlabelcolor     xminortickalign    xscale              xticklabelspace     yautolimitmargin  ylabelrotation   yminorticksize      ytickformat      yticksize
dim2_conversion     spinewidth         title               valign            xlabelfont      xminortickcolor    xtickalign          xticklabelsvisible  yaxisposition     ylabelsize       yminorticksvisible  yticklabelalign  yticksmirrored
flip_ylabel         subtitle           titlealign          width             xlabelpadding   xminorticks        xtickcolor          xticks              ygridcolor        ylabelvisible    yminortickwidth     yticklabelcolor  yticksvisible
halign              subtitlecolor      titlecolor          xautolimitmargin  xlabelrotation  xminorticksize     xtickformat         xticksize           ygridstyle        yminorgridcolor  ypankey             yticklabelfont   ytickwidth

Stacktrace:
 [1] validate_axis_attributes(attributes::Dict{Symbol, Any})
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\figureplotting.jl:101
 [2] create_axis_for_plot(figure::Figure, plot::Scatter{Tuple{Vector{Point{2, Float64}}}}, attributes::Dict{Symbol, Any})
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\figureplotting.jl:108
 [3] create_axis_like(plot::Scatter{Tuple{Vector{Point{2, Float64}}}}, attributes::Dict{Symbol, Any}, ::Nothing)
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\figureplotting.jl:211
 [4] _create_plot(::Function, ::Dict{Symbol, Any}, ::Vector{Float64}, ::Vararg{Vector{Float64}})
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\figureplotting.jl:329
 [5] scatter(::Vector{Float64}, ::Vararg{Vector{Float64}}; kw::@Kwargs{axis::@NamedTuple{blahblah::String}})
   @ MakieCore c:\Users\danjv\.julia\dev\Makie.jl\MakieCore\src\recipes.jl:449
 [6] top-level scope
   @ REPL[63]:1

Maybe something like this would generalise to Blocks in general if these checks were put in the appropriate places.. would probably address #4390 too.

DanielVandH commented 1 month ago

That solution also unfortunately fails for calls like

ulia> Axis(fig[1,1],bad=2)
ERROR: MethodError: no method matching initialize_block!(::Axis; bad::Int64)

Closest candidates are:
  initialize_block!(::Axis; palette) got unsupported keyword argument "bad"
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks\axis.jl:150
  initialize_block!(::Label) got unsupported keyword argument "bad"
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks\label.jl:3
  initialize_block!(::Toggle) got unsupported keyword argument "bad"
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks\toggle.jl:1
  ...

Stacktrace:
 [1] kwerr(::@NamedTuple{bad::Int64}, ::Function, ::Axis)
   @ Base .\error.jl:165
 [2] _block(T::Type{…}, fig_or_scene::Figure, args::Vector{…}, kwdict::Dict{…}, bbox::Nothing; kwdict_complete::Bool)
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:371
 [3] _block
   @ c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:291 [inlined]
 [4] #_block#1490
   @ c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:259 [inlined]
 [5] _block(::Type{Axis}, ::GridPosition; kwargs::@Kwargs{bad::Int64})
   @ Makie c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:253
 [6] _block
   @ c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:246 [inlined]
 [7] #_#1488
   @ c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:237 [inlined]
 [8] top-level scope
   @ REPL[67]:1
Some type information was truncated. Use `show(err)` to see complete types.

so I guess it needs to be injected into _block somewhere, but I have little idea about what's going on in this part. I'm guessing 

@ c:\Users\danjv\.julia\dev\Makie.jl\src\makielayout\blocks.jl:291 [inlined]

would be the place to do it when attribute_kwargs is constructed.

DanielVandH commented 1 month ago

See https://github.com/MakieOrg/Makie.jl/pull/4392