Pipesyntax

[tlienart]

closes #253

[tlienart]

Tests fail on travis due to unrelated error:

Test threw exception
  Expression: include("scitypes.jl")
  LoadError: UndefVarError: KNNRegressor not defined

This is from test/scitypes.jl

for handle in localmodels()
    name = Symbol(handle.name)
    eval(quote
         scitype(($name)())
         end)
end

Could it be an issue with a stale registry which would still contain information on the old KNNRegressor ?

[ablaom]

Well, I don't see how the registry could be stale. You can inspect it here (the registry for MLJModels@0.5.0):

https://github.com/alan-turing-institute/MLJModels.jl/blob/v0.5.0/src/registry/Metadata.toml

Can you reproduce locally? It would be helpful to see a full listing of localmodels() in the test. What else is there that shouldn't be? It's not clear why this error appears here, but not in the #dev tests. Maybe restart those tests to confirm??

[tlienart]

Ok here's more information:

• yes I can reproduce locally
• localmodels gives two different things in my tests.

Case1 fresh session, just called MLJ

julia> using MLJ
julia> localmodels()
13-element Array{NamedTuple,1}:
 (name = ConstantClassifier, package_name = MLJModels, ... )         
 (name = ConstantRegressor, package_name = MLJModels, ... )          
 (name = DecisionTreeClassifier, package_name = DecisionTree, ... )  
 (name = FeatureSelector, package_name = MLJModels, ... )            
 (name = KNNClassifier, package_name = NearestNeighbors, ... )       
 (name = KNeighborsClassifier, package_name = ScikitLearn, ... )     
 (name = LinearRegressor, package_name = GLM, ... )                  
 (name = LinearRegressor, package_name = ScikitLearn, ... )          
 (name = OneHotEncoder, package_name = MLJModels, ... )              
 (name = Standardizer, package_name = MLJModels, ... )               
 (name = StaticTransformer, package_name = MLJModels, ... )          
 (name = UnivariateBoxCoxTransformer, package_name = MLJModels, ... )
 (name = UnivariateStandardizer, package_name = MLJModels, ... )

No KNNRegressor.

Case2 fresh session then running the runtests.jl of MLJ bit by bit until failure, then running localmodels and getting

11-element Array{NamedTuple,1}:
 (name = ConstantClassifier, package_name = MLJModels, ... )         
 (name = ConstantRegressor, package_name = MLJModels, ... )          
 (name = FeatureSelector, package_name = MLJModels, ... )            
 (name = KNNRegressor, package_name = NearestNeighbors, ... )        
 (name = OneHotEncoder, package_name = MLJModels, ... )              
 (name = RidgeRegressor, package_name = MultivariateStats, ... )     
 (name = RidgeRegressor, package_name = ScikitLearn, ... )           
 (name = Standardizer, package_name = MLJModels, ... )               
 (name = StaticTransformer, package_name = MLJModels, ... )          
 (name = UnivariateBoxCoxTransformer, package_name = MLJModels, ... )
 (name = UnivariateStandardizer, package_name = MLJModels, ... )

There's something fishy going on but not sure what might have caused it.

[tlienart]

OH, I think I know, the problem is that operations.jl calls two models, which are then loaded and pollute the environment.

I think the answer is simply to put the operations tests after scitype ones.

[ablaom]

Okay. I see a problem with the test. My bad.

By default localmodels returns the local models in Main (So KNNRegresssor appears because it got loaded into Main in some other test, I guess.). The call in the test should probably be localmodels(modl=TestScitypes) (or the eval should be Main.eval).

Side note: We should add a macro version @localmodels that always uses the calling module:

macro localmodels()
    quote 
        localmodels(modl=__module__)
    end
end

[tlienart]

yeah so wrapping the tests in a module fix the localmodels() being polluted.

The last commit should pass and this should be mergeable then.

[codecov-io]

Codecov Report

Merging #263 into dev will decrease coverage by 4.32%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev     #263      +/-   ##
==========================================
- Coverage   80.77%   76.45%   -4.33%     
==========================================
  Files          15       15              
  Lines        1139     1206      +67     
==========================================
+ Hits          920      922       +2     
- Misses        219      284      +65

Impacted Files	Coverage Δ
src/MLJ.jl	100% <ø> (ø)	⬆️
src/operations.jl	54.54% <ø> (+9.09%)	⬆️
src/machines.jl	82.69% <0%> (-12.87%)	⬇️
src/resampling.jl	77.39% <0%> (-10.2%)	⬇️
src/pipelines.jl	90.29% <0%> (-7.61%)	⬇️
src/tuning.jl	84.82% <0%> (-7.42%)	⬇️
src/ensembles.jl	79.12% <0%> (-4.61%)	⬇️
src/parameters.jl	92.15% <0%> (-3.77%)	⬇️
src/composites.jl	92.89% <0%> (-3.7%)	⬇️
src/utilities.jl	86.3% <0%> (-2.44%)	⬇️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2321444...ac13a53. Read the comment docs.

[tlienart]

Hmm it's only the documentation step that fails... well that's a bit frustrating: https://travis-ci.com/alan-turing-institute/MLJ.jl/builds/131220257

Looks like the Project.toml in the docs is incorrect and clashes, due to the decision tree issue, will block compat to 0.5 and it should be ok

[tlienart]

Good, will merge as the content of the PR was approved before, modifications beyond that are just to make the tests pass on all Julia versions including nightly.