Skip to content

Conversation

@kdheepak
Copy link
Contributor

Fixes #124

@kdheepak kdheepak force-pushed the kd/fix-test-failure branch 9 times, most recently from 215a45c to fe0fbd9 Compare October 21, 2021 21:31
@kdheepak
Copy link
Contributor Author

This is ready for review / ready to be merged.

@kdheepak kdheepak force-pushed the kd/fix-test-failure branch from fe0fbd9 to 9c41355 Compare October 22, 2021 03:40
Copy link
Member

@MichaelHatherly MichaelHatherly left a comment

Choose a reason for hiding this comment

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

Just one comment/question on this inline. Thanks for sorting it out!

@kdheepak kdheepak force-pushed the kd/fix-test-failure branch from 9c41355 to 91b1db5 Compare October 22, 2021 16:37
@kdheepak
Copy link
Contributor Author

There's one printing that I currently don't like but don't know how to fix.

Given this type: Union{TypeVar(:T, Integer), TypeVar(:U, Float64)}, I want to construct this Union{T, U} where T<:Integer where U<:Float64. But I'm really struggling to figure out how to do that.

I thought this would do it but it just constructs the original type again:

julia> UnionAll(TypeVar(:U, Float64), UnionAll(TypeVar(:T, Integer), Union{TypeVar(:T, Integer), TypeVar(:U, Float64)}))
Union{T<:Integer, U<:Float64}

@kdheepak kdheepak force-pushed the kd/fix-test-failure branch from 4f7b316 to 0fd456f Compare October 22, 2021 19:29
@kdheepak
Copy link
Contributor Author

Previously, it was printing this:

k_7(x::Union{Nothing, T<:Integer}, y::Integer) -> Union{Nothing, T} where T<:Integer

Now, it'll print this:

k_7(x::Union{Nothing, T} where T<:Integer, y::Integer) -> Union{Nothing, T} where T<:Integer

Thanks to @pfitzseb for answering questions about this.

@kdheepak
Copy link
Contributor Author

kdheepak commented Oct 22, 2021

The only thing that remains now is the @test_broken tests. When I try to replicate the issue in a Julia script, the docs print correctly. This seems to be failing only in the test suite for some reason.

# test.jl
using DocStringExtensions
const DSE = DocStringExtensions
using Test
import Base.Docs: meta, @var, DocStr, parsedoc

module M end

function find_function_name_symbol(e::Expr)
    if e.head != :call
        return find_function_name_symbol(e.args[1])
    else
        return e.args[1]
    end
end

function get_doc_ext_signature(sym, typesig)
    doc = Docs.DocStr(Core.svec(), nothing, Dict())
    buf = IOBuffer()
    doc.data = Dict(
        :binding => Docs.Binding(M, sym),
        :typesig => eval(typesig),
        :module => M,
    )
    # @show doc.data
    DSE.format(DSE.TYPEDSIGNATURES, buf, doc)
    str = String(take!(buf))
    str = replace(str, "```julia" => "")
    str = replace(str, "```" => "")
    str = strip(str)
    return split(str, "\n")
end

function test()
    exprs = [
        :(
            k_7(x::T) where T = x
        )
    ]

    for expr in exprs

        println("# $(expr.args[1])")
        println()

        docs_sig = Base.Docs.signature(expr)
        Base.eval(M, expr)
        f = find_function_name_symbol(expr)

        docs_ext_sig = get_doc_ext_signature(f, docs_sig)
        @show first(docs_ext_sig)
        println()
    end
end

test()
julia --project test.jl                                                                                                  ─╯
# k_7(x::T) where T

first(docs_ext_sig) = "k_7(x) -> Any"

If I add @show doc.data:

doc.data = Dict{Symbol, Any}(:typesig => Tuple{T} where T, :module => Main.M, :binding => Main.M.k_7)

I get this output, which is what the test is using. So I'm very puzzled as to what is going on here.

@MichaelHatherly
Copy link
Member

I'll try and investigate that one when I next get a chance, not sure what's happening with it either.

@kdheepak
Copy link
Contributor Author

The three @test_broken tests were in there before I started this PR, but I'm pretty sure it's just something silly I'm missing. It'd be nice if we sorted it out in this PR, but also it is not necessary and this can be merged.

@kdheepak kdheepak force-pushed the kd/fix-test-failure branch from 2ff2b30 to a50daf4 Compare October 25, 2021 12:15
@kdheepak
Copy link
Contributor Author

I'm not sure exactly why this happens, but when I comment the other tests out or if I move the test to the beginning of the testset, the broken tests pass. I put some prints in the code to see where the difference was and it happens at this line:

local groups = methodgroups(func, typesig, modname)

When the test is at the end of the testset, this function returns an empty Vector. When it is at the beginning, it returns the expected Vector of method groups. In fact, anywhere before the test for k_3 and it works fine. After k_3 though, it fails. I renamed it to k_0 and moved it to a different location.

@MichaelHatherly
Copy link
Member

Thanks for tracking that down further. Very weird behavior, if things are all working now and you're happy with the implementation then we can merge as is.

@kdheepak
Copy link
Contributor Author

Yup! I am happy with it!

@MichaelHatherly MichaelHatherly merged commit 95c1f38 into JuliaDocs:master Oct 25, 2021
@MichaelHatherly
Copy link
Member

Thanks again for sorting that one out!

@kdheepak kdheepak deleted the kd/fix-test-failure branch October 25, 2021 19:35
@kdheepak
Copy link
Contributor Author

Thanks for pinging me! Feel free to do so again if something else comes up with this.

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.

expected test failure from the incoming Core.Compiler changes

2 participants