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

Allow packages to force depwarns #37296

Merged
merged 1 commit into from
Sep 4, 2020
Merged

Allow packages to force depwarns #37296

merged 1 commit into from
Sep 4, 2020

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Aug 31, 2020

Fixes #37164 in a gentler way than proposed in the OP, by allowing packages to call depwarn(args...; force=true). No macro equivalent is provided in this PR.

@fredrikekre
Copy link
Member

Is there an advantage of using depwarn rather than just @warn if you want to show them unconditionally? I guess the ability to turn them into hard errors with --depwarn=error?

@timholy
Copy link
Sponsor Member Author

timholy commented Aug 31, 2020

@warn shows the line containing the @warn statement itself; depwarn shows the caller. With JuliaImages/ImageCore.jl#140, julia test/deprecated.jl yields

julia> include("deprecated.jl")
[ Info: Deprecation warnings are expected
┌ Warning: `convert(Array{BGR}, img)` will soon switch to returning an array with non-concrete element type, which adds flexibility but
│ with great cost to performance. To maintain the current behavior, use `BGR.(img)` instead.
└ @ ImageCore ~/.julia/dev/ImageCore/src/deprecations.jl:6
┌ Info: It is recommended that you fix this now to avoid breakage when a new version is released and this warning is removed.
└ Tip: to see all deprecation warnings together with code locations, launch Julia with `--depwarn=yes` and rerun your code.

ImageCore/src/deprecations.jl:6 is this line, which is not very useful because what the user wants to know is where in my code did I call a deprecated method from?

Whereas julia --depwarn=yes test/deprecated.jl yields

julia> include("deprecated.jl")
[ Info: Deprecation warnings are expected
┌ Warning: `convert(Array{BGR}, img)` will soon switch to returning an array with non-concrete element type, which adds flexibility but
│ with great cost to performance. To maintain the current behavior, use `BGR.(img)` instead.
│   caller = macro expansion at deprecated.jl:8 [inlined]
└ @ Core ~/.julia/dev/ImageCore/test/deprecated.jl:8
┌ Warning: `convert(Array{Lab}, img)` will soon switch to returning an array with non-concrete element type, which adds flexibility but
│ with great cost to performance. To maintain the current behavior, use `Lab.(img)` instead.
│   caller = macro expansion at deprecated.jl:12 [inlined]
└ @ Core ~/.julia/dev/ImageCore/test/deprecated.jl:12
...

Ignore the 2-vs-1 distinction and focus on the fact that now the file & line number indicates the caller (i.e., here and here).

@johnnychen94
Copy link
Sponsor Member

johnnychen94 commented Aug 31, 2020

Is there an advantage of using depwarn rather than just @warn if you want to show them unconditionally?

Another advantage is to not show thousands of warnings for the same function call 😆 . For example, in a for loop.

@fredrikekre
Copy link
Member

Another advantage is to not show thousands of warnings for the same function call

For that you can use maxlog

julia> function f()
           for i in 1:10
               @warn "hello, world" maxlog=1
           end
       end
f (generic function with 1 method)

julia> f()
┌ Warning: hello, world
└ @ Main REPL[2]:3

but yea, the wrong source of the warning is indeed annoying.

@timholy timholy merged commit 405cd6f into master Sep 4, 2020
@timholy timholy deleted the teh/forced_depwarn branch September 4, 2020 20:34
@KristofferC KristofferC mentioned this pull request Sep 7, 2020
29 tasks
KristofferC pushed a commit that referenced this pull request Sep 7, 2020
KristofferC pushed a commit that referenced this pull request Sep 8, 2020
KristofferC pushed a commit that referenced this pull request Sep 10, 2020
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.

Warn that at least one deprecation warning emitted, default depwarn=yes again
4 participants