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

use concrete type instead of abstract Point2 #23

Merged
merged 5 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
8 changes: 5 additions & 3 deletions Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "TernaryDiagrams"
uuid = "54198458-1476-45ac-9e44-88a4201bfea6"
authors = ["St. Elmo Wilken <[email protected]> and contributors"]
version = "0.1.2"
version = "0.1.3"

[deps]
ColorSchemes = "35d6a980-a343-548e-a6ea-1d62b119f2f4"
Expand All @@ -12,10 +12,11 @@ VoronoiDelaunay = "72f80fcb-8c52-57d9-aff0-40c1a3526986"

[compat]
Aqua = "0.8"
CairoMakie = "0.11, 0.12"
ColorSchemes = "3"
DocStringExtensions = "0.9"
GeometricalPredicates = "0.4"
GLMakie = "0.9, 0.10"
GeometricalPredicates = "0.4"
JLD2 = "0.5"
Makie = "0.20, 0.21"
ReferenceTests = "0.10"
Expand All @@ -26,11 +27,12 @@ julia = "1"

[extras]
Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"
CairoMakie = "13f3f980-e62b-5c42-98c6-ff1f3baf88f0"
GLMakie = "e9467ef8-e4e7-5192-8a1a-b1aee30e663a"
JLD2 = "033835bb-8acc-5ee8-8aae-3f567f8a3819"
ReferenceTests = "324d217c-45ce-50fc-942e-d289b448e8cf"
SafeTestsets = "1bc83da4-3b8d-516f-aca4-4fe02f6d838f"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"

[targets]
test = ["Aqua", "GLMakie", "JLD2", "ReferenceTests", "SafeTestsets", "Test"]
test = ["Aqua", "CairoMakie", "GLMakie", "JLD2", "ReferenceTests", "SafeTestsets", "Test"]
Binary file modified figs/axis.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified figs/contour.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified figs/contourfill.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified figs/lines.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified figs/scatter.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified figs/temp.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
20 changes: 10 additions & 10 deletions src/axis.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
function draw_triangle_base!(tr::TernaryAxis)
lines!(tr, [Point2(r1...), Point2(r2...), Point2(r3...), Point2(r1...)], color = :black)
lines!(tr, [Point2f(r1...), Point2f(r2...), Point2f(r3...), Point2f(r1...)], color = :black)
end

