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

Change ImageLike and VolumeLike deprecation warnings to errors #4685

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@
- Added `transform_marker` attribute to meshscatter and changed the default behavior to not transform marker/mesh vertices [#4606](https://github.com/MakieOrg/Makie.jl/pull/4606)
- Fixed some issues with meshscatter not correctly transforming with transform functions and float32 rescaling [#4606](https://github.com/MakieOrg/Makie.jl/pull/4606)
- Fixed `poly` pipeline for 3D and/or Float64 polygons that begin from an empty vector [#4615](https://github.com/MakieOrg/Makie.jl/pull/4615).
- `empty!` GLMakie screen instead of closing, fixing issue with resetted window position [#3881](https://github.com/MakieOrg/Makie.jl/pull/3881)
- `empty!` GLMakie screen instead of closing, fixing issue with reset window position [#3881](https://github.com/MakieOrg/Makie.jl/pull/3881)
- Added option to display the front spines in Axis3 to close the outline box [#2349](https://github.com/MakieOrg/Makie.jl/pull/4305)
- Fixed gaps in corners of `poly(Rect2(...))` stroke [#4664](https://github.com/MakieOrg/Makie.jl/pull/4664)
- Fixed an issue where `reinterpret`ed arrays of line points were not handled correctly in CairoMakie [#4668](https://github.com/MakieOrg/Makie.jl/pull/4668).
- Fixed various issues with `markerspace = :data`, `transform_marker = true` and `rotation` for scatter in CairoMakie (incorrect marker transformations, ignored transformations, Cairo state corruption) [#4663](https://github.com/MakieOrg/Makie.jl/pull/4663)
- Changed deprecation warnings for Vector and Range inputs in `image`, `volume`, `voxels` and `spy` into **errors** [#4685](https://github.com/MakieOrg/Makie.jl/pull/4685)

## [0.21.18] - 2024-12-12

Expand Down
42 changes: 21 additions & 21 deletions ReferenceTests/src/tests/generic_components.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
@reference_test "picking" begin
scene = Scene(size = (230, 370))
campixel!(scene)

sc1 = scatter!(scene, [20, NaN, 20], [20, NaN, 50], marker = Rect, markersize = 20)
sc2 = scatter!(scene, [50, 50, 20, 50], [20, 50, 80, 80], marker = Circle, markersize = 20, color = [:red, :red, :transparent, :red])
ms = meshscatter!(scene, [20, NaN, 50], [110, NaN, 110], markersize = 10)
Expand All @@ -18,16 +18,16 @@
hm = heatmap!(scene, [80, 110, 140], [140, 170], [1 4; 2 5; 3 6])
# mesh coloring should match triangle placements
m = mesh!(scene, Point2f.([80, 80, 110, 110], [200, 230, 200, 230]), [1 2 3; 2 3 4], color = [1,1,1,2])
vx = voxels!(scene, [65, 155], [245, 305], [-1, 1], reshape([1,2,3,4,5,6], (3,2,1)), shading = NoShading)
vx = voxels!(scene, (65, 155), (245, 305), (-1, 1), reshape([1,2,3,4,5,6], (3,2,1)), shading = NoShading)
vol = volume!(scene, 80..110, 320..350, -1..1, rand(2,2,2))

# reversed axis
i2 = image!(scene, 210..180, 20..50, rand(RGBf, 2, 2))
s2 = surface!(scene, 210..180, 80..110, [1 2; 3 4], interpolate = false)
hm2 = heatmap!(scene, [210, 180], [140, 170], [1 2; 3 4])

# for ranged picks
m2 = mesh!(scene,
m2 = mesh!(scene,
Point2f[(190, 330), (200, 330), (190, 340), (200, 340)],
[1 2 4; 1 4 3]
)
Expand All @@ -52,7 +52,7 @@
else
error("picking tests are only meant to run on GLMakie & WGLMakie")
end

# raw picking tests
@testset "pick(scene, point)" begin
@testset "scatter" begin
Expand Down Expand Up @@ -108,7 +108,7 @@
@test pick(scene, 61, 290) == (nothing, 0)
end

@testset "text" begin
@testset "text" begin
@test pick(scene, 15, 320) == (t, 1)
@test pick(scene, 13, 320) == (nothing, 0)
# edge checks, further outside due to AA
Expand All @@ -131,7 +131,7 @@
)
@test pick(scene, p) == (nothing, 0)
end

# cell centered checks
@test pick(scene, 90, 30) == (i, 1)
@test pick(scene, 110, 30) == (i, 2)
Expand All @@ -145,7 +145,7 @@
@test pick(scene, 100+1, 35-1) == (i, 2)
@test pick(scene, 100-1, 35+1) == (i, 4)
@test pick(scene, 100+1, 35+1) == (i, 5)

@test pick(scene, 120-1, 35-1) == (i, 2)
@test pick(scene, 120+1, 35-1) == (i, 3)
@test pick(scene, 120-1, 35+1) == (i, 5)
Expand All @@ -166,7 +166,7 @@
)
@test pick(scene, p) == (nothing, 0)
end

# cell centered checks
@test pick(scene, 90, 90) == (s, 1)
@test pick(scene, 110, 90) == (s, 2)
Expand All @@ -180,7 +180,7 @@
@test pick(scene, 95+1, 95-1) == (s, 2)
@test pick(scene, 95-1, 95+1) == (s, 4)
@test pick(scene, 95+1, 95+1) == (s, 5)

@test pick(scene, 125-1, 95-1) == (s, 2)
@test pick(scene, 125+1, 95-1) == (s, 3)
@test pick(scene, 125-1, 95+1) == (s, 5)
Expand All @@ -201,7 +201,7 @@
)
@test pick(scene, p) == (nothing, 0)
end

# cell centered checks
@test pick(scene, 80, 140) == (hm, 1)
@test pick(scene, 110, 140) == (hm, 2)
Expand All @@ -215,7 +215,7 @@
@test pick(scene, 96, 154) == (hm, 2)
@test pick(scene, 94, 156) == (hm, 4)
@test pick(scene, 96, 156) == (hm, 5)

@test pick(scene, 124, 154) == (hm, 2)
@test pick(scene, 126, 154) == (hm, 3)
@test pick(scene, 124, 156) == (hm, 5)
Expand Down Expand Up @@ -251,7 +251,7 @@
)
@test pick(scene, p) == (nothing, 0)
end

# cell centered checks
@test pick(scene, 80, 260) == (vx, 1)
@test pick(scene, 110, 260) == (vx, 2)
Expand All @@ -265,15 +265,15 @@
@test pick(scene, 96, 274) == (vx, 2)
@test pick(scene, 94, 276) == (vx, 4)
@test pick(scene, 96, 276) == (vx, 5)

@test pick(scene, 124, 274) == (vx, 2)
@test pick(scene, 126, 274) == (vx, 3)
@test pick(scene, 124, 276) == (vx, 5)
@test pick(scene, 126, 276) == (vx, 6)
end

@testset "volume" begin
# volume doesn't produce indices because we can't resolve the depth of
# volume doesn't produce indices because we can't resolve the depth of
# the pick
@test pick(scene, 80, 320)[1] == vol
@test pick(scene, 79, 320) == (nothing, 0)
Expand Down Expand Up @@ -303,7 +303,7 @@
@testset "linesegments" begin
@test pick(scene, 5, 280, 10) == (ls, 6)
end
@testset "text" begin
@testset "text" begin
@test pick(scene, 32, 320, 10) == (t, 1)
@test pick(scene, 35, 320, 10) == (t, 3)
end
Expand Down Expand Up @@ -393,7 +393,7 @@
end

#=
For Verfication
For Verfication
Note that the text only marks the index in the picking list. The position
that is closest (that pick_sorted used) is somewhere else in the marked
element. Check scene2 to see the pickable regions if unsure
Expand All @@ -410,15 +410,15 @@
end
scatter!(scene, Vec2f(100, 100), color = :white, strokecolor = :black, strokewidth = 2, overdraw = true)
text!(
scene, ps, text = ["$i" for i in 1:14],
strokecolor = :white, strokewidth = 2,
scene, ps, text = ["$i" for i in 1:14],
strokecolor = :white, strokewidth = 2,
align = (:center, :center), overdraw = true)
=#

# pick(scene, Rect)
# grab all indices and generate a plot for them (w/ fixed px_per_unit)
full_screen = last.(pick(scene, scene.viewport[]))

scene2 = Scene(size = 2.0 .* widths(scene.viewport[]))
campixel!(scene2)
image!(scene2, full_screen, colormap = :viridis)
Expand Down
12 changes: 8 additions & 4 deletions ReferenceTests/src/tests/primitives.jl
Original file line number Diff line number Diff line change
Expand Up @@ -861,21 +861,25 @@ end
img = [2 0 0 3; 0 0 0 0; 1 1 0 0; 1 1 0 4]

f = Figure(size = (600, 400))
to_tuple(x) = (first(x), last(x))
to_tuple(x::Makie.Interval) = (Makie.leftendpoint(x), Makie.rightendpoint(x))

for (i, interp) in enumerate((true, false))
for (j, plot_func) in enumerate((
(fp, x, y, cs, interp) -> image(fp, x, y, cs, colormap = :viridis, interpolate = interp),
(fp, x, y, cs, interp) -> image(fp, to_tuple(x), to_tuple(y), cs, colormap = :viridis, interpolate = interp),
(fp, x, y, cs, interp) -> heatmap(fp, x, y, cs, colormap = :viridis, interpolate = interp),
(fp, x, y, cs, interp) -> surface(fp, x, y, zeros(size(cs)), color = cs, colormap = :viridis, interpolate = interp, shading = NoShading)
))

gl = GridLayout(f[i, j])

a, p = plot_func(gl[1, 1], (1, 4), (1, 4), img, interp)
# Test forwards + backwards for each: Tuple, Interval, Range, Vector

a, p = plot_func(gl[1, 1], 1:4, (1, 4), img, interp)
hidedecorations!(a)
a, p = plot_func(gl[2, 1], (1, 4), 4..1, img, interp)
a, p = plot_func(gl[2, 1], [1, 2, 3, 4], 4..1, img, interp)
hidedecorations!(a)
a, p = plot_func(gl[1, 2], (4, 1), (1, 4), img, interp)
a, p = plot_func(gl[1, 2], 4:-1:1, 1..4, img, interp)
hidedecorations!(a)
a, p = plot_func(gl[2, 2], (4, 1), [4, 3, 2, 1], img, interp)
hidedecorations!(a)
Expand Down
4 changes: 2 additions & 2 deletions src/basic_recipes/spy.jl
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ function convert_arguments(::Type{<:Spy}, matrix::AbstractMatrix{T}) where T
end

function convert_arguments(::Type{<:Spy}, xs, ys, matrix::AbstractMatrix)
x = to_endpoints(xs, "x")
y = to_endpoints(ys, "y")
x = to_endpoints(xs, "x", "Spy")
y = to_endpoints(ys, "y", "Spy")
return (x, y, convert(SparseArrays.SparseMatrixCSC, matrix))
end

Expand Down
6 changes: 3 additions & 3 deletions src/basic_recipes/voxels.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ function Makie.convert_arguments(T::Type{<:Voxels}, chunk::Array{<: Real, 3})
end

function convert_arguments(T::Type{<:Voxels}, xs, ys, zs, chunk::Array{<: Real, 3})
xi = Float32.(to_endpoints(xs))
yi = Float32.(to_endpoints(ys))
zi = Float32.(to_endpoints(zs))
xi = Float32.(to_endpoints(xs, "x", Voxels))
yi = Float32.(to_endpoints(ys, "y", Voxels))
zi = Float32.(to_endpoints(zs, "z", Voxels))
return convert_arguments(T, xi, yi, zi, chunk)
end

Expand Down
52 changes: 33 additions & 19 deletions src/conversions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -371,10 +371,17 @@ function to_endpoints(x::Tuple{<:Real,<:Real})
end
to_endpoints(x::Interval) = to_endpoints(endpoints(x))
to_endpoints(x::EndPoints) = x
to_endpoints(x::AbstractVector) = to_endpoints((first(x), last(x)))
function to_endpoints(x, dim)
x isa AbstractVector && !(x isa EndPoints) && print_range_warning(dim, x)
return to_endpoints(x)

to_endpoints(x::AbstractVector, side, trait) = throw_range_error(x, side, trait)
to_endpoints(x::Union{Tuple, EndPoints, Interval}, side, trait) = to_endpoints(x)

function throw_range_error(value::T, side, trait) where {T}
error(
"Encountered `$T` with value $value on side $side in `convert_arguments` for the `$trait`
conversion. Using `$T` to specify one dimension of `$trait` is deprecated because `$trait`
sides always need exactly two values, start and stop. Use interval notation `start .. stop`,
a two-element tuple `(start, stop)` or `Makie.EndPoints(start, stop)` instead."
)
end

function convert_arguments(::GridBased, x::EndPointsLike, y::EndPointsLike,
Expand Down Expand Up @@ -411,17 +418,10 @@ function convert_arguments(::CellGrid, x::EndPointsLike, y::EndPointsLike,
return (EndPoints{Tx}(xe[1] - xstep, xe[2] + xstep), EndPoints{Ty}(ye[1] - ystep, ye[2] + ystep), el32convert(z))
end

function print_range_warning(side::String, value)
@warn "Encountered an `AbstractVector` with value $value on side $side in `convert_arguments` for the `ImageLike` trait.
Using an `AbstractVector` to specify one dimension of an `ImageLike` is deprecated because `ImageLike` sides always need exactly two values, start and stop.
Use interval notation `start .. stop` or a two-element tuple `(start, stop)` instead."
end


function convert_arguments(::ImageLike, xs::RangeLike, ys::RangeLike,
data::AbstractMatrix{<:Union{Real,Colorant}})
x = to_endpoints(xs, "x")
y = to_endpoints(ys, "y")
x = to_endpoints(xs, "x", ImageLike)
y = to_endpoints(ys, "y", ImageLike)
return (x, y, el32convert(data))
end

Expand Down Expand Up @@ -470,7 +470,8 @@ end

function convert_arguments(::VolumeLike, x::RangeLike, y::RangeLike, z::RangeLike,
data::RealArray{3})
return (to_endpoints(x, "x"), to_endpoints(y, "y"), to_endpoints(z, "z"), el32convert(data))
return (to_endpoints(x, "x", VolumeLike), to_endpoints(y, "y", VolumeLike),
to_endpoints(z, "z", VolumeLike), el32convert(data))
end

"""
Expand All @@ -481,7 +482,8 @@ Takes 3 `AbstractVector` `x`, `y`, and `z` and the `AbstractMatrix` `i`, and put
`P` is the plot Type (it is optional).
"""
function convert_arguments(::VolumeLike, x::RealVector, y::RealVector, z::RealVector, i::RealArray{3})
(to_endpoints(x, "x"), to_endpoints(y, "y"), to_endpoints(z, "z"), el32convert(i))
return (to_endpoints(x, "x", VolumeLike), to_endpoints(y, "y", VolumeLike),
to_endpoints(z, "z", VolumeLike), el32convert(i))
end

################################################################################
Expand Down Expand Up @@ -580,7 +582,7 @@ function convert_arguments(::Type{<:Mesh}, geom::GeometryPrimitive{N, T}) where
# we convert to UV mesh as default, because otherwise the uv informations get lost
# - we can still drop them, but we can't add them later on
m = GeometryBasics.expand_faceviews(GeometryBasics.uv_normal_mesh(
geom; pointtype = Point{N, float_type(T)},
geom; pointtype = Point{N, float_type(T)},
uvtype = Vec2f, normaltype = Vec3f, facetype = GLTriangleFace
))
return (m,)
Expand Down Expand Up @@ -646,6 +648,13 @@ function convert_arguments(::Type{<:Arrows}, x::RealVector, y::RealVector, z::Re
return (vec(points), vec(f_out))
end

# Note: AbstractRange must be linear
is_regularly_spaced(x::AbstractRange) = true
function is_regularly_spaced(x::AbstractVector)
delta = x[2] - x[1]
return all(i -> x[i] - x[i-1] ≈ delta, 3:length(x))
end

"""
convert_arguments(P, x, y, z, f)::(Vector, Vector, Vector, Matrix)

Expand All @@ -654,16 +663,21 @@ spanned by `x`, `y` and `z`, and puts `x`, `y`, `z` and `f(x,y,z)` in a Tuple.

`P` is the plot Type (it is optional).
"""
function convert_arguments(VL::VolumeLike, x::RealVector, y::RealVector, z::RealVector, f::Function)
function convert_arguments(::VolumeLike, x::RealVector, y::RealVector, z::RealVector, f::Function)
if !applicable(f, x[1], y[1], z[1])
error("You need to pass a function with signature f(x, y, z). Found: $f")
end
# Verify grid regularity
is_regularly_spaced(x) || throw_range_error(x, "x", VolumeLike)
is_regularly_spaced(y) || throw_range_error(y, "y", VolumeLike)
is_regularly_spaced(z) || throw_range_error(z, "z", VolumeLike)

_x, _y, _z = ntuple(Val(3)) do i
A = (x, y, z)[i]
return reshape(A, ntuple(j -> j != i ? 1 : length(A), Val(3)))
end
# TODO only allow unitranges to map over since we dont support irregular x/y/z values
return (map(to_endpoints, (x, y, z))..., el32convert.(f.(_x, _y, _z)))

return (map(v -> to_endpoints((first(v), last(v))), (x, y, z))..., el32convert.(f.(_x, _y, _z)))
end

function convert_arguments(P::Type{<:AbstractPlot}, r::RealVector, f::Function)
Expand Down
2 changes: 1 addition & 1 deletion test/boundingboxes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ end
@test bb.origin ≈ Point3f(0)
@test bb.widths ≈ Vec3f(10.0, 10.0, 0)

fig, ax, p = image(1..0, 1:10, rand(10, 10))
fig, ax, p = image(1..0, 1..10, rand(10, 10))
bb = boundingbox(p)
@test bb.origin ≈ Point3f(0, 1, 0)
@test bb.widths ≈ Vec3f(1.0, 9.0, 0)
Expand Down
19 changes: 14 additions & 5 deletions test/conversions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -351,16 +351,24 @@ end

o3 = Float32.(m3)

t1 = (1, 10)
t2 = (1, 6)

xx = convert_arguments(Image, m3)
xx == ((0.0f0, 10.0f0), (0.0f0, 6.0f0), o3)
@testset "ImageLike conversion" begin
@test convert_arguments(Image, m3) == ((0.0f0, 10.0f0), (0.0f0, 6.0f0), o3)
@test convert_arguments(Image, v1, r2, m3) == ((1.0f0, 10.0f0), (1.0f0, 6.0f0), o3)
@test convert_arguments(Image, i1, v2, m3) == ((1.0f0, 10.0f0), (1.0f0, 6.0f0), o3)
@test convert_arguments(Image, v3, i1, m3) == ((10, 1), (1, 10), o3)
@test convert_arguments(Image, v1, i3, m3) == ((1, 10), (10, 1), o3)
@test convert_arguments(Image, i1, i2, m3) == ((1.0, 10.0), (1.0, 6.0), o3)
@test convert_arguments(Image, i1, t2, m3) == ((1.0, 10.0), (1.0, 6.0), o3)
@test convert_arguments(Image, t1, t2, m3) == ((1.0, 10.0), (1.0, 6.0), o3)

@test_throws ErrorException convert_arguments(Image, v1, r2, m3)
@test_throws ErrorException convert_arguments(Image, i1, v2, m3)
@test_throws ErrorException convert_arguments(Image, v3, i1, m3)
@test_throws ErrorException convert_arguments(Image, v1, i3, m3)

# TODO: Should probably fail because it's not accepted by backends?
@test convert_arguments(Image, m1, m2, m3) === (m1, m2, m3)
@test convert_arguments(Heatmap, m1, m2) === (m1, m2)
end

@testset "VertexGrid conversion" begin
Expand All @@ -376,6 +384,7 @@ end
end

@testset "CellGrid conversion" begin
@test convert_arguments(Heatmap, m1, m2) === (m1, m2)
o1 = (0.5, 10.5)
o2 = (0.5, 6.5)
or1 = (0.5:1:10.5)
Expand Down
Loading
Loading