doctest and CitationBibliography about documentercitations.jl HOT 11 CLOSED

I can certainly make it a warning, but it might be worth looking a little deeper. At least in principle, I feel like doctest should be able to take plugins into account. For citations, it doesn't really matter, but I could imagine other plugins producing something that would affect testing.

@mortenpi Do you have any general comments on how doctests and plugins should interact?

from documentercitations.jl.

Looking at

https://github.com/JuliaDocs/Documenter.jl/blob/d9db9071d596cb9be81e742789952f2da2de8980/src/Documenter.jl#L942-L949

I feel like this should probably be considered a bug in doctest. It would be good if doctest could get a keyword argument plugins=[list_of_plugins...] and forward that to makedocs there.

from documentercitations.jl.

Opened JuliaDocs/Documenter.jl#2245

from documentercitations.jl.

Looks like we'll address this in Documenter in the next release.

I'm somewhat inclined to leave the @error (and close this issue). The problem is that just using CitationBibilography already inserts the citation logic into Documenter's pipeline, and that logic needs access to the plugin object. It's probably catchable in a way that's recoverable enough for doctest to run through, but at some level, any call to makedocs (which doctest calls) without the plugin object is pretty fundamentally broken.

How much of a blocker is this for you? Can you wait for Documenter 1.0 (and #3)? Would just changing @error to @warn be a sufficient solution? That is, can you dev DocumenterCitations and try it with ClimaAtmos?

from documentercitations.jl.

Thank, @goerz, so much for this!

The reason I was suggesting moving to warning the execution continued after the error (which is not what I would expect from errors). In my particular case, the error is indeed harmless because the information is not used in doctest. For this reason, this is not a big issue for us and we can certainly wait for Documenter 1.0.

Thanks again!

from documentercitations.jl.

Downstream fix: JuliaDocs/Documenter.jl#2249

from documentercitations.jl.

Having implemented JuliaDocs/Documenter.jl#2249 gave me some more detailed insight into how doctest works internally, and I might revise CitationBibliography further to avoid the error message in the context of a doctest run, even is someone calls doctest without the new plugins keyword.

However, just to clarify: Does it actually make a practical difference whether the warning is generated with @warn vs with @error? I think as far as makedocs/doctest goes, the behavior is exactly the same, no? Is there any difference with how a @warn/@error interacts with the strict keyword argument in makedocs (warnonly in Documenter 0.28/1.0)?

To get to the core of my question:

@Sbozzolo:

The reason I was suggesting moving to warning [is that] the execution continued after the error

That's a personal preference, though, right? – And I'm not sure I agree. I've been using the following mental model:

• error(msg): Fatal error, execution cannot continue
• @warn msg: There's something that needs the user's attention (but it might be ok)
• @error msg: There's something wrong. We can keep running, but the user is definitely not getting the result they're expecting.

So in some sense @error is just a @warn with more emphasis ("you can't just ignore this"), but not necessarily a fatal error. In DocumenterCitations, I've been considering, e.g., missing citation references (whether due to forgetting to specify a .bib file or due to citing a non-existing keys) as errors, while, e.g., malformed bibtex entries are warnings.

One reason I often use non-fatal @error messages: It can be useful to get multiple errors (e.g., multiple missing references), so that the user can fix all of the underlying issues before rerunning the program. With fatal errors, you often get a tedius "run, error, fix" loop.

It would be interesting to know if there's a general consensus in the community on whether @error messages are expected to stop execution.

from documentercitations.jl.

See https://discourse.julialang.org/t/semantics-of-error/103760 for the philosophical discussion

from documentercitations.jl.

Having implemented JuliaDocs/Documenter.jl#2249 gave me some more detailed insight into how doctest works internally, and I might revise CitationBibliography further to avoid the error message in the context of a doctest run, even is someone calls doctest without the new plugins keyword.

However, just to clarify: Does it actually make a practical difference whether the warning is generated with @warn vs with @error? I think as far as makedocs/doctest goes, the behavior is exactly the same, no? Is there any difference with how a @warn/@error interacts with the strict keyword argument in makedocs (warnonly in Documenter 0.28/1.0)?

To get to the core of my question:

@Sbozzolo:

The reason I was suggesting moving to warning [is that] the execution continued after the error

That's a personal preference, though, right? – And I'm not sure I agree. I've been using the following mental model:

• error(msg): Fatal error, execution cannot continue
• @warn msg: There's something that needs the user's attention (but it might be ok)
• @error msg: There's something wrong. We can keep running, but the user is definitely not getting the result they're expecting.

So in some sense @error is just a @warn with more emphasis ("you can't just ignore this"), but not necessarily a fatal error. In DocumenterCitations, I've been considering, e.g., missing citation references (whether due to forgetting to specify a .bib file or due to citing a non-existing keys) as errors, while, e.g., malformed bibtex entries are warnings.

One reason I often use non-fatal @error messages: It can be useful to get multiple errors (e.g., multiple missing references), so that the user can fix all of the underlying issues before rerunning the program. With fatal errors, you often get a tedius "run, error, fix" loop.

It would be interesting to know if there's a general consensus in the community on whether @error messages are expected to stop execution.

Thanks for sharing your thoughts about this. I find your mental model quite useful. I didn't think about the option of "error as a log message", and I now see the rationale behind it.

from documentercitations.jl.

Alright, I think I'll keep it an @error then, but I'll look into mitigating this in some other way

from documentercitations.jl.

The correct solution is to just skip all the citation pipeline steps when running in doctest mode by calling, e.g.,

Documenter.is_doctest_only(doc, "CollectCitations") && return

in Selectors.runner(::Type{CollectCitations}, doc).

I'll make sure to do that in a future release.

from documentercitations.jl.