JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.52k stars 5.46k forks source link

allow documentation strings containing (uninterpreted) $ #8963

Open stevengj opened 9 years ago

stevengj commented 9 years ago
julia> @doc """ computes $x+1$ """ -> foobar(x) = x+1
ERROR: syntax: invalid interpolation syntax: " "

I would strongly prefer that the Markdown parser left $ as-is, for embedding equations, showing examples involving interpolation, etcetera.

cc: @one-more-minute

MikeInnes commented 9 years ago

Code in doc strings should be quoted:

julia> @doc doc""" Example: `print("$x = y")` """ -> foobar(x) = x
foobar (generic function with 1 method)

help?> foobar
  Example: print("$x = y") 
stevengj commented 9 years ago

Yes, I noticed that and amended my example: quoting does not help with the case of embedded LaTeX equations (since those should not be quoted).

Although in your example it is still not printed correctly if the intention is to display print("$x = y") (i.e., an example involving interpolation).

Basically, I see the current (undocumented) interpolation in md strings to be more trouble than it is worth. We could just remove it and no one would complain.

MikeInnes commented 9 years ago

Yes, LaTeX support is a high priority of course. When I have a chance I'll extend Markdown.jl with a syntax for embedding LaTeX so that special characters don't cause problems within formulae (though note this is a general problem for e.g. * and [, not specific to the way $ is handled – to be clear, $ causes problems no more or less than they already do).

stevengj commented 9 years ago

Offtopic: Can we please use the same math syntax as Pandoc (hence Jupyter) etcetera? TeX math is recognized as anything between a pair of $ (or a pair of $$) where the first $ must have a non-space character immediately to its right and the closing $ must have a character immediately to its left. This heuristic seems to work extremely well in practice.

stevengj commented 9 years ago

Basically, what is the justification for the manual interpolation implementation of $? Where is this needed? As discussed in MichaelHatherly/Docile.jl#29, it's not even clear that it is occurring at the right time to be useful.

MikeInnes commented 9 years ago

Absolutely, I'm definitely on board with using $$ for LaTeX.

MikeInnes commented 9 years ago

I've mentioned my reasons for wanting interpolation many times before on those threads, but it boils down to:

  1. Embedding example results and graphics in documentation – I don't see any other sane way to do this, and it definitely seems useful to me.
  2. Avoiding us creating a complex custom flavour of markdown. Interpolating Julia objects means we can support e.g. references to other parts of the documentation as simply as $(ref(foo)), without complicating the syntax.

I'm acutely aware that you disagree and don't see the need, but as long as it's not causing problems I'd like to at least give it a shot and see how it goes.

stevengj commented 9 years ago

Regarding plots and graphics, the sane way to do this seems to me to store the plot in a file that is referenced from the doc. Generating the plot when the package is loaded will not only be slow, but also may add a dependency that should not exist (e.g. just because the plots are created by package FooPlot does not mean that I want my Bar package, at runtime, to depend on FooPlot).

For cross references, wouldn't it be preferable to use the Markdown syntax? i.e. [foo] links to the documentation for foo?

(Offtopic: Speaking of files referenced in the docs, it would be good to think about paths used to resolve embedded images etcetera. e.g. for modules, should it look into a Module/doc directory?)

stevengj commented 9 years ago

Normally, if there is doubt about a feature, the default choice in Julia has been to not implement it and wait for a clear need to arise. Adding a feature later is much less painful than adding it now (albeit undocumented) and removing it later.

cc: @StefanKarpinski

StefanKarpinski commented 9 years ago

I have to say that I agree with @stevengj here – embedded LaTeX a la Jupyter-flavored Markdown strikes me as the more important use case. Talking about dollars specifically is a pretty niche thing (even in financial packages, it's rare), whereas talking about math is kind of what we do around here. If we want to be able to make cross-references in docstrings, there are other ways we can do it.

stevengj commented 9 years ago

I think @one-more-minute's point is that for math we need a $...$ heuristic anyway to parse Markdown containing equations (because LaTeX equations may contain other things like * that Markdown would munge), and once this heuristic is implemented then it need not conflict with interpolation anymore.

But I still don't like the general idea of adding a new kind of interpolation, somewhat similar but not quite identical to ordinary Julia interpolation, in documentation strings. I'd rather err on the side of caution: leave this feature out for now, and later on decide whether we need something of this sort.

StefanKarpinski commented 9 years ago

Fair enough.

vtjnash commented 8 years ago

I found that this causes a problem for delayed execution of doc strings, which then in turn makes Markdown a hard dependency for Base. I realize this has all been concluded before (e.g. in https://github.com/MichaelHatherly/Docile.jl/issues/29#issuecomment-57196876, before it was closed with the reason given as "problem moved into Base"). I've started work on a branch to detangle the dependency of Base.Docs on Markdown, and this is one of the last remaining issues I've run into. Is there any significant opposition to me making a PR to remove the undocumented interpolation feature so that Markdown parsing and rendering can be handled entirely by the client instead of needing to happen at eval time?

stevengj commented 8 years ago

No objection from me; is anyone actually using this feature?

vtjnash commented 8 years ago

hard to say, and there will be an alternative function API that would still permit explicit usage of the feature

ScottPJones commented 8 years ago

Objections, none. Kudos, lots.

MichaelHatherly commented 8 years ago

Markdown a hard dependency for Base

+1 for removing the dep.

before it was closed with the reason given as "problem moved into Base"

Where's that quote from @vtjnash?

remove the undocumented interpolation feature

Would this just be removing interpolation support from @doc_str and not bare docstrings as well? If that's the case then no objection from me either.

vtjnash commented 8 years ago

I may have misinterpreted you, but I thought that seemed to be the gist of closure comment https://github.com/MichaelHatherly/Docile.jl/issues/29#issuecomment-70628937 (it was hard to tell for sure, since that comment is an incomplete thought).

Would this just be removing interpolation support from @doc_str and not bare docstrings as well? If that's the case then no objection from me either.

my branch removes interpolation from doc"" (keeping Markdown conversion) and disables markdown parsing for an interpolated docstring (keeping Markdown conversion for regular strings). it maintains the current behavior if the user explicitly writes md"".

MichaelHatherly commented 8 years ago

I may have misinterpreted you, but I thought that seemed to be the gist of closure comment MichaelHatherly/Docile.jl#29 (comment) (it was hard to tell for sure, since that comment is an incomplete thought).

My bad then :) Sorry for the confusion.

my branch removes interpolation from doc"" (keeping Markdown conversion) and disables markdown parsing for an interpolated docstring (keeping Markdown conversion for regular strings). it maintains the current behavior if the user explicitly writes md"".

Thanks. That sounds reasonable to me, :+1:.

ScottPJones commented 8 years ago

We actually use the interpolation in our doc strings, for things like:

module scott
       const mydefault = 123
       """This is my default $mydefault"""
       foo(var = mydefault) = var
end

This is another place where having Swift style interpolation syntax (i.e. \(expr) would make life easier, you could remove the $ style interpolation syntax but not lose the functionality.

ScottPJones commented 8 years ago

@vtjnash You're branch would break a large number of the docstrings we use for our projects at work. I don't think disabling Markdown parsing for an interpolated docstring would be good at all (although we'd be happy to switch our uses of interpolation (everywhere, not just docstrings) to use a Swift style syntax, if that were provided, and you simply removed $ interpolation syntax from both doc""" """ and bare docstrings).

MikeInnes commented 8 years ago

If you store the raw text and the module at load-time, then when you (lazily) parse it you can just evaluate any interpolated code then. That way interpolation is possible without the hard dependency on Markdown.jl.

Using interpolation is arguably better for things like listing values of constants and output so that docs can't get out of sync with code.