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

Update manual: Vararg{T,N} is not a type anymore #39239

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

fingolfin
Copy link
Contributor

@fingolfin fingolfin commented Jan 14, 2021

I used the term "special value" as that is used a few lines above.

Perhaps this could be backported to 1.6, to avoid confusion there, too?

Copy link
Member

@Keno Keno left a comment

Choose a reason for hiding this comment

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

I believe you pointed out that the same text is duplicated in the Vararg docstring in basedocs.jl and should be fixed there also.

doc/src/manual/types.md Outdated Show resolved Hide resolved
@Keno
Copy link
Member

Keno commented Jan 14, 2021

Perhaps this could be backported to 1.6, to avoid confusion there, too?

1.6 does not include the change that made Vargarg not a type.

@mkitti
Copy link
Contributor

mkitti commented Jan 14, 2021

Should Core.TypeofVararg be mentioned somewhere in the docs in case Vararg needs to be passed to a method?

I recently addressed this issue here:
https://github.com/JuliaPy/PyCall.jl/pull/877/files

@Keno
Copy link
Member

Keno commented Jan 14, 2021

That's generally just supposed to be spelled typeof(Vararg), but it's very much an internal detail.

@mkitti
Copy link
Contributor

mkitti commented Jan 15, 2021

That's generally just supposed to be spelled typeof(Vararg), but it's very much an internal detail.

Would the intended way to define a function that accepts Vararg be g(::typeof(Vararg)) = 5 then?

@Keno
Copy link
Member

Keno commented Jan 15, 2021

Would the intended way to define a function that accepts Vararg be g(::typeof(Vararg)) = 5 then?

That is my preference, but opinions differ. More generally, maybe we should just have an expanded collection of reflection methods in base to avoid most uses for this. It's a bit unfortunate how widespread introspection of Vararg is across the package ecosystem.

mkitti added a commit to mkitti/SymbolServer.jl that referenced this pull request Jan 15, 2021
See JuliaLang/julia#39239 (comment)

`Core.TypeofVararg` introduced in JuliaLang/julia#3813
`VERSION >= v"1.7.0-DEV.77"`
@fingolfin
Copy link
Contributor Author

I believe you pointed out that the same text is duplicated in the Vararg docstring in basedocs.jl and should be fixed there also.

Actually I wasn't aware of the docstring -- I was talking about the text a few lines above where I edited it :-). But sure, I can align it with the docstring, too.

@fingolfin
Copy link
Contributor Author

I committed your suggestion, but I see the docstring indeed also needs tweaking, will try to take care of it.

@fingolfin
Copy link
Contributor Author

Updated. I've also contemplated extending the example(s), but it either gets a bit long or a bit complicated...

julia> T1 = Tuple{AbstractString, Vararg};

julia> T2 = Tuple{AbstractString, Vararg{Int}};

julia> T3 = Tuple{AbstractString, Vararg{Int,2}};

julia> map(T -> isa(("1",), T), (T1, T2, T3))
(true, true, false)

julia> map(T -> isa(("1",1), T), (T1, T2, T3))
(true, true, false)

julia> map(T -> isa(("1",1,2), T), (T1, T2, T3))
(true, true, true)

julia> map(T -> isa(("1",1,2,3.), T), (T1, T2, T3))
(false, true, false)

@JeffBezanson JeffBezanson merged commit bea5e6e into JuliaLang:master Jan 15, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
@fingolfin fingolfin deleted the mh/Vararg-no-type branch July 7, 2021 10:01
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.

None yet

4 participants