Latexify.jl icon indicating copy to clipboard operation
Latexify.jl copied to clipboard

simplify & rearrange `unicodedict`

Open t-bltg opened this issue 1 year ago • 14 comments

  • rework unicodedict in a consistent unicode ordered way (add OrderedCollections dependency);
  • check that we can compile all unicode replacements of unicodedict in test/unicode2latex.jl;
  • non-default LaTeX symbols (significant amount of https://docs.julialang.org/en/v1/manual/unicode-input) are handled using the unicode-math package;
  • propagate kwargs to _writetex (simplifies signatures);
  • add preamble keyword for custom additions;
  • update workflows (declaring 1 compatibility in Project.toml should test 1.0).

This PR saves about ~800 lines of code for generating unicodedict, and makes this dict readable & extandable (ordered by following increasing Unicode codes).

Here is the pdf of all the generated replacements compiled using STIX2 and JuliaMono fonts: all sequences are correctly found: unicodedict.pdf.

~~EDIT: It seems CI fails on 1.0 because of dependencies, should we raise the minimal julia version to 1.6 LTS ?~~ ==> dropped julia < 1.6

t-bltg avatar Aug 09 '22 19:08 t-bltg

Codecov Report

Merging #225 (635eb36) into master (03b3dfa) will increase coverage by 0.18%. The diff coverage is 53.84%.

@@            Coverage Diff             @@
##           master     #225      +/-   ##
==========================================
+ Coverage   63.18%   63.36%   +0.18%     
==========================================
  Files          23       23              
  Lines         861      939      +78     
==========================================
+ Hits          544      595      +51     
- Misses        317      344      +27     
Impacted Files Coverage Δ
src/Latexify.jl 46.15% <ø> (ø)
src/unicode2latex.jl 32.82% <ø> (-36.67%) :arrow_down:
src/utils.jl 70.10% <52.63%> (+44.82%) :arrow_up:
src/latexoperation.jl 84.89% <100.00%> (ø)
src/plugins/SymEngine.jl 80.00% <0.00%> (+80.00%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 09 '22 19:08 codecov[bot]

This looks very nice!

We've discussed removing the 1.0 testing before, it might very well be time.

While \text is more idiomatic, it needs to be \textrm to work with (at least) GR's renderer (Maybe also mathjax, but I haven't confirmed that). At least for me, that's a very large portion of my latexify use, so if that is to change it needs to be a keyword setting.

Could you also mark this as a draft or stop force pushing, in case there are any other per-line comments?

gustaphe avatar Aug 10 '22 07:08 gustaphe

Looks good!

EDIT: It seems CI fails on 1.0 because of dependencies, should we raise the minimal julia version to 1.6 LTS ?

We've discussed removing the 1.0 testing before, it might very well be time.

👍 Just bump the Julia compat in Project.toml too.

needs to be \textrm to work with (at least) GR's renderer (Maybe also mathjax, but I haven't confirmed that).

Looks like it might be the other way around for mathjax: https://docs.mathjax.org/en/latest/input/tex/differences.html#tex-differences

Do you know what GR uses to render LaTeX?

korsbo avatar Aug 10 '22 08:08 korsbo

We've discussed removing the 1.0 testing before, it might very well be time.

Just bump the Julia compat in Project.toml too.

Dropped 1.0.

Could you also mark this as a draft or stop force pushing, in case there are any other per-line comments?

:+1:

While \text is more idiomatic, it needs to be \textrm to work with (at least) GR's renderer Looks like it might be the other way around for mathjax:

Thanks, I'll have a look.

t-bltg avatar Aug 10 '22 08:08 t-bltg

While \text is more idiomatic, it needs to be \textrm to work with (at least) GR's renderer

@gustaphe, could you provide an example where \text{} fails with GR ?

Do you know what GR uses to render LaTeX?

Previously, they used gr_mathtex https://github.com/sciapp/gr/commit/0ec121fd5c7b1303ac0fb74665405a79f04ddc5a#diff-a5cedece354edb404e814367c117eb78f205cf83db23468f9e1bc310dc1fafffR5023, but is seems to have been removed in favor of mathtex2.

