Extend describe from DataAPI to allow removing StatsBase dependency #1818
Conversation
| @@ -1323,24 +1323,6 @@ macro tsv_str(s, flags...) | |||
| inlinetable(s, flags...; separator='\t') | |||
| end | |||
|
|
|||
| @deprecate showcols(df::AbstractDataFrame, all::Bool=false, values::Bool=true) describe(df, :eltype, :nmissing, :first, :last) | |||
| @deprecate showcols(io::IO, df::AbstractDataFrame, all::Bool=false, values::Bool=true) show(io, describe(df, :eltype, :nmissing, :first, :last), all) | |||
| function StatsBase.describe(df::AbstractDataFrame; stats=nothing) | |||
There was a problem hiding this comment.
@quinnj Thank you for the PR. It will make our life easier. The only thing I would change is leave this deprecation in line 1328 to stick for some longer time as it was introduced very recently, so some old user code might rely on it.
There was a problem hiding this comment.
@quinnj Please add back these deprecations. I see no reason to remove these, especially showcols which is likely to appear in old blog posts here and there.
There was a problem hiding this comment.
I have accepted this because the deprecations were "deprecated" long enough (but @nalimilan - I know your policy so if you really think we should revert I am OK with this). Thank you for such a careful consideration of things.
There was a problem hiding this comment.
These don't cost us anything so I don't see the point of removing them...
There was a problem hiding this comment.
I don't think it's a good policy to just keep deprecations around because they're "cheap". That leads to users disregarding deprecations because they might think the functionality will stick around forever anyway; or even worse, they won't know what is a "real" deprecation vs. something we're just trying to fade out. These deprecations have been in multiple minor releases at this point, so I see no reason to keep them around.
There was a problem hiding this comment.
Deprecations are so verbose that I really doubt users will keep using them instead of the replacement. All our deprecations are "real" and I think we've been quite clear about that. But note that there are books around that use the old APIs and we haven't even tagged a real major release, so keeping deprecations is really the minimum we can do to respect our users.
There was a problem hiding this comment.
We're still pre-1.0, so each minor release is like a major release.
There was a problem hiding this comment.
Technically yes, but they happen much more frequently than real major releases, and they aren't really advertized.
src/DataFrames.jl
Outdated
| @@ -7,7 +7,8 @@ module DataFrames | |||
| ############################################################################## | |||
|
|
|||
| using Statistics, Printf, REPL | |||
| using Reexport, StatsBase, SortingAlgorithms, Compat, Unicode, PooledArrays | |||
| using Reexport, SortingAlgorithms, Compat, Unicode, PooledArrays | |||
| using DataAPI | |||
|
Alright, rebased the PR here. Tests looked good locally, though I ran into a test/indexing crash on the Julia 1.2-rc2 (reported JuliaLang/julia#32607) |
|
Thank you - I will have a detailed look at it soon. Additionally it seems that tests also still try to reference StatsBase.jl (not sure why - probably we use some function from it for testing purposes). |
|
@quinnj in test/utils.jl you can remove |
|
Also - if I remember things correctly |
|
Updated test/utils.jl. DataAPI has a policy to specifically not export any functions, relying on extending packages to do so. Here, we're just exporting |
| ``` | ||
|
|
||
| """ | ||
| StatsBase.describe(df::AbstractDataFrame, stats::Union{Symbol, Pair{Symbol}}...) = | ||
| DataAPI.describe(io::IO, df::AbstractDataFrame, stats::Union{Symbol, Pair{Symbol}}...) = |
There was a problem hiding this comment.
Do we have to quality here and below with DataAPI. prefix? I think it can be omitted. Right?
There was a problem hiding this comment.
We can omit, but I personally like to always explicitly qualify, just to be explicit about where this method is being defined.
No description provided.