Skip to content

Easier doctests of non-exported functions #2126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tecosaur opened this issue May 21, 2023 · 9 comments
Open

Easier doctests of non-exported functions #2126

tecosaur opened this issue May 21, 2023 · 9 comments

Comments

@tecosaur
Copy link

Hello,

I've recently started adding more doctests to unexported functions in my code, and I seem to be repeating the same boilerplate-y setup quite a bit, e.g.

"""
    parsechecksum(checksum::String)
    
...text...

```jldoctest; setup = :(import DataToolkitCommon.Store.parsechecksum)
julia> parsechecksum("crc32c:9c0188ee")
(:crc32c, 0x9c0188ee)
```
"""

Is there some way this boilerplate could be avoided?

@mortenpi
Copy link
Member

mortenpi commented May 21, 2023

You can set up shared setup code to all docstrings in a module: https://documenter.juliadocs.org/dev/man/doctests/#Module-level-metadata

But if this about actually dynamically figuring out what to import, then I don't think we really have any mechanism for that.

@tecosaur
Copy link
Author

Hmmm, I'm not quite sure how much of a help docmeta is. I suppose one could do DocMeta.setdocmeta!(MyPackage, :DocTestSetup, :(using MyPackage: thing1, things2, thing3, ..., thing12, thing13, ...); recursive=true) but you'd have to remember to keep that up to date since IIRC there's nothing like using MyPackage.*.

What I was hoping for was dynamic importing of just the function being tested.

I can think of two other ways this might be done as a setup step:

  1. Use Core.eval(MyPackage, :(doctest code...))
  2. Before running the doctests for MyPackage, use the result of names(MyPackage, all=true) to generate an import statement

@mortenpi
Copy link
Member

I don't think we'd want to eval into the package scope, since that will start polluting it, and you'd lose the property that all doctests are always evaluated in a clean environment. Just dynamically generating the global DocTestSetup from names seems like a reasonable workaround here though.

I would also argue that importing everything would be the more useful behavior anyway, as opposed to just importing a single function, since doctests often reference other functions from the same module as well, and not just the one the docstring is attached to. So the sort of "evaluate in the current module context (but still in a different module)" behavior seems good (and useful) to me. So maybe it could even be a built-in option here.

@tecosaur
Copy link
Author

Particularly considering multi-step doctests which introduce variable bindings, the Core.eval approach seems like a dodgy idea.

I'm glad to hear that "importing everything via names" sounds like a viable approach to you, I think this could strike a nice balance between convenience and robustness.

@tecosaur
Copy link
Author

I don't have the time to work on this ATM (or any time soon likely), but now it's been a few years I think it may be worth bumping this and stating that I still think this would be a worthwhile QoL improvement.

I've found myself using julia-repl over jldoctest for this reason in library code, because I just cannot be fussed to add a setup block each time.

@fingolfin
Copy link
Collaborator

But what is the plan here? Importing everything surely can't be default behavior. It'd be breaking as hell among other things, and it would also render jldoctest useless for me (because I want to ensure things work as in the REPL.)

All in all this seems like a valid but somewhat niche requirement (BTW we also use jldoctest to test internal functions but we just don't import them; this has the added bonus that I can run the tests manually by copy&pasting into the REPL. But of course I accept that this is not how you want to use it).

The way I understood @mortenpi is that if you want this, you can add a (global) @DocTestSetup which imports everything, like so

  for i in names(Module)  # possibly add `all=true`
    @eval import Module: $i
  end

Alas, that would not require any change to Documenter?

But it seems both of you have something else in mind, but what then? A flag for jldoctest to import everything?

@mortenpi
Copy link
Member

I guess the main ask here is to make it a bit easier to have somewhat complex setup code, but that needs to change slightly per doctest, but without having to write a whole complex jldoctest; setup = :(...) thing every time? So just one thought along those lines:

One pattern that I tend to use in more complex cases is to have my setup code in a reusable function in docs/make.jl:

function do_my_setup!() ... end

and then I call my doctests with

jldoctest; setup = :(Main.do_my_setup())

Potentially you could also pass arguments here, like the name of the function you're testing (though you'd have to keep that in sync automatically).

To help with that, one potential extension of this pattern could be something where you have a function that Documenter will pass some context to, that could identify which doctest it is etc:

function do_my_setup!(::DocumenterDocTestContext) ... end

I think the syntax for passing a function reference to setup is still free, because we currently expect that you pass an Expr generally (should be double checked though). So syntactically this could work if we allow you to pass function without :(...), i.e.:

jldoctest; setup = Main.do_my_setup!

@fingolfin
Copy link
Collaborator

Just to say: for Oscar, Nemo, AbstractAlgebra we use a similar method as that described by @mortenpi (I'll focus on Oscar in the examples): we have functions like Oscar.doctestsetup() which we use like this:

  1. DocMeta.setdocmeta!(Oscar, :DocTestSetup, Oscar.doctestsetup(); recursive = true) is called in our `docs/make.jl
  2. it is also called in CI jobs just before they call doctest(Oscar)
  3. in every .md file we have a @meta block which does DocTestSetup = Oscar.doctestsetup()

This way we have consistent setup for our doctests in all situations. I wish this was a bit more "automatic"... indeed simplifying 3 is the main motivation for my PR #2697 resp. issue #2512

Basically every package I've worked on uses doctests extensively and as a result I found myself implementing solutions to this again and again. I am wondering whether it would make sense to generalize this solution and add something to Documenter make it easier to achieve...

Like, doctest(MyModule) could do something like "if there is a symbol MyModule.DocTestSetup, assume it is a code block and use it as default value for DocTestSetup" (and then the same for DocTestTeardown.

Or slightly more general: "If there is a function MyModule.doctestsetup, invoke it for each jldoctest to get a default setup block; invoke it with parameters that indicate the context (like... the method or binding or whatever the docstring is attached; or for a doctest in a page, maybe the filename? or so)

The @meta blocks in our .md files also all set CurentModule = Oscar. I wonder if this then could also alleviate the need for DocMeta.setdocmeta! (in that case), by saying: if setdocmeta! was not called, but there is a CurentModule.doctestsetup(), then use that...

As to the recursive = true kwarg, one could use a logic like this:

function find_doctestsetup(mod::Module)
  isdefined(mod, :doctestsetup) && return mod.doctestsetup
  parentmodule(mod) === mod && return nothing
  return find_doctestsetup(parentmodule(mod))
end

(CC @lgoettgens @thofma )

@tecosaur
Copy link
Author

tecosaur commented May 6, 2025

Basically every package I've worked on uses doctests extensively and as a result I found myself implementing solutions to this again and again. I am wondering whether it would make sense to generalize this solution and add something to Documenter make it easier to achieve...

This sounds brilliant to me. I'm a big fan of taking common "and then you'll want to do X" steps and promoting them into part of what you get OOTB.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants