Skip to content

Conversation

@briochemc
Copy link
Collaborator

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
Copy link

codecov-io commented Dec 23, 2020

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%) ⬇️

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 briochemc requested a review from jw3126 December 23, 2020 10:28
@briochemc
Copy link
Collaborator Author

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
Copy link
Owner

jw3126 commented Dec 23, 2020

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.

Copy link
Owner

@jw3126 jw3126 left a comment

Choose a reason for hiding this comment

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

Of course would be nice to have tests, but you can also merge as is if you want @briochemc

@briochemc briochemc merged commit b32dabd into jw3126:master Jan 6, 2021
@briochemc
Copy link
Collaborator Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants