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

add a deprecate_stdlib macro that binds to the right functions #25692

Merged
merged 1 commit into from
Jan 25, 2018

Conversation

vchuravy
Copy link
Member

@vchuravy vchuravy commented Jan 22, 2018

The long anticipated followup to #24843. Since we actually have the stdlib modules in the sysimage
we can deprecate correctly and still point to the right functions.

Turns

julia> readdlm("test.csv")
ERROR: Base.readdlm has been moved to the standard library package DelimitedFiles.
Restart Julia and then run `using DelimitedFiles` to load it.
Stacktrace:
 [1] error(::Function, ::String, ::String, ::String, ::String, ::String, ::String) at ./error.jl:42
 [2] #readdlm#829(::Base.Iterators.IndexValue{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::Function, ::String, ::Vararg{String,N} where N) at ./deprecated.jl:138
 [3] readdlm(::String, ::Vararg{String,N} where N) at ./deprecated.jl:138
 [4] top-level scope

julia> using DelimitedFiles
WARNING: using DelimitedFiles.readdlm in module Main conflicts with an existing identifier.

julia> readdlm("test.csv")
ERROR: Base.readdlm has been moved to the standard library package DelimitedFiles.
Restart Julia and then run `using DelimitedFiles` to load it.
Stacktrace:
 [1] error(::Function, ::String, ::String, ::String, ::String, ::String, ::String) at ./error.jl:42
 [2] #readdlm#829(::Base.Iterators.IndexValue{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}, ::Function, ::String, ::Vararg{String,N} where N) at ./deprecated.jl:138
 [3] readdlm(::String, ::Vararg{String,N} where N) at ./deprecated.jl:138
 [4] top-level scope

into:

julia> readdlm("test.csv")
┌ Warning: `readdlm`, has been moved to the standard library package `DelimitedFiles`.
│ Add a `using DelimitedFiles` to your imports.
│   caller = top-level scope
└ @ Core :0
ERROR: ArgumentError: Cannot open 'test.csv': not a file

This still leads to:

julia> readdlm("test.csv")
julia> using DelimitedFiles
WARNING: using DelimitedFiles.readdlm in module Main conflicts with an existing identifier.
  • Check if kwargs work
  • Deprecation warnings are printed multiple times

@eval @deprecate_moved $(Symbol("@sprintf")) "Printf" true true

# PR #21709
@deprecate cov(x::AbstractVector, corrected::Bool) cov(x, corrected=corrected)
Copy link
Member Author

Choose a reason for hiding this comment

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

The below was certainly not planned to be removed

@vchuravy vchuravy force-pushed the vc/stdlib_deprecations branch 2 times, most recently from 58803f6 to fc01c4d Compare January 23, 2018 20:11
@vchuravy
Copy link
Member Author

Ok this is good to go from my side.

base/bitarray.jl Outdated
@@ -431,7 +431,7 @@ end

function copyto!(dest::BitArray, src::Array)
length(src) > length(dest) && throw(BoundsError(dest, length(dest)+1))
length(src) == 0 && return det
length(src) == 0 && return dest
Copy link
Member Author

Choose a reason for hiding this comment

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

sneaky... I will split this out and make a separate PR.

Copy link
Member

@KristofferC KristofferC Jan 24, 2018

Choose a reason for hiding this comment

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

@code_warntype copyto!(BitArray([true]), [1])
....
 end::Union{typeof(Base.det), BitArray{1}}

lol...

Copy link
Member Author

Choose a reason for hiding this comment

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

I now understand how Jameson found this:

WARNING: Base.det is deprecated: it has been moved to the standard library package `LinearAlgebra`.
Add a `using LinearAlgebra` to your imports..
 in module Base
in copyto! at bitarray.jl

@vchuravy
Copy link
Member Author

Absent any further objections I plan to merge this this evening (EST)

- New `deprecate_stdlib` macro that adds user friendly deprecation for
  bindings that have been moved to the stdlib, but are still in the sysimage.
- Simplify `deprecate_moved` macro.
- add deprecation for BLAS submodule

Also-by: Jameson Nash <[email protected]>
@vchuravy vchuravy force-pushed the vc/stdlib_deprecations branch from e463b6b to cf729a1 Compare January 24, 2018 20:41
@vchuravy vchuravy merged commit fe8d448 into master Jan 25, 2018
@vchuravy vchuravy deleted the vc/stdlib_deprecations branch January 25, 2018 02:55
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.

2 participants