mathtex2 translates latex escape sequences into unicode codepoints: https://github.com/sciapp/gr/blob/master/lib/gr/mathtex2.c, thus bypassing the latex compiler.

t-bltg avatar Aug 10 '22 08:08 t-bltg

julia> Pkg.status("Plots")
Status `/tmp/jl_MksNwK/Project.toml`
  [91a5bcdd] Plots v1.31.6
julia> plot(randn(10); xguide=raw"$\textrm{\`a textrm}$", yguide=raw"$\text{\`a text}$")

textrm

Maybe this is a problem best solved on the GR side then, both \text and \textrm are legal in LaTeX. Both of them work fine on Mathjax's demo page. Could we maybe hold off on that change until it's solved in GR - or make it a kwarg?

gustaphe avatar Aug 10 '22 11:08 gustaphe

Thanks for the example.

both \text and \textrm are legal in LaTeX

Let's not forget that \textrm{} is a LaTeX command, but \text{} comes from amsmath (which we add in the preamble).

Could we maybe hold off on that change until it's solved in GR - or make it a kwarg?

It would be easy to add a kwarg.

t-bltg avatar Aug 10 '22 12:08 t-bltg

Ah, I thought I tested it without amsmath, but I had siunitx loaded in my test doc. That means GR is actually doing it right (apart from swallowing the space).

gustaphe avatar Aug 10 '22 12:08 gustaphe

Anyway, the Plots example using the gr backend does NOT use Latexify at all. Maybe there is an impact when using pfgplotsx backend ... I'll run Plots test suite.

t-bltg avatar Aug 10 '22 12:08 t-bltg

No, but it demonstrates the difference between the support for text and textrm clearly. I could produce the same result by loading this PR and running

plot(randn(10); yguide=latexify("à"))

gustaphe avatar Aug 10 '22 12:08 gustaphe

A better alternative is \textnormal (in the latex kernel), as explained here.

GR only supports these font commands , so I'll open a PR there (EDIT: https://github.com/sciapp/gr/pull/159).

A keyword arg would be difficult to implement since we generate unicodedict at precompile time, and we don't want to impact the package load time.

t-bltg avatar Aug 10 '22 15:08 t-bltg

I've tested this PR on MathJax and it seems fine (using the code outputted using MIME("juliavscode/html")).

t-bltg avatar Aug 10 '22 19:08 t-bltg

The sequence tex -> div -> png is pretty bad, because dvi does not have OTF (OpenType Font) support (see e.g. https://tex.stackexchange.com/q/330676/75098), or the warnings in https://github.com/korsbo/Latexify.jl/pull/224#issue-1321497720. I propose to replace it with the sequence tex -> pdf -> png, only if convert (from imagemagick - pretty common on unix or even windows under the name imgconvert) is found (takes the same amount of time to generate the png image), falling back to dvi if needed.

We could also use \usepackage[convert]{standalone} to generate the png on the fly instead, but this requires the -shell-escape latex flag.

I also propose to replace varwidth=100cm by varwidth=true (load the package varwidth in order to wrap equations in a varwidth environment) and let latex handle that.

Also disable wrapping mainscript in \mathrm{} if function name is non-ascii (e.g. ∂f(t)), which can cause font failures:

render(latexify(:(iħ * (∂Ψ(𝐫, t) / ∂t) = -ħ^2 / 2m * ΔΨ(𝐫, t) + V * Ψ(𝐫, t))), MIME("image/png"))

t-bltg avatar Aug 10 '22 22:08 t-bltg

~~I'm not sure to where to go from there. I've added svg, pdf and png tests. Unfortunately, rendering to dvi seems to be failing because of unicode-math. We seem to hit https://tex.stackexchange.com/q/386072, although https://tex.stackexchange.com/q/537281 claims that it is possible to use OTF fonts for dvi since 2020 (I'm using a recent texlive distribution: TeX Live 2022).~~

This is now fixed by using dvisvgm --pdf and use tex -> pdf -> svg instead of tex -> dvi -> svg.

t-bltg avatar Aug 11 '22 16:08 t-bltg

I think that this PR is ready for review / comments.

t-bltg avatar Aug 27 '22 07:08 t-bltg