Improve tests #28
Improve tests #28
Conversation
Running tests for each package separately isn't very useful as they run their tests already, and it hits timeout given how long this takes. What we would ideally do is run them in the same process after loading StatsKit, but that's currently not possible. Testing that reexxported packages do not have conflicting identifiers seems more useful.
| exceptions = [:ci, :head, :lead, :predict, :predict, :rename!, :rename, :msd, :evaluate, | ||
| :rmsd, :ci, :rename!, :lag, :lag, :tail, :colwise, :colwise, :head, :meanad, | ||
| :msd, :transform, :tail, :rmsd, :rename, :transform, :evaluate, :lead] |
There was a problem hiding this comment.
So it turns out that this test was more interesting than I thought! It would be worth running it on all JuliaStats and JuliaData packages. Conflicts seem to indicate that we should really create a StatsAPI package so that we override common definitions of standard methods.
See #22 (comment) about conflicts between ShiftedArrays and TimeSeries (since the addition of ShiftedArrays hasn't be released yet).
Conflicts between StatsBase and Distances should probably be resolved by deprecating the StatsBase functions in favor of those in Distances, since the latter is more specialized and more lightweight.
conflicts = Dict{Any,Symbol}(TimeSeries.rename => :rename,DataFrames.rename => :rename)
conflicts = Dict{Any,Symbol}(ShiftedArrays.lead => :lead,TimeSeries.lead => :lead)
conflicts = Dict{Any,Symbol}(Loess.predict => :predict,StatsBase.predict => :predict)
conflicts = Dict{Any,Symbol}(HypothesisTests.ci => :ci,Bootstrap.ci => :ci)
conflicts = Dict{Any,Symbol}(StatsBase.meanad => :meanad,Distances.meanad => :meanad)
conflicts = Dict{Any,Symbol}(HypothesisTests.ci => :ci,Bootstrap.ci => :ci)
conflicts = Dict{Any,Symbol}(Distances.evaluate => :evaluate,MultivariateStats.evaluate => :evaluate)
conflicts = Dict{Any,Symbol}(ShiftedArrays.lag => :lag,TimeSeries.lag => :lag)
conflicts = Dict{Any,Symbol}(StatsBase.msd => :msd,Distances.msd => :msd)
conflicts = Dict{Any,Symbol}(StatsBase.msd => :msd,Distances.msd => :msd)
conflicts = Dict{Any,Symbol}(StatsBase.rmsd => :rmsd,Distances.rmsd => :rmsd)
conflicts = Dict{Any,Symbol}(TimeSeries.rename => :rename,DataFrames.rename => :rename)
conflicts = Dict{Any,Symbol}(ShiftedArrays.lag => :lag,TimeSeries.lag => :lag)
conflicts = Dict{Any,Symbol}(TimeSeries.rename! => :rename!,DataFrames.rename! => :rename!)
conflicts = Dict{Any,Symbol}(Distances.evaluate => :evaluate,MultivariateStats.evaluate => :evaluate)
conflicts = Dict{Any,Symbol}(StatsBase.meanad => :meanad,Distances.meanad => :meanad)
conflicts = Dict{Any,Symbol}(DataFrames.transform => :transform,MultivariateStats.transform => :transform)
conflicts = Dict{Any,Symbol}(StatsBase.rmsd => :rmsd,Distances.rmsd => :rmsd)
conflicts = Dict{Any,Symbol}(DataFrames.transform => :transform,MultivariateStats.transform => :transform)
conflicts = Dict{Any,Symbol}(TimeSeries.rename! => :rename!,DataFrames.rename! => :rename!)
conflicts = Dict{Any,Symbol}(Loess.predict => :predict,StatsBase.predict => :predict)
conflicts = Dict{Any,Symbol}(ShiftedArrays.lead => :lead,TimeSeries.lead => :lead)
There was a problem hiding this comment.
Yes - this is super useful. StatsAPI.jl is needed. But I guess this should not delay merging this PR. Right?
There was a problem hiding this comment.
Yes, this PR ensures we catch new conflicts so better merge it.
| # JuliaStats/StatsModels.jl#218, | ||
| # JuliaStats/MultivariateStats.jl#142, | ||
| # JuliaArrays/ShiftedArrays.jl#42, | ||
| # JuliaStats/TimeSeries.jl#487 |
There was a problem hiding this comment.
That PR has been merged already JuliaStats/TimeSeries.jl#487
| # JuliaStats/TimeSeries.jl#487 |
There was a problem hiding this comment.
Indeed. I've pushed a commit to remove a few functions which have been fixed since then.
There was a problem hiding this comment.
Actually, it appears that Julia 1.0 still uses old versions of most of these packages (because of some dependencies?). Let's keep this for now, once the new LTS is out we'll probably drop support for 1.0.
This reverts commit f7cd74a.
Running tests for each package separately isn't very useful as they run their tests already, and it hits timeout given how long this takes.
What we would ideally do is run them in the same process after loading StatsKit, but that's currently not possible.
Testing that reexxported packages do not have conflicting identifiers seems more useful.
This depends on #27.