Skip to content
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

DOC prefix docstring by method signature #9838

Closed
wants to merge 1 commit into from
Closed

Conversation

hayd
Copy link
Member

@hayd hayd commented Jan 19, 2015

This prefixes the help text with method name:

julia> @doc "hello" -> f(x::Int) = ()
f (generic function with 1 method)

julia> @doc "world" -> f(x::String) = ()
f (generic function with 2 methods)

help?> f
f(x::Int64)
  hello

f(x::AbstractString)
  world

Previously this would have printed:

  hello

  world

which doesn't seem as useful/doesn't match the convention used currently in julia, e.g.

help?> +
Base.Graphics.+(bb1::BoundingBox, bb2::BoundingBox) -> BoundingBox

   Returns the smallest box containing both boxes

Base.+(x, y...)

   Addition operator. "x+y+z+..." calls this function with all
   arguments, i.e. "+(x, y, z, ...)".

cc @one-more-minute (apologies in advance if there's a better place in the code to put this).

Note: this needs a test, but thought I'd throw it out there.

@jiahao jiahao added the docs This change adds or pertains to documentation label Jan 19, 2015
@MikeInnes
Copy link
Member

Thanks for having a go at this, but I'm not sure it's the right way to go. It would be OK if it was opt-in for methods, but I suspect that in many cases you won't want to simply copy the signature. For example, in your example above where the returned value is named, or in cases where the docs have more descriptive argument names than the code itself – in those cases trying to add that information on top of this is going to add a fair bit more clutter.

@hayd
Copy link
Member Author

hayd commented Jan 19, 2015

For example, in your example above where the returned value is named

Do you mean the RHS here?

Base.Graphics.+(bb1::BoundingBox, bb2::BoundingBox) -> BoundingBox

I agree the return type would be nice to have... but I would say opt-out would be better (I think most functions the signature will be sufficient), i.e. for a specific function instead of the default signature write the following instead, or append the following... Or better yet, always include the return type.

Is there an example you have in mind where the signature would not be valuable? (i.e. you want less information?)


At the moment the help for the function is kind of useless if the functions are different because it doesn't include any signature information... you are forced to look at methods(f) and then at the help for the specific signature you want e.g. @doc f(1). This is a lot of indirection.

IMO it's not clutter, the documentation is meaningless without its corresponding signature information... :(

@MikeInnes
Copy link
Member

I not saying the signature isn't important, I'm just saying that in most cases you'd have to write it out by hand anyway, because the generated one isn't going to be clear enough. If you have two signatures that's going to be confusing, so it has to be opt-out at least.

I may well be wrong about that, and if it turns out that people are regularly copying and pasting method signatures into docs I'm happy to change my mind. But where the doc system has been used so far I haven't seen that happening.

@nalimilan
Copy link
Member

I not saying the signature isn't important, I'm just saying that in most cases you'd have to write it out by hand anyway, because the generated one isn't going to be clear enough.

Why wouldn't the default signature be clear enough? That's what people look at using methods().

My two cents is that in R one needs to write the signature by hand, and now we're in the absurd situation where the check tool warns you that the actual signature is not in sync with the docs', but leaves you fix it by hand. So much wasted time...

@hayd
Copy link
Member Author

hayd commented Jan 19, 2015

because the generated one isn't going to be clear enough

What can we do to make it clear enough? Append the return type? Something else? I think we need a sane default here, and copy and pasting the signature isn't it.

Another benefit is consistency if we always include the signature (everywhere).

But where the doc system has been used so far I haven't seen that happening.

I think this is because it's not-obvious that this signature info isn't available when you do @help, I can't think of a case when you wouldn't want to have the doc labelled with the signature... are you advocating pasting it in or just not showing it ?

@MikeInnes
Copy link
Member

Ok, to make it more clear what I mean by the generated signature not being clear:

Say you want to annotate return type:

Base.Graphics.+(bb1::BoundingBox, bb2::BoundingBox) -> BoundingBox

This can only be automated in a very limited set of cases.

Maybe you want to document a general method like so:

@doc "*(s1::String, s2::String) ..." ->
*(::ASCIIString, ::ASCIIString) = ...

@doc "fft(A[, dims]) ..." fft

rather than showing the very complex fft signatures, or otherwise alter the signature to make it more readable (making argument names more verbose, using infix notation, avoiding redundant type hints and complex parametisations etc.)

string1 * string2
widen(type | x)
...

You might not even want a signature for the particular method.

There are multiple different ways to annotate optional and named parameters:

# combined
fft(A, [dims])
# separate
fft(A)
....
fft(A, dims)
....

At the moment this PR doesn't support optional arguments at all, let alone all of the possible ways you might want to document them. To be clear, I'm not saying that having some automation isn't useful, I'm saying that it has to deal with all of these cases well (even if that just means getting out of the way).

And they're not edge cases, by any means. Take a look at the manual; the vast majority of the signatures there fall into one of the above categories and simply couldn't be automatically generated (at least not in such a simplistic way). Hence my suggestion; functionality that only serves a minority of cases should be opt-in.

@hayd
Copy link
Member Author

hayd commented Jan 19, 2015

re return types, here is an old issue about declaring them: #1090

But there is a way to work around this for now: either add syntax to / attach "return type" to the doc, display it nicely (as above) if it exists in the doc.

(I think you said this before: doc allows attaching arbitrary things... so we can define a return_type type.)


re the optional arguments, it's nice to have but IMO you don't actually lose that much by having them separate...

There are multiple different ways to annotate optional and named parameters:

I don't think at the moment this isn't supported at all in docs anyhow... Perhaps I have this wrong... currently you attach the doc to a specific signature / definition?

@nalimilan
Copy link
Member

Yeah, it's going to be complex (though not impossible). But the advantage of an automated solution will be a greater consistency, making it easier to read the docs.

Maybe one solution is to allow associating a "signature" section or metadata, and in the absence of that information the default signature would be printed. This would make everybody happy and would allow a progressive transition to automatically-generated signatures when the system improves.

@prcastro
Copy link
Contributor

Being opt-out could be the way to go. Maybe with a macro @doc_nosignature or using doc_nosig"documentation string" or something like this. If we make this opt-in, maybe people will forget to use it at all, and that will be a problem (as @nalimilan said is happening with R).

@hayd hayd mentioned this pull request Aug 20, 2015
@hayd hayd changed the title DOC prefix docstring by method name DOC prefix docstring by method signature Aug 20, 2015
@kshyatt
Copy link
Contributor

kshyatt commented Dec 19, 2017

I feel like we can probably close this...

@kshyatt kshyatt closed this Dec 19, 2017
@hayd hayd deleted the doc branch December 20, 2017 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants