Basic Makie.jl support #165
Conversation
Benchmark Results
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
Just saw this really cool update from Julius on the AoG side of things! MakieOrg/AlgebraOfGraphics.jl#619 https://aog.makie.org/v0.9.6/examples/scales/units#units This also looks to address MakieOrg/Makie.jl#3890 in one fell swoop |
Cool! Let me know (by tagging me) when you're ready for review |
Hi @MilesCranmer, I think this should be just about ready for a first pass of reviews now. The main additions are:
I'm not quite sure yet how I feel about Makie's choice to automatically change non-compound units for folks out from under them, so I kept that bit of machinery out for now. I do see the convenience of it though, it just seems to add a fair bit of complexity that we maybe don't need right now. Instead, this current design favors explicit over implicit unit handling, e.g., Unitful example (taken from ReferenceTests)# Don't swallow units past the first
f, a, p = scatter((1:10) .* u"J/s")
# Don't simplify (assume the user knows better)
scatter(f[1, 2], (1:10) .* u"K", exp.(1:10) .* u"mm/m^2")
# Only change prefixes of simple units, not compound units
scatter(f[2, 1], 10 .^ (1:6) .* u"W/m^2", (1:6) .* 1000 .* u"nm")
# Only change units/prefixes for simple units when adding more plots
scatter(f[2, 2], (0:10) .* u"W/m^2", (0:10) .* u"g")
scatter!((0:10) .* u"kW/m^2", (0:10) .* u"kg")
f
Here the nanometer plot in the bottom left is auto-converted to microns without the user specifying that unit. In contrast: This PRusing DynamicQuantities, Makie
const DQConversion = Base.get_extension(DynamicQuantities, :DynamicQuantitiesMakieExt).DQConversion
fig = Figure()
ax1 = Axis(fig[1, 1]; dim2_conversion=DQConversion(us"J/s"))
ax2 = Axis(fig[1, 2]; dim2_conversion=DQConversion(us"mm/m^2"))
ax3 = Axis(fig[2, 1]; dim1_conversion=DQConversion(us"W/m^2"), dim2_conversion=DQConversion(us"μm"))
ax4 = Axis(fig[2, 2]; dim1_conversion=DQConversion(us"W/m^2"))
scatter!(ax1, (1:10) .* u"J/s")
scatter!(ax2, (1:10) .* u"K", exp.(1:10) .* u"mm/m^2")
scatter!(ax3, 10 .^ (1:6) .* u"W/m^2", (1:6) .* 1000 .* u"nm")
scatter!(ax4, (0:10) .* u"W/m^2", (0:10) .* u"g")
scatter!(ax4, (0:10) .* u"kW/m^2", (0:10) .* u"kg")
figor with the appropriate use of fig = Figure()
scatter(fig[1, 1], (1:10) .* us"J/s")
scatter(fig[1, 2], (1:10) .* u"K", exp.(1:10) .* us"mm/m^2")
scatter(fig[2, 1], 10 .^ (1:6) .* us"W/m^2", (1:6) .* 1000 .* u"nm"; axis=(; dim2_conversion=DQConversion(us"μm")))
scatter(fig[2, 2], (0:10) .* u"W/m^2", (0:10) .* u"g"; axis=(; dim1_conversion=DQConversion(us"W/m^2")))
scatter!(fig[2, 2], (0:10) .* u"kW/m^2", (0:10) .* us"kg")
fig
Happy to save that kind of discussion for a separate PR though if it's starting to get too in the weeds for this basic functionality PR. Thanks again for taking a look! |
bertini97 commented Apr 29, 2025
This would be great for a project I'm working on right now. Can it be pushed? |
Sure let me review it; sorry i missed the earlier notification. I think I unsubscribed from the thread as it was a draft |
Project.toml Outdated
| DynamicQuantitiesMeasurementsExt = "Measurements" | ||
| DynamicQuantitiesScientificTypesExt = "ScientificTypes" | ||
| DynamicQuantitiesUnitfulExt = "Unitful" | ||
| [compat] | ||
| DisDoctor = "0.4" | ||
| LinearAlgebra = "1" | ||
| Makie = "0.22.2" |
There was a problem hiding this comment.
Can you comment on why choose this version as a minimum compat?
There was a problem hiding this comment.
I think I was looking at Measurements.jl for prior art in adding Makie as a package extension when I was just getting started, and was just being conservative. Looking back, I think this could be relaxed to Makie v0.21.0 since it looks like that is when MakieOrg/Makie.jl#3226 was added
| y0 = 10km | ||
| v0 = 250m/s | ||
| θ = deg2rad(60) | ||
| g = 9.81m/s^2 | ||
| nothing # hide |
There was a problem hiding this comment.
| y0 = 10km | |
| v0 = 250m/s | |
| θ = deg2rad(60) | |
| g = 9.81m/s^2 | |
| nothing # hide | |
| y0 = 10km | |
| v0 = 250m/s | |
| θ = deg2rad(60) | |
| g = 9.81m/s^2; |
A bit simpler. Can you also use ; rather than nothing elsewhere too
There was a problem hiding this comment.
hmm, no dice just yet:
I noticed you mention this in your discussion with Abhro in #175, and I've wanted to try it out ever since. Is there something else that I could try? Found some relevant discussion here: JuliaDocs/Documenter.jl#1509
ext/DynamicQuantitiesMakieExt.jl Outdated
| DQConversion(unit=automatic; units_in_label=false) | ||
| Allows to plot arrays of DynamicQuantity objects into an axis. | ||
| # Arguments | ||
| - `unit=automatic`: sets the unit as conversion target. If left at automatic, the best unit will be chosen for all plots + values plotted to the axis (e.g. years for long periods, or km for long distances, or nanoseconds for short times). | ||
| - `units_in_label=true`: controls, whether plots are shown in the label_prefix of the axis labels, or in the tick labels | ||
| # Examples | ||
| ```julia | ||
| using DynamicQuantities, CairoMakie | ||
| # DQConversion will get chosen automatically: | ||
| scatter(1:4, [1u"ns", 2u"ns", 3u"ns", 4u"ns"]) | ||
| ``` | ||
| Fix unit to always use Meter & display unit in the xlabel: | ||
| ```julia | ||
| # Temporary until this is upstreamed to Makie.jl | ||
| const DQConversion = Base.get_extension(DynamicQuantities, :DynamicQuantitiesMakieExt).DQConversion | ||
| dqc = DQConversion(us"m"; units_in_label=false) | ||
| scatter(1:4, [0.01u"km", 0.02u"km", 0.03u"km", 0.04u"km"]; axis=(dim2_conversion=dqc, xlabel="x (km)")) | ||
| ``` |
There was a problem hiding this comment.
Can these be spaced out a bit more
| end | ||
| function M.convert_dim_observable(conversion::DQConversion, value_obs::M.Observable, deregister) | ||
| # TODO: replace with update_extrema |
There was a problem hiding this comment.
Right, so I think this ties into this bit of my comment here:
I'm not quite sure yet how I feel about MakieOrg/Makie.jl#4583 (comment) for folks out from under them, so I kept that bit of machinery out for now. I do see the convenience of it though, it just seems to add a fair bit of complexity that we maybe don't need right now. Instead, this current design favors explicit over implicit unit handling
So I've essentially side-stepped Makie's version of convert_dim_observable which calls to the best_unit business that I am not totally convinced about using
This issue that came up with using it (MakieOrg/Makie.jl#4940 (comment)) also makes me less inclined to use it. Idk though if you had any thoughts about this already?
There was a problem hiding this comment.
Update: Added a comment in that issue to hopefully get some more discussion going
Co-authored-by: Miles Cranmer <[email protected]>
Co-authored-by: Miles Cranmer <[email protected]>
Hi @MilesCranmer, haha, I think I missed a notification too, sorry for the delay! Thanks for the comments, I think I have responded to all of them now for your review |
Closes #164
Demo
See docs for more examples using Makie's dimension conversion machinery
TODO
Future PR(s)?