Refactor Latexify.jl

[korsbo]

Completely change the logic behind latexraw. And update the rest accordingly.

This has a large impact on extensibility and it will break many recipes. However, it will enable far more powerful extensibility.

[codecov]

Codecov Report

Base: 60.21% // Head: 58.77% // Decreases project coverage by -1.44% ⚠️

Coverage data is based on head (1d86347) compared to base (9416101).
Patch coverage: 92.36% of modified lines in pull request are covered.

❗ Current head 1d86347 differs from pull request most recent head 29a6711. Consider uploading reports for the commit 29a6711 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #161      +/-   ##
==========================================
- Coverage   60.21%   58.77%   -1.45%     
==========================================
  Files          23       24       +1     
  Lines         734      798      +64     
==========================================
+ Hits          442      469      +27     
- Misses        292      329      +37     

Impacted Files	Coverage Δ
src/utils.jl	25.51% <50.00%> (+4.33%)	⬆️
src/latexarray.jl	88.46% <81.81%> (-7.00%)	⬇️
src/Latexify.jl	42.85% <83.33%> (-3.30%)	⬇️
src/latexraw.jl	83.33% <83.33%> (-3.34%)	⬇️
src/latexraw_recipes.jl	97.02% <97.02%> (ø)
src/config.jl	100.00% <100.00%> (ø)
src/latexalign.jl	78.94% <100.00%> (ø)
src/latexify_function.jl	71.42% <100.00%> (+3.42%)	⬆️
src/recipes.jl	0.00% <0.00%> (-73.81%)	⬇️
... and 1 more

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[gustaphe]

Nice! I look forward to trying this out.

Just a quick thing: descend? (Anglofoner och deras s-ljud...)

[korsbo]

An example of how external packages might do extensions.

module MyTest

using Latexify
struct MyType
    args1
    args2
end

my_instructions = [
    function mymultiplication(io, ex, prevop, config)
    ## Recipe overrides everything and pirates evilly 
    if operation(ex) == :* 
        decend(io, arguments(ex)[1], operation(ex))
        write(io, " EvilMult! ")
        decend(io, arguments(ex)[2], operation(ex))
        return true
    end
    end,
    function mydivision(io, ex, prevop, config)
    ## Recipe only applied when called below a `MyType`
    if operation(ex) == :/ && getconfig(:MyType, false) 
        decend(io, arguments(ex)[1], operation(ex))
        write(io, " \\div ")
        decend(io, arguments(ex)[2], operation(ex))
        return true
    end
    end,
    function mytype(io, ex, prevop, config)
    if ex isa MyType
        setconfig!(:MyType => true)
        decend(io, Latexify.Equation(ex.args1))
        return true
    end
    end,
]

append!(Latexify.USER_INSTRUCTIONS, my_instructions)
end

using Latexify
t = MyTest.MyType(:(1 + x/y * x), :(x))
## Both multiplication and division are non-default.
latexify(t)

## Multiplication has been affected but not division!
latexify(Latexify.Inline(:(a+x/y+b*x)))

It's not ready yet but it just shows what one might be capable of. One could of course still just set new default configs and such without having to redefine entire operations but the world is your oyster here in a way that it never was before.

Maybe of interest to you @gustaphe

Input is always welcomed!

[Edit: just a note that this only works in my local branch which has not yet been merged into this one.]

[gustaphe]

I have some questions:

getconfig(:MyType, false) and setconfig!(:MyType => true) are supposed to be get(config, :MyType, false) and push!(config, :MyType, true), right? Or is there a global config, and if so when is :MyType reset above?

de(s)cend calls every function in Latexify.USER_INSTRUCTIONS until one of them returns true? Is there a difference between returning nothing and false? (if not, false is a good default because it makes the function type-stable)

For a simple wrapper:

struct Wrapper{T}
    x::T
end

would the main way to do it be

push!(Latexify.USER_INSTRUCTIONS, 
    function wrapper(io, ex, prevop, config)
        ex isa Wrapper || return false
        write(io, "\\wrapper{")
        descend(io, ex.x)
        write(io, "}")
        return true
    end
)

or to overload decend?

Can decend call applicable(f, (io, ex, prevop, config)) && f(io, ex, prevop, config) on each f in USER_INSTRUCTIONS (until true), so you can use a type annotation instead of the first line of wrapper()?

[korsbo]

getconfig(:MyType, false) and setconfig!(:MyType => true) are supposed to be get(config, :MyType, false) and push!(config, :MyType, true), right? Or is there a global config, and if so when is :MyType reset above?

