JuliaSymbolics / SymbolicUtils.jl

Symbolic expressions, rewriting and simplification
https://docs.sciml.ai/SymbolicUtils/stable/
Other
545 stars 114 forks source link

Add a doctest #333

Closed rikhuijzer closed 3 years ago

rikhuijzer commented 3 years ago

This is just a suggestion. In the Julia library, Documenter.jl is used to verify all the documentation via doctests. This means that Documenter's doctest logic should be quite robust. Since this package has a lot of examples in the docstrings, maybe it is a good idea to run doctests to avoid broken documentation.

EDIT: For example, other packages which also use doctests are DataFrames.jl and MCMCChains.jl.

shashi commented 3 years ago

Nice!

How does jldoctest get rendered with Franklin? btw The SymbolicUtils documentation is generated with Franklin... We may have to hack https://github.com/JuliaSymbolics/SymbolicUtils.jl/blob/master/page/utils.jl#L5-L26 if it doesn't already render the code block properly.

rikhuijzer commented 3 years ago

How does jldoctest get rendered with Franklin?

I couldn't run the SymbolicUtils Page build locally due to some broken dependency (I'm running NixOS where I get those issues from time to time). Therefore, I've tested it in Franklin.jl and there the output is indeed incorrect for jldoctest code blocks.

The workaround is added in d3423d2, but note that I didn't test the workaround.

codecov-commenter commented 3 years ago

Codecov Report

Merging #333 (de423d2) into master (aee8482) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #333   +/-   ##
=======================================
  Coverage   83.89%   83.89%           
=======================================
  Files          12       12           
  Lines        1217     1217           
=======================================
  Hits         1021     1021           
  Misses        196      196           
Impacted Files Coverage Δ
src/utils.jl 60.37% <ø> (ø)

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 aee8482...de423d2. Read the comment docs.

shashi commented 3 years ago

The workaround is added in d3423d2

Looks good enough.

shashi commented 3 years ago

Thanks! This is great!