Move UnitfulLatexify into Unitful extension

[Ickaser]

This should supercede #668, including with the discussion that took place there.

A couple notes for posterity:

• In Add Latexify extension #668, there is a dispatch for ranges of quantities that would latexify with \qtyrange. I dropped that dispatch here, since Add range operator korsbo/Latexify.jl#333 turns ranges into vectors now.
• UnitfulLatexify had a format check in the tests with JuliaFormatter, but I am letting Unitful's own CI setup take its place.

[Ickaser]

Once this is merged and released, gustaphe/UnitfulLatexify.jl#21 should also get merged and released.

[Ickaser]

CI failure on Julia 1.0 is because Latexify v.0.15.16 (current version: 0.16.8, needed for extension) upped compat bound to Julia 1.6, and for testing, Latexify needs to be loaded. I suspect that pre-1.6 users who don't need Latexify might still be able to install and use Unitful, although I don't know for sure.

I'm not sure what the value is of testing as far back as Julia 1.0, at this point--even 1.6 is no longer considered LTS.

[Ickaser]

(I guess @gustaphe may not see this unless I ping him directly.)

[gustaphe]

Fantastic!

I will try this out at some point during the day, but I have one thought already: I think the range thing is still relevant. Ranges are only expanded when the flag is set, and then only for evaluated ranges. If you use siunitx you want the siunitx range command.

[gustaphe]

Okay, that doesn't currently work anyway, I'll work on a PR for it once this is merged. I must have forgotten to write tests for it.

The documentation still refers to qtyrange, that should probably be looked over.

Other than that LGTM

[Ickaser]

Alright, with that doc fix then I think I am happy with this @sostock . Do you have any other questions?

[Ickaser]

@sostock any objections to this PR? Or things you would like clarified?

[codecov]

Codecov Report

❌ Patch coverage is 80.30303% with 26 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@c1a9b4e). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
ext/LatexifyExt.jl	80.30%	26 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #795   +/-   ##
=========================================
  Coverage          ?   89.51%           
=========================================
  Files             ?       21           
  Lines             ?     1698           
  Branches          ?        0           
=========================================
  Hits              ?     1520           
  Misses            ?      178           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Ickaser]

Is there anyone who feels qualified to comment on this? This is holding up JuliaPlots/Plots.jl#5143, which will make it so that the Plots.jl extension for Unitful can fully drop UnitfulLatexify as a dependency and make using Plots, Unitful load correctly in the gradually-approaching Plots.jl 2.0 (JuliaPlots/Plots.jl#5093 ).

[sostock]

When I build the docs locally, some things don’t look right. But that may just be a problem with my setup?

• In the “Arrays” section, the latex code is not rendered:
  

• The long table has no header:
  

[Ickaser]

I hadn't thought to build the docs locally🫣 but doing so now did allow me to catch and fix a couple other details in the docs.

When I build the docs locally, some things don’t look right. But that may just be a problem with my setup?

* In the “Arrays” section, the latex code is not rendered:       

This was happening for me too, because the generated LaTeX was delimited with \begin{equation}...\end{equation} rather than $$..$$. So for the docs' I wrapped it in dollar signs, and now it renders.

* The long table has no header:

This is a font setup thing, I had to pick a font to install for the CI (since none were there by default) and picked one that probably isn't on your machine. It works correctly on my machine, and I believe it works on CI (no missing-font errors in the CI logs).

Because these docs load Plots, which at present still has UnitfulLatexify as a dependency, the plots in the docs won't render quite correctly until that dependency gets dropped from Plots. But since that circular dependency is only at the docs level, I am inclined to say it would be better to move ahead here and perhaps plan to re-deploy the docs in CI after a new release from Plots has the required changes.

(And although there are almost certainly exist possible improvements, I am satisfied with the code as it stands.)

[sostock]

I just have some comments regarding spacing. I think \, is more commonly used than \;, but we don’t have to change that right now. In my opinion, if the spacing in the docs is changed so that it is consistent with what is generated by the code, this is ready to merge.

[giordano]

