-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
assorted subtyping tests #20627
assorted subtyping tests #20627
Conversation
these should use the new syntax |
Why should it error? You defined two methods that overrides each other and both of them matches the signature you are calling. |
@tkelman edit: got it, I think. incoming |
I don't understand the error message
and can't work out from |
test/subtype.jl
Outdated
f19041{K}(x::A19041{K}) = deepcopy(x) | ||
@test f19041(A19041(B19041)) == A19041{B19041}() | ||
|
||
# #19985 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo?
Have you tried doing exactly what the deprecation warning says? |
It's on the second line of the error message in case you missed it (I did on first reading). |
test/subtype.jl
Outdated
struct B12814{N, T} <: A12814{N, T} | ||
x::NTuple{N, T} | ||
end | ||
Base.call{T <: A12814, X <: Array}(::Type{T}, x::X) = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
call overloading in this style is deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw that when I tried this is in 0.5, but couldn't figure out the transformation (and the warning had disappeared in 0.6?). How should this be spelled now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should follow the deprecation warning on 0.5.
(::Type{T}){T<:A12814,X<:Array}(x::X) = 1
Thanks all for the help! |
The meaning of
What didn't work? The depwarn shouldn't be printed anymore? |
Writing
and
on two of the Travis builds. |
Please check the line number. That's not the same line at all. https://github.com/JuliaLang/julia/pull/20627/files#diff-4cf37a18308090507d83e9d96dd01692R935 is wrong. |
I don't think that deprecation message is all that obvious, fwiw |
What would be a more obvious one? |
printing the actual contents of the line/signature that caused the warning instead of a |
The abbreviated part is not important since it does not need to be changed at all. It should be ok to print it if it's short but when it's longer it'll make it much harder to spot the actual change that needs to be made. |
it is important if it makes it easier to identify what actually needs changing. right now it's not very clear, like when there are multiple inner constructors |
What about multiple inner constructors? If none of them have type parameters they need to be changed in exactly the same way and are printed as such in the deprecation warning? |
then you should get the warning once per constructor, otherwise fixing one will only change the line number of the warning (if you're lucky) so you don't get much feedback that you partially fixed it |
You do? julia> struct A{T}
A() = 1
A(a) = 2
end
WARNING: deprecated syntax "inner constructor A(...) around REPL[1]:2".
Use "A{T}(...) where T" instead.
WARNING: deprecated syntax "inner constructor A(...) around REPL[1]:3".
Use "A{T}(...) where T" instead. |
Just noticed that the inner constructor with type parameters is not printed correctly. julia> struct C{T}
C{T2}(a::T2) = T2
end
WARNING: deprecated syntax "inner constructor C(...) around REPL[3]:2".
Use "C{T}(...) where T" instead. Should say WARNING: deprecated syntax "inner constructor C{T2}(...) around REPL[3]:2".
Use "C{T}(...) where {T,T2}" instead. Though this gets a lot trickier for struct D{T}
D{T}(::T) = 1
end which should probably suggest sth like |
What should the correct version of lines 930 - 935 be? |
mutable struct D8915{T <: Union{Float32,Float64}}
D8915{T}(a) where T = 1
D8915{T}(a::Int) where T = 2
end
@test D8915{Float64}(1) == 2 |
preferably with curly braces |
Is there a problem with remote exceptions? Not sure what to make of this message in the logs, seems to be a different remote worker that failed in each case
|
Yes there are some issues with test result serialization so it doesn't actually show the error. OTOH, please run the tests locally first before pushing. |
bump, needs rebase |
Rebased (hope you don't mind) |
likely needs "fixing whatever was broken about the tests" too in addition to just a rebase, so this'll probably fail at the moment |
Thanks for rebase @pabloferz! ❤️ Fixed tests, builds build. LGTM? |
Bump @JeffBezanson |
Bump @JeffBezanson are any of these still relevant? |
c.f. master list #19998
tests for closed issues #2552 #8625 #8915 #11407
and #12814 #16922 #17943 #18892 (#19159 #19413) #18985 #19041
[Edit: misunderstood method overwriting as in #18892, thanks yuyichao for correction, now has a test]