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

Added "Provide hints to similar functions" #23788

Merged
merged 8 commits into from
Sep 25, 2017

Conversation

ChristianKurz
Copy link
Contributor

This PR proposes a standard wording for hints to similar functions.
Feel free to correct and change!

#23763, #20531

This PR proposes a standard wording for hints to similar functions.
Feel free to correct and change!
discoverability please provide a short list of these similar functions in a `See also: ` section.

```
See also: bar!, baz, baaz
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps write this as

See also: [`bar!`](@ref), [`baz`](@ref),

etc. This will cause the identifier to be quoted and a link to its documentation will be generated in the HTML.

in a list:

```
See also: bar! - mutating variant, see for description of keyword arguments
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure this will render correctly?

5. Include any code examples in an `# Examples` section.
5. Provide hints to similar functions

Sometimes there are functions of similar functionality, which are named quite different. To increase
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps similar functionality -> related functionality?
Also, which are named quite different is not needed; it is useful to list related functions even if they have similar names.

5. Provide hints to similar functions

Sometimes there are functions of similar functionality, which are named quite different. To increase
discoverability please provide a short list of these similar functions in a `See also: ` section.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps: please provide a short list of these similar functions in a... -> please provide a short list of these in a ... to avoid duplication

```
See also: bar!, baz, baaz
```
If the difference in functionality is not obvious from the name, you can provide a short description
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this second part. Docstrings should be distinct and describe the function, not describe other functions. If similar functions are listed it is easy enough to look up what they do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not like that part either. I proposed this list as it seems to be common practice to refer other functions with a short description in docstrings:

 Sort a multidimensional array A along the given dimension. See sort! for a description of possible keyword arguments.
  shuffle([rng=GLOBAL_RNG,] v)

  Return a randomly permuted copy of v. The optional rng argument specifies a random number generator (see Random Numbers). To
  permute v in-place, see shuffle!. To obtain randomly permuted indices, see randperm.

Perhaps the better approach is to remove these redundant descriptions and just rely on the See also:-list

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difference is that the examples you listed here refer to more documentation about the documented function -- they don’t describe other functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least the docstring to shuffle does:

 To permute v in-place, see shuffle!. To obtain randomly permuted indices, see randperm.

And i think i have seen this pattern quite often.

Should we change those docs?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that can be decided from case to case, and those examples are probably there for a reason. I just don't think we should explicitly recommend that style, as suggested in this PR.

Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mschauer
Copy link
Contributor

Note that there are often better alternatives to adding references under "See also".

Often the relationship can make explicit, "This function quit is equivalent to exit(0)" is better than "See also: exit".

Having a long list of see alsos indicates that there is an undocumented higher-level concept:
"RoundNearest is the default RoundingMode" is better than "See also: RoundNearest, RoundNearestTiesAway, RoundNearestTiesUp, RoundToZero, RoundFromZero, RoundUp, RoundDown"

(This is basically what the Wikipedia community figured out after some disagreements.)

discoverability please provide a short list of these similar functions in a `See also: ` section.

```
See also: [`bar!`](@ref), [`baz`](@ref), [`baaz`](@ref)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should be a # section, just like # Examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a heading is a bit too verbose. # See also: pop!, shift! renders as

     See also: pop!, shift!
    ≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡≡

I would rather bold **See also:** to add visibility.

I know, i called it section in the docs, which might be a bit confusing, if it's not rendered as a section...

Copy link
Member

@nalimilan nalimilan Sep 20, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a more general issue (even for # Examples) with the three lines which are too heavy (see #22870). So I'd rather use a consistent style with one section for "See also " and one section for "Examples", and fix both at the same time later. Note that the REPL isn't the only display format: there's also the HTML manual, where sections look much nicer (see screenshot at #22870).

Also, since you recommend to use a bullet list when describing functions, it's important to clearly separate it from the description of the function itself. So overall I suggest always having # See also section heading just like # Examples, and then always a line break, followed either by a list of functions on a single line, or list with bullet points.

EDIT: see also the argument made by @StefanKarpinski at #22870 (comment) regarding the semantic content of sections, as opposed to just using bold.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it should be a section like that, I think it could even just be the last sentence of the docstring:

    push!(collection, items...) -> collection

Insert one or more items at the end of collection. See also: `append!`, `pop!`

# Examples
[...]

IMO this should just be a tiny side note, definitely not as important as examples for the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see you dropped the suggestion to use a bullet list. Let's go with the lighter solution then, but at least add a line break before it. Also please remove the term "section" (maybe call it "paragraph").

@@ -81,7 +81,15 @@ As in the example above, we recommend following some simple conventions when wri
...
"""
```
5. Include any code examples in an `# Examples` section.
5. Provide hints to similar functions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better use "related" here too. exp and log are definitely related, but I wouldn't consider them as "similar". Also missing ending period.

5. Provide hints to related functions.

Sometimes there are functions of related functionality. To increase discoverability please provide
a short list of these in a `**See also:**` paragraph.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer ditching the boldness here, it draws attention from the docstring.

Copy link
Contributor Author

@ChristianKurz ChristianKurz Sep 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, the newline should be enough.

I updated the pop!, push! etc. docs to comply with these rules.
I kind of like the boldness better than the newline, though. Any further comments?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion regarding the bold, but we probably don't need a new line after "See also". A line break before it should be enough IMHO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the bold looks ugly. With a new line, you mean an empty line between the docstring and "See also" right? Just a new line won't render as a such.

julia> """
           foo

       Calculate foo.
       See also: `bar`
       """
       foo
foo

help?> foo
search: floor pointer_from_objref OverflowError RoundFromZero FileMonitor functionloc StackOverflowError @functionloc Factorization OutOfMemoryError unsafe_pointer_to_objref default_worker_pool

  foo

  Calculate foo. See also: bar

vs

julia> """
           foo

       Calculate foo.

       See also: `bar`
       """
       foo
foo

help?> foo
search: floor pointer_from_objref OverflowError RoundFromZero FileMonitor functionloc StackOverflowError @functionloc Factorization OutOfMemoryError unsafe_pointer_to_objref default_worker_pool

  foo

  Calculate foo.

  See also: bar

a short list of these in a `See also:` paragraph.

```
See also:[`bar!`](@ref), [`baz`](@ref), [`baaz`](@ref)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after :.

@KristofferC
Copy link
Sponsor Member

I fixed it the space. This LGTM now.

@ChristianKurz
Copy link
Contributor Author

ChristianKurz commented Sep 25, 2017

Thank you and sorry for making some less than helpful edits. I'm still new to this and i really should start editing files locally and not through the GitHub 'edit' feature...

@fredrikekre
Copy link
Member

#23796 × 2

@fredrikekre fredrikekre merged commit ee7b0b8 into JuliaLang:master Sep 25, 2017
@nalimilan
Copy link
Member

Thanks @Shorty66! Hopefully if you make more PRs they will be merged more quickly now that we have decided what conventions to follow.

ChristianKurz pushed a commit to ChristianKurz/julia that referenced this pull request Oct 13, 2017
"can I apply function to columns of a matrix or rows ? something like map..."
I see this question asked a lot lately - so i think the best way is to hint at `mapslices` from the documentation of `map`.

See [JuliaLang#23788]
ararslan pushed a commit that referenced this pull request Oct 15, 2017
"can I apply function to columns of a matrix or rows ? something like map..."
I see this question asked a lot lately - so i think the best way is to hint at `mapslices` from the documentation of `map`.

See [#23788]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants