jw3126 / UnitfulRecipes.jl

Plots.jl recipes for Unitful.jl arrays
MIT License
37 stars 10 forks source link

Fix String -> AbstractString #35

Closed briochemc closed 3 years ago

briochemc commented 3 years ago

This PR "extends" the types of labels from String to AbstractString, so that then one can use, e.g., a LaTeXString as a label. See this recent discourse post.

With that PR, the following MWE:

using Unitful, UnitfulRecipes, Plots, LaTeXStrings
pyplot() # this use of LaTeXStrings does not work with the GR backend for me
const a = 1u"m/s^2"
v(t) = a * t
x(t) = a/2 * t^2
t = (0:0.01:100)*u"s"
plot(x.(t), v.(t), xlabel=L"Position $\xi$", ylabel=L"Speed $\sigma$")

outputs

Screen Shot 2020-12-23 at 6 43 22 pm
codecov-io commented 3 years ago

Codecov Report

Merging #35 (c4228a5) into master (d2a11d8) will decrease coverage by 1.02%. The diff coverage is 86.20%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
- Coverage   84.14%   83.11%   -1.03%     
==========================================
  Files           1        1              
  Lines          82       77       -5     
==========================================
- Hits           69       64       -5     
  Misses         13       13              
Impacted Files Coverage Δ
src/UnitfulRecipes.jl 83.11% <86.20%> (-1.03%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 386112f...c4228a5. Read the comment docs.

briochemc commented 3 years ago

I'd like to add a test with LaTeXStrings, but this PR ends up turning labels into things like L"text $math$ (unit)" and the GR backend does not work well with these (yet?). Maybe this is worth an issue on Plots.jl?

jw3126 commented 3 years ago

Great feature!

I'd like to add a test with LaTeXStrings, but this PR ends up turning labels into things like L"text $math$ (unit)" and the GR backend does not work well with these (yet?). Maybe this is worth an issue on Plots.

Yeah, I think reporting this does not hurt. Things we could do on our side is use a different backend and maybe add a few unit tests, that just look at RecipesBase.apply_recipe.

briochemc commented 3 years ago

OK, I don't think these tests are too important either, and we can come back to it whenever (if ever?) Plots fixes the LaTeXStrings issue :)