function draw_triangle_vertex_labels!(tr::TernaryAxis)
Expand Down Expand Up @@ -43,7 +43,7 @@ function draw_triangle_axis_labels!(tr::TernaryAxis)
x1 = x0 - sqrt(3) * (y1 - y0)
isnothing(tr.labelz_arrow[]) || text!(
tr,
Point2(x1, y1);
Point2f(x1, y1);
text = tr.labelz_arrow[],
align = (:center, :center),
rotation = π / 3 * arrow_label_rot_adj, # sometimes this is not aligned
Expand All @@ -60,7 +60,7 @@ function draw_triangle_axis_labels!(tr::TernaryAxis)
x1 = x0 + sqrt(3) * (y1 - y0)
isnothing(tr.labely_arrow[]) || text!(
tr,
Point2(x1, y1);
Point2f(x1, y1);
text = tr.labely_arrow[],
align = (:center, :center),
rotation = -π / 3 * arrow_label_rot_adj,
Expand All @@ -83,7 +83,7 @@ function draw_triangle_axis_labels!(tr::TernaryAxis)
y1 = y0 - y_adj
isnothing(tr.labelx_arrow[]) || text!(
tr,
Point2(x1, y1);
Point2f(x1, y1);
text = tr.labelx_arrow[],
align = (:center, :center),
fontsize = tr.arrow_label_fontsize[] * !tr.hide_triangle_labels[],
Expand All @@ -106,8 +106,8 @@ function draw_grid!(tr::TernaryAxis)
vec1 = [f1, f2, 0]
vec2 = [f1, 0, f2]

x1 = Point2((R*vec1)[2:3]...)
x2 = Point2((R*vec2)[2:3]...)
x1 = Point2f((R*vec1)[2:3]...)
x2 = Point2f((R*vec2)[2:3]...)

lines!(tr, [x1, x2], linewidth = grid_line_width, color = grid_line_color)

Expand All @@ -128,8 +128,8 @@ function draw_grid!(tr::TernaryAxis)
vec1 = [0, f2, f1]
vec2 = [f1, f2, 0]

x1 = Point2((R*vec1)[2:3]...)
x2 = Point2((R*vec2)[2:3]...)
x1 = Point2f((R*vec1)[2:3]...)
x2 = Point2f((R*vec2)[2:3]...)

lines!(tr, [x1, x2], linewidth = grid_line_width, color = grid_line_color)

Expand All @@ -150,8 +150,8 @@ function draw_grid!(tr::TernaryAxis)
vec1 = [f2, 0, f1]
vec2 = [0, f2, f1]

x1 = Point2((R*vec1)[2:3]...)
x2 = Point2((R*vec2)[2:3]...)
x1 = Point2f((R*vec1)[2:3]...)
x2 = Point2f((R*vec2)[2:3]...)

lines!(tr, [x1, x2], linewidth = grid_line_width, color = grid_line_color)

Expand Down
4 changes: 2 additions & 2 deletions src/scatter.jl
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
function Makie.plot!(tr::TernaryScatter)

# create observables
dpoints = Observable(Point2[])
dpoints = Observable(Point2f[])

function update_plot(xs, ys, zs)
empty!(dpoints[])
for (x, y, z) in zip(xs, ys, zs)
carts = R * [x, y, z]
push!(dpoints[], Point2(carts[2], carts[3]))
push!(dpoints[], Point2f(carts[2], carts[3]))
end
end

Expand Down
12 changes: 12 additions & 0 deletions test/issues.jl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not create a new file specific to issues, it might be better to append it to the reference tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to have only tests that compare produced plots/images against reference images in the referencetests.jl file to keep the different types of tests separate, but I would of course be fine to move them if you insist :)

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#This testset checks that reported issues have been resolved; prevents regressions.

#https://github.com/stelmo/TernaryDiagrams.jl/issues/22
using CairoMakie
using TernaryDiagrams
@test_nowarn begin
fig = Figure()
ax = Axis(fig[1, 1])
ternaryaxis!(ax)
ternaryscatter!(ax, [0.2, 0.1], [0.2, 0.6], [0.6, 0.3])
fig
end
70 changes: 45 additions & 25 deletions test/referencetests.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using GLMakie
#using ColorSchemes
using TernaryDiagrams
using JLD2
using ReferenceTests
Expand All @@ -10,42 +9,56 @@ a2 = a2[1:20]
a3 = a3[1:20]
mus = mus[1:20]

#Somehow fig seems to get converted in AbstractArray{<:Colorant} of different sizes, depending on
#what display is used; the function below is a workaround through file saving/loading. Not
#elegant, but it works.
function save_load_remove(fig)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I asked on Slack and one can use colorbuffer(fig) to directly get a Matrix of Colorant.

Copy link
Contributor Author

@thomvet thomvet Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but at least when I am testing it locally, I get the wrong size of matrix out. Doing:

fig = Figure(size = (900, 600), px_per_unit = 2)
arr = colorbuffer(fig)

I would have expected an array of 1800 x 1200 pixels, but I am getting a 900×1350 PermutedDimsArray(view(::Array{RGB{N0f8},2}, :, 900:-1:1), (2, 1)) with eltype ColorTypes.RGB{FixedPointNumbers.N0f8} returned to me.

Furthermore, what I am getting back is dependent on what display/monitor I am having the figure open on! I have two monitors: monitor 1 gives the above. When I move the same figure and run arr = colorbuffer(fig) again, I am getting a 604 x 907 matrix. Monitor 1 is landscape, monitor 2 is portrait.

That kind of made me move in the direction of a workaround...

Is there a direct and robust way of doing this correctly?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm let me ask again on Slack if there is a standard way to do this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But anyway that's off topic from this MR. Let's just get this merged and we can apply an eventual better solution in a separate MR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solution seems to be colorbuffer(fig; px_per_unit=2) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a new commit; needed some more tweaking, but I think this version is cleaner than before.

n = "tmp.png"
save(n, fig, size = (900, 600), px_per_unit = 2)
arr = load(n)
rm(n)
return arr
end

function testimage_axis()
fig = Figure();
ax = Axis(fig[1, 1]);
fig = Figure()
ax = Axis(fig[1, 1])

ternaryaxis!(
ax;
labelx = "a1",
labely = "a2",
labelz = "a3",
# more options available, check out attributes with ?ternaryaxis
)

xlims!(ax, -0.2, 1.2) # to center the triangle
ylims!(ax, -0.3, 1.1) # to center the triangle
hidedecorations!(ax) # to hide the axis decos
fig

arr = save_load_remove(fig)
return arr
end

function testimage_lines()
fig = Figure();
ax = Axis(fig[1, 1]);
fig = Figure()
ax = Axis(fig[1, 1])

ternaryaxis!(ax);
ternaryaxis!(ax)
ternarylines!(ax, a1, a2, a3; color = :blue)

xlims!(ax, -0.2, 1.2)
ylims!(ax, -0.3, 1.1)
hidedecorations!(ax)
fig

arr = save_load_remove(fig)
return arr
end

function testimage_scatter()
fig = Figure();
ax = Axis(fig[1, 1]);
fig = Figure()
ax = Axis(fig[1, 1])

ternaryaxis!(ax);
ternaryaxis!(ax)
ternaryscatter!(
ax,
a1,
Expand All @@ -59,12 +72,14 @@ function testimage_scatter()
xlims!(ax, -0.2, 1.2)
ylims!(ax, -0.3, 1.1)
hidedecorations!(ax)
fig

arr = save_load_remove(fig)
return arr
end

function testimage_contour()
fig = Figure();
ax = Axis(fig[1, 1]);
fig = Figure()
ax = Axis(fig[1, 1])

ternarycontour!(
ax,
Expand All @@ -79,28 +94,32 @@ function testimage_contour()
pad_data = true,
)

ternaryaxis!(ax);
ternaryaxis!(ax)

xlims!(ax, -0.2, 1.2)
ylims!(ax, -0.3, 1.1)
hidedecorations!(ax)
fig

arr = save_load_remove(fig)
return arr
end

function testimage_contourf()
fig = Figure();
ax = Axis(fig[1, 1]);
fig = Figure()
ax = Axis(fig[1, 1])
ternarycontourf!(ax, a1, a2, a3, mus; levels = 10)
ternaryaxis!(ax);
ternaryaxis!(ax)
xlims!(ax, -0.2, 1.2)
ylims!(ax, -0.3, 1.1)
hidedecorations!(ax)
fig

arr = save_load_remove(fig)
return arr
end

function testimage_temp()
fig = Figure();
ax = Axis(fig[1, 1]);
fig = Figure(size = (900, 600), px_per_unit = 2)
ax = Axis(fig[1, 1])

ternarycontourf!(
ax,
Expand Down Expand Up @@ -135,13 +154,14 @@ function testimage_temp()
markersize = 10,
)

ternaryaxis!(ax);
ternaryaxis!(ax)

xlims!(ax, -0.2, 1.2)
ylims!(ax, -0.3, 1.1)
hidedecorations!(ax)
fig

arr = save_load_remove(fig)
return arr
end

@test_reference "../figs/axis.png" testimage_axis()
Expand Down
1 change: 1 addition & 0 deletions test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ using Test

@testset "TernaryDiagrams" begin
@safetestset "Aqua" include("aqua.jl")
@safetestset "Issue check" include("issues.jl")
@safetestset "ReferenceTests" include("referencetests.jl")
end