Currently there is a global config that keeps these ones. I first thought I might be able to not pass config along in each descend but now I'm sort-of thinking that it may be necessary to pass config after all. Having a global const could mean that it is execution order and not expression-tree-lineage that determines your configs.

Currently, then global config is reset every top-level call.

[korsbo]

de(s)cend calls every function in Latexify.USER_INSTRUCTIONS until one of them returns true? Is there a difference between returning nothing and false? (if not, false is a good default because it makes the function type-stable)

I used to use a String or nothing. If the return type is nothing then I keep searching along the list of functions. Now that I switched to IOBuffer this has become less elegant since I no longer have to explicitly return the string but instead mutate the IO. Right now, I just need to return something but I should indeed make this type stable.

[korsbo]

For a simple wrapper:
struct Wrapper{T}
x::T
end
would the main way to do it be
push!(Latexify.USER_INSTRUCTIONS,
function wrapper(io, ex, prevop, config)
ex isa Wrapper || return false
write(io, "\wrapper{")
descend(io, ex.x)
write(io, "}")
return true
end
)
or to overload decend?

pretty much this. One should not overload descend. `descend will iterate the list of functions available so it will pick up anything you push to the list.

[korsbo]

Can decend call applicable(f, (io, ex, prevop, config)) && f(io, ex, prevop, config) on each f in USER_INSTRUCTIONS (until true), so you can use a type annotation instead of the first line of wrapper()?

Technically possible but not what I want. A conditional can check values and not just types so it'll be more powerful. Trying to place all possible information in type annotations might make things both ugly and slow to compile (overspecialisation galore).

Edit: Also, I don't care at all about runtime right now - it's already fast enough. So I don't want to offload too much of the branching to the compiler. Specialisation is great in most applications but it's not something I want here since there is already an imbalance towards slow compilation but fast runtime.

[korsbo]

One might still make some syntactic sugar for this though. I think it's good if there is a full-featured but maybe a bit complex function API for this, but we could have some convenience function/macro to hide many of the details for the simplest and most common use-cases.

[gustaphe]

Can decend call applicable(f, (io, ex, prevop, config)) && f(io, ex, prevop, config) on each f in USER_INSTRUCTIONS (until true), so you can use a type annotation instead of the first line of wrapper()?

Technically possible but not what I want. A conditional can check values and not just types so it'll be more powerful. Trying to place all possible information in type annotations might make things both ugly and slow to compile (overspecialisation galore).

Yeah, I see why you're not only checking applicability, but I would imagine many recipes would become simpler if it was part of the chain. But yes, @latexrecipe f(x::MyType) = x.x could be translated into

push!(Latexify.USER_INSTRUCTION, 
    function f(io, x, prevop, config)
        x isa MyType || return false
        descend(io, x.x)
        return true
    end
)

and the user won't need to know that they are not actually using multiple dispatch...

[korsbo]

Maybe we should think about splitting Latexify.jl into a few separate packages. With this refactor, we could have the default list of conversion functions in one package, the basic plumbing and utilities in another, rendering in a third, etc. Latexify.jl could then be a meta-package that just pulls in all the parts.

I wonder if this might help deal with breaking releases. It has always been a bit problematic that even minute changes to the output formatting breaks tests. Separating formatting from the more high-level functionalities might provide a way of letting formatting be included in SemVer without having to bump the major version every other PR.

I have not really thought this through. I'm mostly writing so that I don't forget until I can actually take some time to do this.

[korsbo]

I have not really thought this through. I'm mostly writing so that I don't forget until I can actually take some time to do this.

Thanks past me, that was helpful 😄

[korsbo]

The core is almost there now.

• latexify expressions
• handle LaTeX environments - Align, Array, Table, Inline, etc. based on input type and env specification.
• handle Markdown environments
• Sort out the interface for external packages to extend the list of matching functions.

This PR is massively breaking for latexrecipes so we'll need to either bridge the gap by letting the old machinery translate into the new or we need to PR/add support for the most critical parts of the ecosystem. Support should be on par with what we already have right now (a low bar for DataFrames, for example).

• DataFrames.jl
• Symbolics.jl
• ModelingToolkit.jl (?)

Other items

• Purge old dependencies
• Purge old code
• Split package into Latexify, LatexifyUnicode, LatexifyMatchers, LatexRender (separate PR)
• Test latency and runtime performance.
• Document!

The most major pieces of work left is figuring out the interface external packages to extend latexify and to get Symbolics.jl up to snuff.