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

Fix some warnings in base tests #1228

Merged
merged 1 commit into from
Sep 9, 2016
Merged

Conversation

ja-thomas
Copy link
Contributor

So there seem to occur some warnings in the base tests when running them locally

Warnings -----------------------------------------------------------------------
1. getParamSet (@test_base_getParamSet.R#12) - 'setId' is deprecated.
Use 'setLearnerId' instead.
See help("Deprecated")

2. makeLearners (@test_base_makeLearners.R#9) - 'function (learner, id) 
{
    .Deprecated("setLearnerId")
    learner = checkLearner(learner)
    assertString(id)
    learner$id = id
    return(learner)
}' is deprecated.
Use 'setLearnerId' instead.
See help("Deprecated")

3. makeLearners (@test_base_makeLearners.R#9) - 'function (learner, id) 
{
    .Deprecated("setLearnerId")
    learner = checkLearner(learner)
    assertString(id)
    learner$id = id
    return(learner)
}' is deprecated.
Use 'setLearnerId' instead.
See help("Deprecated")

4. check measure calculations (@test_base_measures.R#258) - MAPE is undefined if any truth value is equal to 0.

5. check measure calculations (@test_base_measures.R#259) - MAPE is undefined if any truth value is equal to 0.

6. measures quickcheck (@test_base_measures.R#659) - NAs produced by integer overflow

1 - 3 are just deprecated calls to getId

4 - 5 added supress warning test to check the actual value (NA) as well as an test for the warning message.

6 I have no idea why this is failing, seems to be consistently failing when running rcheck but works when running the chunk of code by hand.

@bhvieira
Copy link
Contributor

bhvieira commented Sep 9, 2016

About 4-5 please look at #1211 so we keep it unified.

@larskotthoff
Copy link
Member

Why don't we see this in Travis or Jenkins?

@berndbischl
Copy link
Member

Why don't we see this in Travis or Jenkins?

i am pretty sure we DO see them. its just that we dont look at it.
this was the reason why i always wanted to escalate warning to errors.
but this then sometimes produced false positives.....

i still think that such an escalation would be ideal.

@larskotthoff
Copy link
Member

There's nothing in the output (and I think @ja-thomas is saying that you see this in the output, even if the build doesn't fail).

@berndbischl
Copy link
Member

There's nothing in the output (and I think @ja-thomas is saying that you see this in the output, even if the build doesn't fail).

its at least visible when you run the tests locally.
it think the difference is this:
locally we run with rtest, on travis it is run inside of R CMD check. there you dont see the warnings.

@larskotthoff
Copy link
Member

Ok, but what does that do differently, i.e. how can we make those warnings show up?

@berndbischl
Copy link
Member

berndbischl commented Sep 9, 2016

Ok, but what does that do differently,

well, rtest just calls testthat. r cmd check calls r cmd check.

Ok, but what does that do differently, i.e. how can we make those warnings show up?

i dont think you can teach r cmd check to output more.
this is why i said i would like to escalate warning.

@bhvieira
Copy link
Contributor

bhvieira commented Sep 9, 2016

Using suppressWarnings has the undesirable side effect that if a warning happens due to other reasons than the one you were expecting you won't see it.

@berndbischl
Copy link
Member

Using suppressWarnings has the undesirable side effect that if a warning happens due to other reasons than the one you were expecting you won't see it.

thats not what we are discussing here. there are quite a few places in our tests where we produce (undesired) warnings. which are not suppressed. they are thrown because our tests or code is imperfect.
rtest shows these. r cmd doesnt

SteveBronder pushed a commit to SteveBronder/mlr that referenced this pull request Oct 12, 2016
fixed broken link in README

updated NEWS for PR#1026

remove .S3methods import (mlr-org#1216)

update auto-generated documentation [ci skip]

removed unused variables (mlr-org#1215)

updated README

benchmark: better arg handling (mlr-org#1224)

NEWS

xgboost: better handling of arg 'missing' (mlr-org#1225)

set default of shw.missing.values to TRUE (mlr-org#1223)

* set default to FALSE, identical to ParamHelpers

* TRUE TRUE TRUE

remove deprecated call and catch warning in mape (mlr-org#1228)

fix mlr-org#804 replace preproc with imputed (mlr-org#1231)

NEWS

NEWS: are OK until HERE

xgboost: missing: go back to set it NA in mlr

xgboost: missing: simply use NULL as default

fix xgboost tests (mlr-org#1234)

* fix xgboost tests

* fix more tests

test for xgboost printer

Add support for visualizing tasks with 2 or more hyperparameters (mlr-org#1233)

* Add support for visualizing tasks with 2 or more hyperparameters

* Add tests for partial dependence

* Edit documentation

* Forgot to regenerate documentation

* Fixed checks for using partial dependence and minor style fixes

* Fix typos in argname

* Fix arg name in test

NEWS for mlr-org#1233

remove weight.fun in place of expanded fun in generatePartialDependence (mlr-org#1235)

* remove weight.fun in place of expanded fun in generatePartialDependence

 - internal wrapper for fun arg to allow passing of internal
   newdata (prediction grid) and data (training data from input arg)
   which allows computation of weights in fun instead of via an extra
   step using another arg, weight.fun (now removed)

* fix typo

NEWS for mlr-org#1235

update auto-generated documentation [ci skip]

Update description with mason (mlr-org#1237)

travis does not work with rdevel, i will open an issue

Added ctb (mlr-org#1242)

* Added Bruno Vieira as ctb.

* Added Bruno Vieira as ctb.

fixes for issue mlr-org#63 in the tutorial (mlr-org#1243)

- incorrect jacobian function in doPartialDerivativeIteratoin
 - improper fun/fun.wrapper (for weights use)
 - test added based on tutorial fail
 - simplified code a bit

renamed file for consistency

update auto-generated documentation [ci skip]

added the colsample_bylevel parameter in the xgboost learners (mlr-org#1245)

* Update RLearner_classif_xgboost.R

* Update RLearner_regr_xgboost.R

NEWS for mlr-org#1245 and add xgboost version number requirements

forgot space...

ksvm mini tunable fix for hyper par settings (mlr-org#1249)

New measures: Cohen's Kappa and Mean Quadratic Weighted Kappa (mlr-org#1250)

* new measure 'mean quadratic weighted kappa'

* add note for mqwk

* rename objects in test

* rename to wk and fix typo in note

* yet another typo

* rename wk to wkappa

* new_measure_cohens_kappa

* correct measure ranges

NEWS for mlr-org#1250

fixed broken url

listLearners output as S3 class with print (mlr-org#1213)

Make hyperparseffect tests faster with less iterations (mlr-org#1260)

Created TimeRegrTask and started on Arima Learner

Added ARMA learner. For now, allowing cl on line 92 of predictLearner (checkPredictLearnerOutput) to be a ts object

Predict added for Arima.

Prediction now returns the response, but the 'truth' variables is NA, since forecasts do not know the true value at the time of forecast

Added new forecast function. Need to figure out why Arima and forecast are not going to the namespace.

Fixed forecast to use holdout set, made mase measure

Updated namespace to import forecast, then use the method for WrappedModel. Dunno if this meeses with forecast() in the forecast package.

Created Windowing description functions and starting adding Windowing instances

Created fixed and growing window instances, may not work for horizon > 1.

Added window() function, mostly copying resample(). Need to add functions for windowing with aggregation.

Added window level to zzz.R, Created checkAggrBeforeWindow function, WindowPrediction, makeWindowPrediction. Fixed growing and fixed windows by using code from caret.

Windowing works for arima, should probably do something about n.ahead and horizon being the same thing.

Imported forecast to resample, no longer need forecast functions or windows

Removed window and forecast functions, removed window from zzz levels

Added skip parameter to growing CV and fixed CV so user does not have to run every iteration.

had to capitalize L in makeRLearner for Arima

Added docs for time components in resample and resampleDesc

Added imports from xts and zoo. Added xreg to Arima.

Added Lag and Difference preprocess wrapper.

Made createLagDiffFeatures a task preprocessor.

Changed names of timeReg to ForecastRegr and timeregr to fcregr.

Testing

Making sure rebase worked.

Updates now pass base tests

Updated prediction from timereg to forecastreg. Updated README with some examples of using forecasting.

Trying to upload caret picture for windowing.

Updated readme with examples.

Updated readme.

Fixed createLagDiffFeatures. But NA's are handled poorly.

added bats, ets, garch, nnetr, tbats, and thief. Not tested yet, but garch works.

garch now works for resampling.

bats, ets, garch, nnetar, tbats are now working. Updated Readme. thief is not working (frowny face)

Made pre processing wrapper using LambertW transform

Added LambertW to description suggests and updated the readme.

Updated lag and diff preproc func for seasonal lag and differences. Untested.

Updated lag and diff preproc to have seasonal lags and diffs.

Fixed lag diff preproc to include padding and lag lengths for differencing.

Updated docs for createLagDiffFeatures

Added forecast helper objects and started working on unit test for Arima.

Created TimeRegrTask and started on Arima Learner

Added ARMA learner. For now, allowing cl on line 92 of predictLearner (checkPredictLearnerOutput) to be a ts object

Predict added for Arima.

Prediction now returns the response, but the 'truth' variables is NA, since forecasts do not know the true value at the time of forecast

Added new forecast function. Need to figure out why Arima and forecast are not going to the namespace.

Fixed forecast to use holdout set, made mase measure

Updated namespace to import forecast, then use the method for WrappedModel. Dunno if this meeses with forecast() in the forecast package.

Created Windowing description functions and starting adding Windowing instances

Created fixed and growing window instances, may not work for horizon > 1.

Added window() function, mostly copying resample(). Need to add functions for windowing with aggregation.

Added window level to zzz.R, Created checkAggrBeforeWindow function, WindowPrediction, makeWindowPrediction. Fixed growing and fixed windows by using code from caret.

Windowing works for arima, should probably do something about n.ahead and horizon being the same thing.

Imported forecast to resample, no longer need forecast functions or windows

Removed window and forecast functions, removed window from zzz levels

Added skip parameter to growing CV and fixed CV so user does not have to run every iteration.

had to capitalize L in makeRLearner for Arima

Added docs for time components in resample and resampleDesc

Added imports from xts and zoo. Added xreg to Arima.

Added Lag and Difference preprocess wrapper.

Made createLagDiffFeatures a task preprocessor.

Changed names of timeReg to ForecastRegr and timeregr to fcregr.

Testing

Making sure rebase worked.

Updates now pass base tests

Updated prediction from timereg to forecastreg. Updated README with some examples of using forecasting.

Trying to upload caret picture for windowing.

Updated readme with examples.

Updated readme.

Fixed createLagDiffFeatures. But NA's are handled poorly.

added bats, ets, garch, nnetr, tbats, and thief. Not tested yet, but garch works.

garch now works for resampling.

bats, ets, garch, nnetar, tbats are now working. Updated Readme. thief is not working (frowny face)

Added LambertW to description suggests and updated the readme.

Updated lag and diff preproc func for seasonal lag and differences. Untested.

Updated lag and diff preproc to have seasonal lags and diffs.

Fixed lag diff preproc to include padding and lag lengths for differencing.

Updated docs for createLagDiffFeatures

Updated merge for Arima prediction.

Fixed training for fcregr tasks to only use subsets.

Moved test for bats to testthat.

Added tests for tbats and ets

Added garch unit test.

Added test for createLagDiffFeatures

Added helper objects for forecast unit testing and Arima can now return standard errors at set levels

fixed typo in garch test

Moved thief to to-do and implimented arfima with a test.

Added se prediction type to arfima, bats, ets, nnetar, and tbats

Added updateLearner function and updateModel function to update online models.

Added docs for updateModel and built basic test for forecast task. Need to test multiplexer.

Fixed Lambert W and created test for forecast
@schiffner schiffner deleted the fix_some_warnings_in_base_tests branch October 12, 2016 11:51
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.

4 participants