Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rangebars() does not honor convert_single_argument override #3655

Closed
3 tasks done
staticfloat opened this issue Feb 25, 2024 · 1 comment
Closed
3 tasks done

rangebars() does not honor convert_single_argument override #3655

staticfloat opened this issue Feb 25, 2024 · 1 comment
Labels
bug Makie Backend independent issues (Makie core)

Comments

@staticfloat
Copy link
Contributor

staticfloat commented Feb 25, 2024

  • are you running newest version (version from docs) ? (WGLMakie v0.9.7)
  • can you reproduce the bug with a fresh environment ? (]activate --temp; add Makie)
  • What platform + GPU are you on? (M1 MBP, macOS)

If I have a custom type that I want Makie to be able to peel via convert_single_argument() it works with things like lines(), but not rangebars():

using WGLMakie

struct CustomType
    v::Float64
end

Makie.convert_single_argument(c::CustomType) = c.v
Makie.convert_single_argument(cs::AbstractArray{<:CustomType}) = [c.v for c in cs]

# This works
xs = 1:10
ys = CustomType.(Float64.(1:10))
lines(xs, ys)
# This does not
rangebars(ys, xs .- 1, xs .+ 1)

The above results in:

julia> rangebars(ys, xs .- 1, xs .+ 1)

       # We must manually peel the ys for rangebars:
ERROR: MethodError: Cannot `convert` an object of type CustomType to an object of type Float32
The function `convert` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  convert(::Type{T}, ::Ratios.SimpleRatio{S}) where {T<:AbstractFloat, S}
   @ Ratios ~/.julia/packages/Ratios/FsiCW/src/Ratios.jl:51
  convert(::Type{T}, ::Number) where T<:Number
   @ Base number.jl:7
  convert(::Type{T}, ::Quantity) where T<:Real
   @ Unitful ~/.julia/packages/Unitful/R4J37/src/conversion.jl:139
  ...

Stacktrace:
  [1] macro expansion
    @ ~/.julia/packages/StaticArraysCore/P6PH7/src/StaticArraysCore.jl:88 [inlined]
  [2] convert_ntuple
    @ ~/.julia/packages/StaticArraysCore/P6PH7/src/StaticArraysCore.jl:84 [inlined]
  [3] Vec{3, Float32}(x::Tuple{CustomType, Int64, Int64})
    @ GeometryBasics ~/.julia/packages/GeometryBasics/bLARu/src/fixed_arrays.jl:19
  [4] StaticArray
    @ ~/.julia/packages/StaticArrays/oOCPP/src/convert.jl:165 [inlined]
  [5] _broadcast_getindex_evalf
    @ ./broadcast.jl:673 [inlined]
  [6] _broadcast_getindex
    @ ./broadcast.jl:646 [inlined]
  [7] getindex
    @ ./broadcast.jl:605 [inlined]
  [8] copy
    @ ./broadcast.jl:906 [inlined]
  [9] materialize
    @ ./broadcast.jl:867 [inlined]
 [10] broadcast
    @ ./broadcast.jl:805 [inlined]
 [11] convert_arguments
    @ ~/.julia/packages/Makie/z2T2o/src/basic_recipes/error_and_rangebars.jl:119 [inlined]
 [12] (Plot{Makie.rangebars})(args::Tuple{Vector{…}, UnitRange{…}, UnitRange{…}}, plot_attributes::Dict{Symbol, Any})
    @ Makie ~/.julia/packages/Makie/z2T2o/src/interfaces.jl:139
 [13] _create_plot(::Function, ::Dict{Symbol, Any}, ::Vector{CustomType}, ::Vararg{Any})
    @ Makie ~/.julia/packages/Makie/z2T2o/src/figureplotting.jl:248
 [14] rangebars(::Vector{CustomType}, ::Vararg{Any}; kw::@Kwargs{})
    @ Makie ~/.julia/packages/MakieCore/UAwps/src/recipes.jl:175
 [15] top-level scope
    @ REPL[99]:1
Some type information was truncated. Use `show(err)` to see complete types.
# We must manually peel the ys for rangebars:
rangebars(Makie.convert_single_argument(ys), xs .- 1, xs .+ 1; direction=:x)
@asinghvi17 asinghvi17 added the Makie Backend independent issues (Makie core) label Feb 25, 2024
@SimonDanisch
Copy link
Member

I will take a look at this as part of #3226 ... I think, this happens, because rangebars overloads convert_arguments quite broadly so it doesn't get dispatched to Makie's generic convert_arguments, which is the one that applies convert_single_argument.

SimonDanisch added a commit that referenced this issue Apr 30, 2024
* take over most of the work from #1347

* add typed argument conversion (#3565)

* add typed argument conversion

* fix volume

* add function to get available conversions

* make conversion apply more narrowly

* more cleanly separate recursion in convert_arguments

* clean up

* allow to get axis before creating a plot

* clean up

* fix tests

* bring back dim converts (axis_convert)

* update tests

* fix tests and work around conversion problems

* fix WGLMakie

* fix errors

* clean up conversion pipeline

* fix tests

* add changelog entry

* disable project run

* improve performance slightly

* might as well use array

* tmp

* wip

* implement axis convert recursion

* fix tests

* fix datashader

* fix datashader

* move unitful integration

* fix performance regression!?

* fix merge & new date time improvements

* fix scaling test

* remove test false

* clean up

* converts shouldnt be here

* move axis converts to scene

* further refactor [skip ci]

* finish refactor for AxisConversion type

* allow limit setting and ticks

* make tests less noisy

* cleanup

* clean up and fix unitful/date conversion

* make sure all tests work correctly

* remove rand

* rename, clean up and make axis spec work

* clean up and test new conversion pipeline

* undo feature deletion, don't reintroduce Rect2f

* be explicit about Volume Interval types

* minor docstring cleanup

* try to clarify new conversion docstrings

* remove convert_arguments_typed in favor of types_for_plot_arguments

* fix remaining bugs for conversion simplification

* fix ticks not updating

* fix specapi

* fix qqnorm

* clean up types_for_plot_arguments

* fix tuple conversion

* try to fix compile time regression

* try to fix compile time regression

* clean up and introduce expand_dimensions

* fix #3655 and clean up convert_arguments + add tests

* fix #3509 and add tests for

* clean up observables and more docs

* final rename

* fix docs

* cleanup

* small clean up

* small doc improvements

* improve docs

* fix docs

* try relative link

* try without .md

* take out link

* try fix

---------

Co-authored-by: ffreyer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Makie Backend independent issues (Makie core)
Projects
None yet
Development

No branches or pull requests

3 participants