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

Several minor bugfixes #245

Merged
merged 8 commits into from
Jul 14, 2015
Merged

Several minor bugfixes #245

merged 8 commits into from
Jul 14, 2015

Conversation

desilinguist
Copy link
Member

  • Allow non-default base estimators for AdaBoost (Base estimators other than default are not supported for AdaBoost #238)
    • Correctly interpret the base_estimator fixed parameter for AdaBoostClassifiers and AdaBoostRegressors as objects instead of strings.
    • Make sure that the base estimator for Adaboost has a fixed seed so that results are replicable.
    • Add corresponding unit tests for both classification and regression and update documentation.
  • Fix hasher_features typo in experiments.py (Bug when checking if hasher_features is a valid option #234).
  • Remove unnecessary setting of numpy global random seeds (Stop modifying global numpy random seed #220)
    • We are already passing in random_state seeds in model_kwargs for all classifiers/regressors so we don't need to set the numpy global random seed.
    • Include Lasso, ElasticNet, LinearSVR, and SVC when setting the above seeds in model_kwargs.
  • Remove requirements for several learners to have dense input (Remove decision tree and random forest learners from requires_dense set #207)
    • In the new version of scikit-learn, only GradientBoostingClassifier and GradientBoostingRegressor require dense input.
    • Move make_sparse_data() to utils.py
    • Update test_sparse_predict() to include all sparse-friendly classifiers.
  • Update expected values in certain tests to account for all of the above changes.

- Correctly interpret the `base_estimator` fixed parameter for AdaBoostClassifiers and AdaBoostRegressors as objects instead of strings (#238).
- Add corresponding unit tests for both classification and regression.
- Update documentation.
- We are already passing in seeds in `model_kwargs` for all classifiers/regressors so we don't need to set the numpy global random seed.
- Include `Lasso`, `ElasticNet`, and `LinearSVR` when setting `random_state` in `model_kwargs`.
- Update AdaBoost classification test expected value that is affected by this change.
- Make sure that the base estimator for Adaboost has a fixed seed so that results are replicable.
- Update test expected value again.
- In the new version of scikit-learn, only `GradientBoostingClassifier` and `GradientBoostingRegressor` seem to require dense input.
- Move `make_sparse_data()` to `utils.py`
- Update `test_sparse_predict()` to include all sparse-friendly classifiers.
- Update any other tests' expected values.
@dan-blanchard
Copy link
Contributor

👍 I looked at this on my phone at lunch and forgot to comment. Looks good to me.

@aoifecahill
Copy link
Collaborator

Looks good to me too, thanks!

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.

None yet

3 participants