Change signatures that help is showing (revive #25672, fix #20064) #29102

thofma · 2018-09-09T06:29:17Z

This is a rebased version of #25672, which fixes the regression noticed in #20064.

I hope you don't mind @KristofferC

I used the check suggested by Jameson and added tests.

Here is the old and new behavior

1.0/master

help?> +(::Dates.Date, ::Dates.Time)
  +(x, y...)

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

  Examples
  ≡≡≡≡≡≡≡≡≡≡

  julia> 1 + 20 + 4
  25
  
  julia> +(1, 20, 4)
  25

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  dt::Date + t::Time -> DateTime

  The addition of a Date with a Time produces a DateTime. The hour, minute, second, and millisecond parts of
  the Time are used along with the year, month, and day of the Date to create the new DateTime. Non-zero
  microseconds or nanoseconds in the Time type will result in an InexactError being thrown.

This PR

help?> +(::Dates.Date, ::Dates.Time)
  dt::Date + t::Time -> DateTime

  The addition of a Date with a Time produces a DateTime. The hour, minute, second, and millisecond parts of
  the Time are used along with the year, month, and day of the Date to create the new DateTime. Non-zero
  microseconds or nanoseconds in the Time type will result in an InexactError being thrown.

Consider

"""
foobar
"""
function foobar end

"""
Number
"""
foobar(x::Number) = x

"""
Real
"""
foobar(x::Real) = x


"""
Int
"""
foobar(x::Int) = x


"""
Real, Int
"""
foobar(x::Real, y::Int) = x

1.0/master

help?> foobar(1)
  Number

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Real

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Int

help?> foobar(::Real)
  Number

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Real

help?> foobar(::Real, ::Real)
  Number

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Real

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Int

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Real, Int

This PR

help?> foobar(1)
  Int

help?> foobar(2.0)
  Real

help?> foobar(2.0, 1)
  Real, Int
help?> foobar(::Real)
  Real

  ───────────────────────────────────────────────────────────────────────────────────────────────────────────

  Int

help?> foobar(::Real, ::Real)
  Real, Int

Should be squashed when merged.

thofma · 2018-09-12T05:15:36Z

Since this fixes a regression/bug introduced in 0.7, can this be backported to 1.0?

KristofferC · 2018-09-12T14:42:39Z

stdlib/REPL/src/docview.jl

    # Lookup `binding` and `sig` for matches in all modules of the docsystem.
    for mod in modules
        dict = meta(mod)
        if haskey(dict, binding)
            multidoc = dict[binding]
            push!(groups, multidoc)
            for msig in multidoc.order
-                sig <: msig && push!(results, multidoc.docs[msig])
+                if typeintersect(msig, sig) != Union{}
+                #if (isconcretetype(sig) && sig <: msig) || (!isconcretetype(sig) && msig <: sig) || sig == Union{}


KristofferC · 2018-09-12T14:42:50Z

I'm not super comfortable putting this straight into 1.0.1. It has had little testing and I am not sure exactly what effects it will have when it comes to rendered docs. Feels better to have this live on master for a while first.

KristofferC · 2018-09-12T15:30:59Z

One thing that would be good is to render the docs with and without this PR and diff them.

thofma · 2018-09-13T03:35:11Z

Thanks for your feedback @KristofferC. You are right about the backporting. It has not been tested enough in the wild.

By diffing them, do you mean posting here the different output as you did in #25672?

thofma · 2018-09-17T06:48:19Z

@KristofferC I added the diff at the top.

KristofferC · 2018-09-17T14:25:35Z

Thanks, but what I meant was more along the lines of rendering the HTML documentation with this PR and without and then looking at the diff between them to see the effect on a more large scale, than just a toy example.

thofma · 2018-09-18T03:14:58Z

@KristofferC Thanks. I checked and they are different:
With this PR:
http://www.mathematik.uni-kl.de/~thofmann/build_pr/html/en/base/collections.html#Base.pairs
Without:
http://www.mathematik.uni-kl.de/~thofmann/build/html/en/base/collections.html#Base.pairs

In the documentation source it reads @doc Base.pairs.
I don't know what Documenter is doing, because it works in the REPL properly:

help?> pairs search: pairs uppercasefirst Pair partialsort partialsort! partialsortperm partialsortperm! PartialQuickSort pairs(collection) Return an iterator over key => value pairs for any collection that maps a set of keys to a set of values. This includes arrays, where the keys are the array indices. ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── pairs(IndexLinear(), A) pairs(IndexCartesian(), A) pairs(IndexStyle(A), A) An iterator that accesses each element of the array A, returning i => x, where i is the index for the element and x = A[i]. Identical to pairs(A), except that the style of index can be selected. Also similar to enumerate(A), except i will be a valid index for A, while enumerate always counts from 1 regardless of the indices of A. Specifying IndexLinear() ensures that i will be an integer; specifying IndexCartesian() ensures that i will be a CartesianIndex; specifying IndexStyle(A) chooses whichever has been defined as the native indexing style for array A. Mutation of the bounds of the underlying array will invalidate this iterator. Examples ≡≡≡≡≡≡≡≡≡≡ julia> A = ["a" "d"; "b" "e"; "c" "f"]; julia> for (index, value) in pairs(IndexStyle(A), A) println("$index $value") end 1 a 2 b 3 c 4 d 5 e 6 f julia> S = view(A, 1:2, :); julia> for (index, value) in pairs(IndexStyle(S), S) println("$index $value") end CartesianIndex(1, 1) a CartesianIndex(2, 1) b CartesianIndex(1, 2) d CartesianIndex(2, 2) e See also: IndexStyle, axes.

thofma · 2018-09-18T04:21:29Z

I guss we have to apply the change also here:
https://github.com/JuliaDocs/Documenter.jl/blob/master/src/DocSystem.jl#L176-L225
Does this make sense, to you @mortenpi?

Actually, I don't know why the Documenter output should be different at all. It seems that Documenter is collecting the docstrings itself and is not calling Docs.doc.

KristofferC · 2018-09-18T12:22:55Z

I didn't know this was only for repl use, I thought Documenter used this as well to extract what docstrings to show in the rendered documentation.

mortenpi · 2018-09-18T21:29:12Z

@thofma I am not actually very familiar the docsystem code in Documenter, but as a general principle, Documenter should behave similarly to the REPL I'd say. If there is / will be a difference between them, please do open an issue in Documenter for this. Or if you'd willing to PR the fix, that would be even more awesome.

Also, if we can offload as much of the docsystem logic to Base, instead of duplicating it in Documenter, that would be good too. There's maybe a reason why Documenter does not use Docs.doc, but I think it would be worth looking into if we can get rid of this apparently duplication.

thofma · 2018-09-19T04:50:56Z

I tried to investigate why the built documentation looks different with this PR (it really should not since Documenter does its own thing and is not affected by the changes here). But whenever I tried to debug it using some print statements in Documenter, it gave me the correct result. I think I am hitting some weird bug.

I will look at Documenter.

thofma · 2018-10-02T09:55:55Z

@KristofferC Rebased and the bug is gone. Documenation built is not affected.

This PR here only affects the REPL helpmode and nothing else, which is explained in the OP and covered by tests.

Is there something else that needs to be addressed?

thofma · 2018-10-26T11:28:12Z

Friendly bump

thofma · 2019-03-25T08:57:22Z

The issue persists with latest master, so again a friendly bump.

fredrikekre requested a review from KristofferC September 12, 2018 06:10

fredrikekre added docsystem The documentation building system backport pending 1.0 labels Sep 12, 2018

KristofferC reviewed Sep 12, 2018

View reviewed changes

fredrikekre removed the backport pending 1.0 label Sep 12, 2018

thofma force-pushed the th/whatdoctoshow branch from b79d0d2 to 9932db7 Compare September 13, 2018 03:32

thofma changed the title ~~Revive #25672, fix #20064~~ Change signatures that help is showing (revive #25672, fix #20064) Sep 18, 2018

thofma mentioned this pull request Sep 25, 2018

Filtering included docstrings JuliaDocs/Documenter.jl#839

Open

KristofferC and others added 2 commits October 2, 2018 11:32

change signatures that help is showing [ci skip]

b27fd13

Rebase and add tests

c409508

thofma force-pushed the th/whatdoctoshow branch from 9932db7 to c409508 Compare October 2, 2018 09:48

thofma closed this Sep 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change signatures that help is showing (revive #25672, fix #20064) #29102

Change signatures that help is showing (revive #25672, fix #20064) #29102

thofma commented Sep 9, 2018 •

edited

Loading

thofma commented Sep 12, 2018

KristofferC Sep 12, 2018

KristofferC commented Sep 12, 2018 •

edited

Loading

KristofferC commented Sep 12, 2018

thofma commented Sep 13, 2018

thofma commented Sep 17, 2018

KristofferC commented Sep 17, 2018

thofma commented Sep 18, 2018 •

edited

Loading

thofma commented Sep 18, 2018 •

edited

Loading

KristofferC commented Sep 18, 2018

mortenpi commented Sep 18, 2018

thofma commented Sep 19, 2018

thofma commented Oct 2, 2018

thofma commented Oct 26, 2018

thofma commented Mar 25, 2019

Change signatures that help is showing (revive #25672, fix #20064) #29102

Change signatures that help is showing (revive #25672, fix #20064) #29102

Conversation

thofma commented Sep 9, 2018 • edited Loading

1.0/master

This PR

1.0/master

This PR

thofma commented Sep 12, 2018

KristofferC Sep 12, 2018

Choose a reason for hiding this comment

KristofferC commented Sep 12, 2018 • edited Loading

KristofferC commented Sep 12, 2018

thofma commented Sep 13, 2018

thofma commented Sep 17, 2018

KristofferC commented Sep 17, 2018

thofma commented Sep 18, 2018 • edited Loading

thofma commented Sep 18, 2018 • edited Loading

KristofferC commented Sep 18, 2018

mortenpi commented Sep 18, 2018

thofma commented Sep 19, 2018

thofma commented Oct 2, 2018

thofma commented Oct 26, 2018

thofma commented Mar 25, 2019

thofma commented Sep 9, 2018 •

edited

Loading

KristofferC commented Sep 12, 2018 •

edited

Loading

thofma commented Sep 18, 2018 •

edited

Loading

thofma commented Sep 18, 2018 •

edited

Loading