korsbo / Latexify.jl

Convert julia objects to LaTeX equations, arrays or other environments.
MIT License
544 stars 55 forks source link

Bugfix/abstract string #273

Closed a2k42 closed 9 months ago

a2k42 commented 9 months ago

Fixes https://github.com/korsbo/Latexify.jl/issues/272

a2k42 commented 9 months ago

I'm still a bit of a Julia noob so this was quite a naive fix, but it seems to work. Fingers crossed it doesn't break anything!

codecov[bot] commented 9 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/latexoperation.jl 85.21% <100.00%> (ø)
src/latexraw.jl 91.17% <100.00%> (ø)
src/numberformatters.jl 83.33% <100.00%> (ø)
src/unicode2latex.jl 96.21% <100.00%> (ø)
src/utils.jl 70.83% <100.00%> (ø)
src/mdtext.jl 0.00% <0.00%> (ø)

:loudspeaker: Thoughts on this report? Let us know!.

gustaphe commented 9 months ago

I think this is a bit overzealous, changing to AbstractString in some places where I can't imagine we would ever need one.

Could @latexrecipe f(x::AbstractString) = string(x) do the same job?

a2k42 commented 9 months ago

Using @latexrecipe f(x::AbstractString) = string(x) does also solve the String7 error, but I run into another error.

When I fixed the problem using AbstractString, the error message given was quite instructive:

in Latexify.jl: You are trying to create latex-maths from a String that cannot be parsed as an expression: Hello World.

latexify will, by default, try to parse any string inputs into expressions and this parsing has just failed.

If you are passing strings that you want returned verbatim as part of your input, try making them LaTeXStrings first.

If you are trying to make a table with plain text, try passing the keyword argument latex=false. You should also ensure that you have chosen an output environment that is capable of displaying not-maths objects. Try for example env=:table for a latex table or env=:mdtable for a markdown table.

For my particular usecase latexify(df, env=:mdtable, latex=false) turned out to be the right thing to do. Adding latex=false, as I've just discovered, works just fine without any fixes.

I suspect I was trying to use Latexify in a way it wasn't supposed to be used, but only when resolving the String error did the correct usage become apparent.

There might be a more minimal change I could make that would make the "correct" error appear, but I'd need to do some digging so I understand the package usage better. Will close for now.