Adding the package itself to the docs environment without something like 7682716 (from #794) is generally incorrect because you get the docs of the wrong version.

[Ickaser]

Valid. Rookie mistake on my part, I wasn't sure how to do this in a more robust way--thanks for pointing to an example.

[giordano]

Also, it'd be always good to have compat bounds at least for non-stdlibs non-jll packages.

[giordano]

For the record, this completely broke the possibility of building the docs on macOS, because exponents are rendered differently than all other systems (which is an issue on its own, but at least doesn't normally fully prevent building the docs)

[sostock]

Should we revert it for now?

We could let the CI build the docs on all three major systems (but only deploy it on one) to catch something like this in the future, but I’m not sure it’s worth it.

[giordano]

I'm a bit contrived. It's annoying this is running with a somewhat unrelated issue (platform-specific rendering of the units). I don't remember, is there a way to render the powers the same across all platforms, which would be used for generating the docs?

[sostock]

Yes, there is the environment variable UNITFUL_FANCY_EXPONENTS:

Unitful.jl/src/display.jl

Line 230 in 3a7ceb5

v = get(ENV, "UNITFUL_FANCY_EXPONENTS", Sys.isapple() ? "true" : "false")

[giordano]

@Ickaser

julia> ENV["UNITFUL_FANCY_EXPONENTS"] = "false"
"false"

julia> include("make.jl")
[ Info: Generating latex images for documentation
[ Info: Precompiling TectonicExt [35afe39f-415e-5b39-968f-951aa6823229]
┌ Info: Skipping precompilation due to precompilable error. Importing TectonicExt [35afe39f-415e-5b39-968f-951aa6823229].
└   exception = Error when precompiling module, potentially caused by a __precompile__(false) declaration in the module.
ERROR: LoadError: cannot latexify objects of type Unitful.FreeUnits{(kg, m, s^-2), 𝐋 𝐌 𝐓^-2, nothing}
Stacktrace:
  [1] _latexraw(args::Unitful.FreeUnits{(kg, m, s^-2), 𝐋 𝐌 𝐓^-2, nothing}; kwargs::@Kwargs{env::Symbol})
    @ Latexify ~/.julia/packages/Latexify/TbZGe/src/latexraw.jl:110
  [2] process_latexify(args::Unitful.FreeUnits{(kg, m, s^-2), 𝐋 𝐌 𝐓^-2, nothing}; kwargs::@Kwargs{env::Symbol})
    @ Latexify ~/.julia/packages/Latexify/TbZGe/src/latexify_function.jl:49
  [3] process_latexify
    @ ~/.julia/packages/Latexify/TbZGe/src/latexify_function.jl:40 [inlined]
  [4] _latexinline(x::Unitful.FreeUnits{(kg, m, s^-2), 𝐋 𝐌 𝐓^-2, nothing}; kwargs::@Kwargs{})
    @ Latexify ~/.julia/packages/Latexify/TbZGe/src/latexinline.jl:4
  [5] _latexinline
    @ ~/.julia/packages/Latexify/TbZGe/src/latexinline.jl:3 [inlined]
  [6] #process_latexify#166
    @ ~/.julia/packages/Latexify/TbZGe/src/latexify_function.jl:49 [inlined]
  [7] process_latexify
    @ ~/.julia/packages/Latexify/TbZGe/src/latexify_function.jl:40 [inlined]
  [8] latexify(args::Unitful.FreeUnits{(kg, m, s^-2), 𝐋 𝐌 𝐓^-2, nothing}; kwargs::@Kwargs{})
    @ Latexify ~/.julia/packages/Latexify/TbZGe/src/latexify_function.jl:27
  [9] latexify(args::Unitful.FreeUnits{(kg, m, s^-2), 𝐋 𝐌 𝐓^-2, nothing})
    @ Latexify ~/.julia/packages/Latexify/TbZGe/src/latexify_function.jl:25
 [10] top-level scope
    @ none:1
 [11] eval
    @ ./boot.jl:430 [inlined]
 [12] eval
    @ ./sysimg.jl:48 [inlined]
 [13] (::var"#1#2")(command::Expr)
    @ Main ~/.julia/dev/Unitful/docs/generate_latex_images.jl:13
 [14] iterate
    @ ./generator.jl:48 [inlined]
 [15] collect_to!
    @ ./array.jl:849 [inlined]
 [16] collect_to_with_first!
    @ ./array.jl:827 [inlined]
 [17] _collect(c::Vector{Expr}, itr::Base.Generator{Vector{Expr}, var"#1#2"}, ::Base.EltypeUnknown, isz::Base.HasShape{1})
    @ Base ./array.jl:821
 [18] collect_similar(cont::Vector{Expr}, itr::Base.Generator{Vector{Expr}, var"#1#2"})
    @ Base ./array.jl:720
 [19] map(f::Function, A::Vector{Expr})
    @ Base ./abstractarray.jl:3371
 [20] top-level scope
    @ ~/.julia/dev/Unitful/docs/generate_latex_images.jl:12
 [21] include(fname::String)
    @ Main ./sysimg.jl:38
 [22] top-level scope
    @ ~/.julia/dev/Unitful/docs/make.jl:4
 [23] include(fname::String)
    @ Main ./sysimg.jl:38
 [24] top-level scope
    @ REPL[2]:1
in expression starting at /Users/mose/.julia/dev/Unitful/docs/generate_latex_images.jl:12
in expression starting at /Users/mose/.julia/dev/Unitful/docs/make.jl:4

what am I doing wrong?

[sostock]

I don’t know what happens there, but to me the error doesn’t suggest that it has anything to do with the exponent rendering.

[giordano]

Ok, so this is even worse 😂

[sostock]

Are you building the docs with a different Unitful version (which would happen as you noted in #795 (comment))? I get that same error if I try to build the docs from this PR with the released Unitful (v1.24.0), since that doesn’t have the Latexify extension.

[giordano]

Nuking the manifest and re-resolving the environment on master, I get a different error (which is probably progress):

julia> include("make.jl")
Precompiling Documenter...
  1 dependency successfully precompiled in 15 seconds. 52 already precompiled.
[ Info: Generating latex images for documentation
[ Info: Precompiling TectonicExt [35afe39f-415e-5b39-968f-951aa6823229]
┌ Info: Skipping precompilation due to precompilable error. Importing TectonicExt [35afe39f-415e-5b39-968f-951aa6823229].
└   exception = Error when precompiling module, potentially caused by a __precompile__(false) declaration in the module.
warning: accessing absolute path `/dev/null`; build may not be reproducible in other environments
ERROR: LoadError: IOError: could not spawn `gs -sDEVICE=pngalpha -dTextAlphaBits=4 -r300 -o /Users/mose/.julia/dev/Unitful/docs/src/assets/latex-examples.png /var/folders/v2/hmy3kzgj4tb3xsy8qkltxd0r0000gn/T/jl_7ehMJT.pdf`: no such file or directory (ENOENT)

This should really be using Ghostscript_jll....

[Ickaser]

Shoot, sorry @giordano . I will freely admit that I did not anticipate people needing to build docs on another OS--in the CI I separately installed ghostscript (and also a small Debian font package) in order to get the docs working.

Nuking the manifest and re-resolving the environment on master, I get a different error (which is probably progress):

julia> include("make.jl")
Precompiling Documenter...
  1 dependency successfully precompiled in 15 seconds. 52 already precompiled.
[ Info: Generating latex images for documentation
[ Info: Precompiling TectonicExt [35afe39f-415e-5b39-968f-951aa6823229]
┌ Info: Skipping precompilation due to precompilable error. Importing TectonicExt [35afe39f-415e-5b39-968f-951aa6823229].
└   exception = Error when precompiling module, potentially caused by a __precompile__(false) declaration in the module.
warning: accessing absolute path `/dev/null`; build may not be reproducible in other environments
ERROR: LoadError: IOError: could not spawn `gs -sDEVICE=pngalpha -dTextAlphaBits=4 -r300 -o /Users/mose/.julia/dev/Unitful/docs/src/assets/latex-examples.png /var/folders/v2/hmy3kzgj4tb3xsy8qkltxd0r0000gn/T/jl_7ehMJT.pdf`: no such file or directory (ENOENT)

This should really be using Ghostscript_jll....

This error looks like what you get if ghostscript isn't installed, which is why in the CI I had to manually install Ghostscript. I had no idea there was such a thing as Ghostscript_jll, that should probably have an extension for Latexify (where right now ghostscript is documented as a dependency of Latexify.render for .png output, but not installed). I'll try to address that with Latexify, and make another PR to Unitful with the sources in docs/Project.toml fix.

[Ickaser]

#808 should make the docs build easier to work with once korsbo/Latexify.jl#344 merges. I apologize for the confusion about that, and thanks @giordano, @sostock , @gustaphe for following me along here!