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 support scalar values in fit_params in SearchCV #15863

Merged
merged 35 commits into from Dec 31, 2019

Conversation

adrinjalali
Copy link
Member

@adrinjalali adrinjalali commented Dec 11, 2019

Fixes #15805

Add back support for scalar values in SearchCV and cross-validation helpers.

@adrinjalali adrinjalali added this to the 0.22.1 milestone Dec 11, 2019
@adrinjalali
Copy link
Member Author

@adrinjalali adrinjalali commented Dec 11, 2019

@adrinjalali adrinjalali changed the title in GridSearch support a scalar fit param FIX support a scalar fit param in GridSearch Dec 11, 2019
sklearn/model_selection/_search.py Outdated Show resolved Hide resolved
sklearn/model_selection/_search.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_search.py Outdated Show resolved Hide resolved
sklearn/model_selection/tests/test_search.py Outdated Show resolved Hide resolved
Copy link
Member

@jnothman jnothman left a comment

Sorry, pressed the wrong button

@adrinjalali
Copy link
Member Author

@adrinjalali adrinjalali commented Dec 12, 2019

Thinking a bit more about this, isn't it the case that we don't validate/process fit params in most meta-estimators? I'm happy to just remove those two lines and have it as it was.

@jnothman
Copy link
Member

@jnothman jnothman commented Dec 12, 2019

Remove the indexable, okay, but then we need to ensure __array_function__ is not called later.

@adrinjalali
Copy link
Member Author

@adrinjalali adrinjalali commented Dec 14, 2019

But why should that be guaranteed in the GS? Shouldn't that logic be in individual estimators?

@jnothman
Copy link
Member

@jnothman jnothman commented Dec 14, 2019

Because cv processes the fit params. If you can get away with not modifying it, great

@glemaitre glemaitre self-assigned this Dec 23, 2019
doc/whats_new/v0.22.rst Outdated Show resolved Hide resolved
doc/whats_new/v0.22.rst Outdated Show resolved Hide resolved
Copy link
Member

@ogrisel ogrisel left a comment

First batch of comments:

sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
@glemaitre glemaitre changed the title FIX support a scalar fit param in GridSearch FIX support scalar values in fit_params in SearchCV Dec 23, 2019
@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Dec 23, 2019

I think this is ready for a review

@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Dec 24, 2019

I do not see how this can be caused by your lightgbm related changes though.

lightgbm have a dependence on scikit-learn. So I assume that we were installing another 0.22 (or 0.21.3) and the dev as well and collided badly. Now, I just move to install lightgbm without the dependence

@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Dec 24, 2019

I used the build which already installs pandas and pyamg.

@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Dec 24, 2019

Not sure why codecov still complain because I can see that the tests are not skipped anymore

@ogrisel
Copy link
Member

@ogrisel ogrisel commented Dec 24, 2019

The code coverage is of the patch is not 100% because the codecov upload does not happen for this job:

https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=11576&view=logs&j=78a0bf4f-79e5-5387-94ec-13e67d216d6e&t=80793e4d-ec39-5212-d625-56647bcbed6e

COVERAGE="true" is not set.

However this job is already the longest, so I am not sure we should enable it.

All the tests from the sklearn/model_selection/tests/test_search.py file pass.

@jnothman
Copy link
Member

@jnothman jnothman commented Dec 26, 2019

Do we really need to have a test requiring lgbm? I think it would be much more explicit to define a fake estimator that accepts and stores its fit_params about which assertions can then be made

Copy link
Member Author

@adrinjalali adrinjalali left a comment

I also agree with Joel on the testing. Sorry if I've missed an already discussed discussion. And thanks for the work @glemaitre :)

Object to be converted to an indexable iterable.
"""
if sp.issparse(iterable):
return iterable.tocsr()
Copy link
Member Author

@adrinjalali adrinjalali Dec 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have to convert to csr? I think if the estimator needs to convert the param, they'll do it themselves.

Copy link
Contributor

@glemaitre glemaitre Dec 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For backward-compatibility only (we were doing it before). I assume that csr would be a good default since we are converting to csr when the number of samples in the arrays are the same than in X meaning that we should be efficient taking rows.

"""
if sp.issparse(iterable):
return iterable.tocsr()
elif hasattr(iterable, "__getitem__") or hasattr(iterable, "iloc"):
Copy link
Member Author

@adrinjalali adrinjalali Dec 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if I've missed something, but if the point is not to pass anything which implements __array_function__, shouldn't we test for that instead? An object may implement that protocol and implement __getitem__, can it not?

Copy link
Contributor

@glemaitre glemaitre Dec 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you mean something like:

if sp.sparse(iterable):
    # efficient indexing per rows
    return iterable.csr()
elif hasattr(iterable, "iloc"):
    # pandas series or dataframe
    return iterable
elif hasattr(iterable, "__array_function__"):
    # do not rely on array protocol
    return np.asarray(iterable)
elif hasattr(iterable, "__getitem__"):
    return iterable
return np.asarray(iterable)

Copy link
Member Author

@adrinjalali adrinjalali Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this looks better. I'd put the __array_function__ condition on top or right after sp.issparse though.

fit_params_validated = {}
for param_key, param_value in fit_params.items():
if (not _is_arraylike(param_value) or
_num_samples(param_value) != _num_samples(X)):
Copy link
Member Author

@adrinjalali adrinjalali Dec 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we also support non-sample-aligned fit params? I though we wanted to support only the scalars for now.

Also, shouldn't we pass all non-scalars to _make_indexable?

Copy link
Contributor

@glemaitre glemaitre Dec 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we also support non-sample-aligned fit params

At least it seems we were supporting it. This is the code used within the cross-validation originally.

Copy link
Member

@jnothman jnothman Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we also support non-sample-aligned fit params? I though we wanted to support only the scalars for now.

No, we want to be backward compatible, which means supporting non-sample-aligned fit params

Copy link
Member Author

@adrinjalali adrinjalali Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but we should still make sure the non-sample-aligned params are also not implementing __array_function__, I think.

Copy link
Member

@jnothman jnothman Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let the downstream estimator deal with it for unaligned parameters I reckon. Pass them untouched

Copy link
Member

@ogrisel ogrisel Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this specific PR I think we should just restore compatibility with the prior behavior.

For the future API, I am not so sure but I think I would also be in favor of passing the params untouched?

@@ -212,6 +212,26 @@ def check_consistent_length(*arrays):
" samples: %r" % [int(l) for l in lengths])


def _make_indexable(iterable):
Copy link
Member

@jnothman jnothman Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't have two functions indexable and _make_indexable, especially not if both are used outside of this module.

Keep indexable as the only API and make this local to indexable, e.g. def _check(iterable):

Copy link
Contributor

@glemaitre glemaitre Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK but how do I reuse _check in _check_fit_params. The idea was to avoid code duplication in indexable and _check_fit_params.

fit_params_validated = {}
for param_key, param_value in fit_params.items():
if (not _is_arraylike(param_value) or
_num_samples(param_value) != _num_samples(X)):
Copy link
Member

@jnothman jnothman Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we also support non-sample-aligned fit params? I though we wanted to support only the scalars for now.

No, we want to be backward compatible, which means supporting non-sample-aligned fit params

sklearn/utils/validation.py Outdated Show resolved Hide resolved
sklearn/utils/validation.py Outdated Show resolved Hide resolved
Copy link
Member

@qinhanmin2014 qinhanmin2014 left a comment

Considering the popularity of scikit-learn and the popularity of some third party libraries (e.g., xgboost, lightgbm, catboost), perhaps we should be more tolerant.

..............................

- |Fix| :class:`model_selection.GridSearchCV` and
:class:`model_selection.RandomizedSearchCV` accept scalar values provided in
Copy link
Member

@qinhanmin2014 qinhanmin2014 Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do other things such as callable belong to "scalar values"?

Copy link
Member

@jnothman jnothman Dec 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the change log entry understates the change, but that's okay, as this is relatively readable.

[(GridSearchCV, {'learning_rate': [0.1, 0.01]}),
(RandomizedSearchCV, {'learning_rate': uniform(0.01, 0.1)})]
)
def test_scalar_fit_param_lightgbm(metric, SearchCV, param_search):
Copy link
Member

@qinhanmin2014 qinhanmin2014 Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can simply remove this test (and lightgbm dependency)

Copy link
Contributor

@glemaitre glemaitre Dec 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No because we already use lightgbm for some tests in HistGradientBoosting*and this could be handy to detect any breaking change.

Copy link
Member

@jnothman jnothman Dec 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree. I don't think lightgbm happening to be on the receiving end of these fit_params tells us anything more. If we want to test for breaking changes with external software, we or they need to develop extensive integration test suites. Here we are trying to test that a specific feature of scikit-learn works as advertised, towards which the specific behaviours of lightgbm are irrelevant.
The use of lightgbm in HistGradientBoosting* estimators is specifically necessitated and does not set precedent for its inclusion here, IMO.

@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Dec 27, 2019

Considering the popularity of scikit-learn and the popularity of some third party libraries (e.g., xgboost, lightgbm, catboost), perhaps we should be more tolerant.

I don't think this is that simple. Changes in our (developer) API to accommodate different use-cases should also be discussed to assess the implications. We should not make changes solely on the basis that a third-party library is popular.

Copy link
Member

@qinhanmin2014 qinhanmin2014 left a comment

@glemaitre how about this comment: #15863 (comment)
Apart from this, LGTM

@jnothman
Copy link
Member

@jnothman jnothman commented Dec 29, 2019

@ogrisel
Copy link
Member

@ogrisel ogrisel commented Dec 30, 2019

I have opened a dedicated issue for Continuous Integration testing for downstream projects: #15992

@qinhanmin2014
Copy link
Member

@qinhanmin2014 qinhanmin2014 commented Dec 31, 2019

Let's remove the lgb test and merge?
Or merge directly?

@ogrisel
Copy link
Member

@ogrisel ogrisel commented Dec 31, 2019

Let's remove the lgb test and merge?
Or merge directly?

Let me write a replacement test that does not require LightGBM.

@ogrisel ogrisel merged commit 46001e7 into scikit-learn:master Dec 31, 2019
21 checks passed
@qinhanmin2014
Copy link
Member

@qinhanmin2014 qinhanmin2014 commented Dec 31, 2019

@ogrisel Do we still need to change azure-pipelines.yml and build_tools/azure/install.sh?

@ogrisel
Copy link
Member

@ogrisel ogrisel commented Dec 31, 2019

I believe this is useful for the hist gradient boosting tests that actually benefit from being executed against lightgbm on the CI.

ogrisel added a commit to ogrisel/scikit-learn that referenced this issue Dec 31, 2019
* support a scalar fit param

* pep8

* TST add test for desired behavior

* FIX introduce _check_fit_params to validate parameters

* DOC update whats new

* TST tests both grid-search and randomize-search

* PEP8

* DOC revert unecessary change

* TST add test for _check_fit_params

* olivier comments

* TST fixes

* DOC whats new

* DOC whats new

* TST revert type of error

* add olivier suggestions

* address olivier comments

* address thomas comments

* PEP8

* comments olivier

* TST fix test by passing X

* avoid to call twice tocsr

* add case column/row sparse in check_fit_param

* provide optional indices

* TST check content when indexing params

* PEP8

* TST update tests to check identity

* stupid fix

* use a distribution in RandomizedSearchCV

* MNT add lightgbm to one of the CI build

* move to another build

* do not install dependencies lightgbm

* MNT comments on the CI setup

* address some comments

* Test fit_params compat without dependency on lightgbm

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
ogrisel added a commit to ogrisel/scikit-learn that referenced this issue Jan 2, 2020
* support a scalar fit param

* pep8

* TST add test for desired behavior

* FIX introduce _check_fit_params to validate parameters

* DOC update whats new

* TST tests both grid-search and randomize-search

* PEP8

* DOC revert unecessary change

* TST add test for _check_fit_params

* olivier comments

* TST fixes

* DOC whats new

* DOC whats new

* TST revert type of error

* add olivier suggestions

* address olivier comments

* address thomas comments

* PEP8

* comments olivier

* TST fix test by passing X

* avoid to call twice tocsr

* add case column/row sparse in check_fit_param

* provide optional indices

* TST check content when indexing params

* PEP8

* TST update tests to check identity

* stupid fix

* use a distribution in RandomizedSearchCV

* MNT add lightgbm to one of the CI build

* move to another build

* do not install dependencies lightgbm

* MNT comments on the CI setup

* address some comments

* Test fit_params compat without dependency on lightgbm

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
ogrisel added a commit that referenced this issue Jan 2, 2020
* DOC fixed default values in dbscan (#15753)

* DOC fix incorrect branch reference in contributing doc (#15779)

* DOC relabel Feature -> Efficiency in change log (#15770)

* DOC fixed Birch default value (#15780)

* STY Minior change on code padding in website theme (#15768)

* DOC Fix yticklabels order in permutation importances example (#15799)

* Fix yticklabels order in permutation importances example

* STY Update wrapper width (#15793)

* DOC Long sentence was hard to parse and ambiguous in _classification.py (#15769)

* DOC Removed duplicate 'classes_' attribute in Naive Bayes classifiers (#15811)

* BUG Fixes pandas dataframe bug with boolean dtypes (#15797)

* BUG Returns only public estimators in all_estimators (#15380)

* DOC improve doc for multiclass and types_of_target (#15333)

* TST Increases tol for check_pca_float_dtype_preservation assertion (#15775)

* update _alpha_grid class in _coordinate_descent.py (#15835)

* FIX Explicit conversion of ndarray to object dtype. (#15832)

* BLD Parallelize sphinx builds on circle ci (#15745)

* DOC correct url for preprocessing (#15853)

* MNT avoid generating too many cross links in examples (#15844)

* DOC Correct wrong doc in precision_recall_fscore_support (#15833)

* DOC add comment in check_pca_float_dtype_preservation (#15819)

Documenting the changes in #15775

* DOC correct indents in docstring _split.py (#15843)

* DOC fix docstring of KMeans based on sklearn guideline (#15754)

* DOC fix docstring of AgglomerativeClustering based on sklearn guideline (#15764)

* DOC fix docstring of AffinityPropagation based on sklearn guideline (#15777)

* DOC fixed SpectralCoclustering and SpectralBiclustering docstrings following sklearn guideline (#15778)

* DOC fix FeatureAgglomeration and MiniBatchKMeans docstring following sklearn guideline (#15809)

* TST Specify random_state in test_cv_iterable_wrapper (#15829)

* DOC Include LinearSV{C, R} in models that support sample_weights (#15871)

* DOC correct some indents (#15875)

* DOC Fix documentation of default values in tree classes (#15870)

* DOC fix typo in docstring (#15887)

* DOC FIX default value for xticks_rotation in plot_confusion_matrix (#15890)

* Fix imports in pip3 ubuntu by suffixing affected files (#15891)

* MNT Raise erorr when normalize is invalid in confusion_matrix (#15888)

* [MRG] DOC Increases search results for API object results (#15574)

* MNT Ignores warning in pyamg for deprecated scipy.random (#15914)

* DOC Instructions to troubleshoot Windows path length limit (#15916)

* DOC add versionadded directive to some estimators (#15849)

* DOC clarify doc-string of roc_auc_score and add references (#15293)

* MNT Adds skip lint to azure pipeline CI (#15904)

* BLD Fixes bug when building with NO_MATHJAX=1 (#15892)

* [MRG] BUG Checks to number of axes in passed in ax more generically (#15760)

* EXA Minor fixes in plot_sparse_logistic_regression_20newsgroups.py (#15925)

* BUG Do not shadow public functions with deprecated modules (#15846)

* Import sklearn._distributor_init first (#15929)

* DOC Fix typos, via a Levenshtein-style corrector (#15923)

* DOC in canned comment, mention that PR title becomes commit me… (#15935)

* DOC/EXA Correct spelling of "Classification" (#15938)

* BUG fix pip3 ubuntu update by suffixing file (#15928)

* [MRG] Ways to compute center_shift_total were different in "full" and "elkan" algorithms. (#15930)

* TST Fixes integer test for train and test indices (#15941)

* BUG ensure that parallel/sequential give the same permutation importances (#15933)

* Formatting fixes in changelog (#15944)

* MRG FIX: order of values of self.quantiles_ in QuantileTransformer (#15751)

* [MRG] BUG Fixes constrast in plot_confusion_matrix (#15936)

* BUG use zero_division argument in classification_report (#15879)

* DOC change logreg solver in plot_logistic_path (#15927)

* DOC fix whats new ordering (#15961)

* COSMIT use np.iinfo to define the max int32 (#15960)

* DOC Apply numpydoc validation to VotingRegressor methods (#15969)

Co-authored-by: Tiffany R. Williams <Tiffany8@users.noreply.github.com>

* DOC improve naive_bayes.py documentation (#15943)

Co-authored-by: Jigna Panchal <40188288+jigna-panchal@users.noreply.github.com>

* DOC Fix default values in Perceptron documentation (#15965)

* DOC Improve default values in logistic documentation (#15966)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

* DOC Improve documentation of default values for imputers (#15964)

* EXA/MAINT Simplify code in manifold learning example (#15949)

* DOC Improve default values in SGD documentation (#15967)

* DOC Improve defaults in neural network documentation (#15968)

* FIX use safe_sparse_dot for callable kernel in LabelSpreading (#15868)

* BUG Adds attributes back to check_is_fitted (#15947)

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* DOC update check_is_fitted what's new

* DOC change python-devel to python3-devel for yum. (#15986)

* DOC Correct the default value of values_format in plot_confusion_matrix (#15981)

* [MRG] MNT Updates pypy to use 7.2.0 (#15954)

* FIX Add missing 'values_format' param to disp.plot() in plot_confusion_matrix (#15937)

* FIX support scalar values in fit_params in SearchCV (#15863)

* support a scalar fit param

* pep8

* TST add test for desired behavior

* FIX introduce _check_fit_params to validate parameters

* DOC update whats new

* TST tests both grid-search and randomize-search

* PEP8

* DOC revert unecessary change

* TST add test for _check_fit_params

* olivier comments

* TST fixes

* DOC whats new

* DOC whats new

* TST revert type of error

* add olivier suggestions

* address olivier comments

* address thomas comments

* PEP8

* comments olivier

* TST fix test by passing X

* avoid to call twice tocsr

* add case column/row sparse in check_fit_param

* provide optional indices

* TST check content when indexing params

* PEP8

* TST update tests to check identity

* stupid fix

* use a distribution in RandomizedSearchCV

* MNT add lightgbm to one of the CI build

* move to another build

* do not install dependencies lightgbm

* MNT comments on the CI setup

* address some comments

* Test fit_params compat without dependency on lightgbm

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* Remove abstractmethod that silently brake downstream packages (#15996)

* FIX restore BaseNB._check_X without abstractmethod decoration (#15997)

* Update v0.22 changelog for 0.22.1 (#16002)

- set the date
- move entry for quantile transformer to the 0.22.1 section
- fix alphabetical ordering of modules

* STY Removes hidden scroll bar (#15999)

* Flake8 fixes

* Fix: remove left-over lines that should have been deleted during conflict resolution when rebasing

* Fix missing imports

* Update version

* Fix test_check_is_fitted

* Make test_sag_regressor_computed_correctly deterministic (#16003)

Fix #15818.

Co-authored-by: cgsavard <claire.savard@colorado.edu>
Co-authored-by: Joel Nothman <joel.nothman@gmail.com>
Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com>
Co-authored-by: Matt Hall <matt@agilegeoscience.com>
Co-authored-by: Kathryn Poole <kathryn.poole2@gmail.com>
Co-authored-by: lucyleeow <jliu176@gmail.com>
Co-authored-by: JJmistry <jayminm22@gmail.com>
Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>
Co-authored-by: SylvainLan <sylvain.s.lannuzel@gmail.com>
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Hanmin Qin <qinhanmin2005@sina.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Vachan D A <vachanda@users.noreply.github.com>
Co-authored-by: Sambhav Kothari <sambhavs.email@gmail.com>
Co-authored-by: wenliwyan <12013376+wenliwyan@users.noreply.github.com>
Co-authored-by: shivamgargsya <shivam.gargshya@gmail.com>
Co-authored-by: Reshama Shaikh <rs2715@stern.nyu.edu>
Co-authored-by: Oliver Urs Lenz <oulenz@users.noreply.github.com>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Co-authored-by: Brian Wignall <BrianWignall@gmail.com>
Co-authored-by: Ritchie Ng <ritchieng@u.nus.edu>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: inderjeet <43402782+inder128@users.noreply.github.com>
Co-authored-by: scibol <scibol@users.noreply.github.com>
Co-authored-by: Tirth Patel <tirthasheshpatel@gmail.com>
Co-authored-by: Bibhash Chandra Mitra <bibhashm220896@gmail.com>
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Co-authored-by: Tiffany R. Williams <Tiffany8@users.noreply.github.com>
Co-authored-by: Jigna Panchal <40188288+jigna-panchal@users.noreply.github.com>
Co-authored-by: @nkish <19225359+ankishb@users.noreply.github.com>
Co-authored-by: Pulkit Mehta <pulkit_mehta_work@yahoo.com>
Co-authored-by: David Breuer <DavidBreuer@users.noreply.github.com>
Co-authored-by: Niklas <niklas.sm+github@gmail.com>
Co-authored-by: Windber <guolipengyeah@126.com>
Co-authored-by: Stephen Blystone <29995339+blynotes@users.noreply.github.com>
Co-authored-by: Brigitta Sipőcz <b.sipocz@gmail.com>
@adrinjalali adrinjalali added this to Done in Meeting Issues Jan 6, 2020
panpiort8 pushed a commit to panpiort8/scikit-learn that referenced this issue Mar 3, 2020
* support a scalar fit param

* pep8

* TST add test for desired behavior

* FIX introduce _check_fit_params to validate parameters

* DOC update whats new

* TST tests both grid-search and randomize-search

* PEP8

* DOC revert unecessary change

* TST add test for _check_fit_params

* olivier comments

* TST fixes

* DOC whats new

* DOC whats new

* TST revert type of error

* add olivier suggestions

* address olivier comments

* address thomas comments

* PEP8

* comments olivier

* TST fix test by passing X

* avoid to call twice tocsr

* add case column/row sparse in check_fit_param

* provide optional indices

* TST check content when indexing params

* PEP8

* TST update tests to check identity

* stupid fix

* use a distribution in RandomizedSearchCV

* MNT add lightgbm to one of the CI build

* move to another build

* do not install dependencies lightgbm

* MNT comments on the CI setup

* address some comments

* Test fit_params compat without dependency on lightgbm

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@adrinjalali adrinjalali deleted the fit_params/scaler branch Apr 23, 2020
Pseudomanifold pushed a commit to BorgwardtLab/scikit-learn that referenced this issue Apr 24, 2020
* DOC fixed default values in dbscan (scikit-learn#15753)

* DOC fix incorrect branch reference in contributing doc (scikit-learn#15779)

* DOC relabel Feature -> Efficiency in change log (scikit-learn#15770)

* DOC fixed Birch default value (scikit-learn#15780)

* STY Minior change on code padding in website theme (scikit-learn#15768)

* DOC Fix yticklabels order in permutation importances example (scikit-learn#15799)

* Fix yticklabels order in permutation importances example

* STY Update wrapper width (scikit-learn#15793)

* DOC Long sentence was hard to parse and ambiguous in _classification.py (scikit-learn#15769)

* DOC Removed duplicate 'classes_' attribute in Naive Bayes classifiers (scikit-learn#15811)

* BUG Fixes pandas dataframe bug with boolean dtypes (scikit-learn#15797)

* BUG Returns only public estimators in all_estimators (scikit-learn#15380)

* DOC improve doc for multiclass and types_of_target (scikit-learn#15333)

* TST Increases tol for check_pca_float_dtype_preservation assertion (scikit-learn#15775)

* update _alpha_grid class in _coordinate_descent.py (scikit-learn#15835)

* FIX Explicit conversion of ndarray to object dtype. (scikit-learn#15832)

* BLD Parallelize sphinx builds on circle ci (scikit-learn#15745)

* DOC correct url for preprocessing (scikit-learn#15853)

* MNT avoid generating too many cross links in examples (scikit-learn#15844)

* DOC Correct wrong doc in precision_recall_fscore_support (scikit-learn#15833)

* DOC add comment in check_pca_float_dtype_preservation (scikit-learn#15819)

Documenting the changes in scikit-learn#15775

* DOC correct indents in docstring _split.py (scikit-learn#15843)

* DOC fix docstring of KMeans based on sklearn guideline (scikit-learn#15754)

* DOC fix docstring of AgglomerativeClustering based on sklearn guideline (scikit-learn#15764)

* DOC fix docstring of AffinityPropagation based on sklearn guideline (scikit-learn#15777)

* DOC fixed SpectralCoclustering and SpectralBiclustering docstrings following sklearn guideline (scikit-learn#15778)

* DOC fix FeatureAgglomeration and MiniBatchKMeans docstring following sklearn guideline (scikit-learn#15809)

* TST Specify random_state in test_cv_iterable_wrapper (scikit-learn#15829)

* DOC Include LinearSV{C, R} in models that support sample_weights (scikit-learn#15871)

* DOC correct some indents (scikit-learn#15875)

* DOC Fix documentation of default values in tree classes (scikit-learn#15870)

* DOC fix typo in docstring (scikit-learn#15887)

* DOC FIX default value for xticks_rotation in plot_confusion_matrix (scikit-learn#15890)

* Fix imports in pip3 ubuntu by suffixing affected files (scikit-learn#15891)

* MNT Raise erorr when normalize is invalid in confusion_matrix (scikit-learn#15888)

* [MRG] DOC Increases search results for API object results (scikit-learn#15574)

* MNT Ignores warning in pyamg for deprecated scipy.random (scikit-learn#15914)

* DOC Instructions to troubleshoot Windows path length limit (scikit-learn#15916)

* DOC add versionadded directive to some estimators (scikit-learn#15849)

* DOC clarify doc-string of roc_auc_score and add references (scikit-learn#15293)

* MNT Adds skip lint to azure pipeline CI (scikit-learn#15904)

* BLD Fixes bug when building with NO_MATHJAX=1 (scikit-learn#15892)

* [MRG] BUG Checks to number of axes in passed in ax more generically (scikit-learn#15760)

* EXA Minor fixes in plot_sparse_logistic_regression_20newsgroups.py (scikit-learn#15925)

* BUG Do not shadow public functions with deprecated modules (scikit-learn#15846)

* Import sklearn._distributor_init first (scikit-learn#15929)

* DOC Fix typos, via a Levenshtein-style corrector (scikit-learn#15923)

* DOC in canned comment, mention that PR title becomes commit me… (scikit-learn#15935)

* DOC/EXA Correct spelling of "Classification" (scikit-learn#15938)

* BUG fix pip3 ubuntu update by suffixing file (scikit-learn#15928)

* [MRG] Ways to compute center_shift_total were different in "full" and "elkan" algorithms. (scikit-learn#15930)

* TST Fixes integer test for train and test indices (scikit-learn#15941)

* BUG ensure that parallel/sequential give the same permutation importances (scikit-learn#15933)

* Formatting fixes in changelog (scikit-learn#15944)

* MRG FIX: order of values of self.quantiles_ in QuantileTransformer (scikit-learn#15751)

* [MRG] BUG Fixes constrast in plot_confusion_matrix (scikit-learn#15936)

* BUG use zero_division argument in classification_report (scikit-learn#15879)

* DOC change logreg solver in plot_logistic_path (scikit-learn#15927)

* DOC fix whats new ordering (scikit-learn#15961)

* COSMIT use np.iinfo to define the max int32 (scikit-learn#15960)

* DOC Apply numpydoc validation to VotingRegressor methods (scikit-learn#15969)

Co-authored-by: Tiffany R. Williams <Tiffany8@users.noreply.github.com>

* DOC improve naive_bayes.py documentation (scikit-learn#15943)

Co-authored-by: Jigna Panchal <40188288+jigna-panchal@users.noreply.github.com>

* DOC Fix default values in Perceptron documentation (scikit-learn#15965)

* DOC Improve default values in logistic documentation (scikit-learn#15966)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>

* DOC Improve documentation of default values for imputers (scikit-learn#15964)

* EXA/MAINT Simplify code in manifold learning example (scikit-learn#15949)

* DOC Improve default values in SGD documentation (scikit-learn#15967)

* DOC Improve defaults in neural network documentation (scikit-learn#15968)

* FIX use safe_sparse_dot for callable kernel in LabelSpreading (scikit-learn#15868)

* BUG Adds attributes back to check_is_fitted (scikit-learn#15947)

Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* DOC update check_is_fitted what's new

* DOC change python-devel to python3-devel for yum. (scikit-learn#15986)

* DOC Correct the default value of values_format in plot_confusion_matrix (scikit-learn#15981)

* [MRG] MNT Updates pypy to use 7.2.0 (scikit-learn#15954)

* FIX Add missing 'values_format' param to disp.plot() in plot_confusion_matrix (scikit-learn#15937)

* FIX support scalar values in fit_params in SearchCV (scikit-learn#15863)

* support a scalar fit param

* pep8

* TST add test for desired behavior

* FIX introduce _check_fit_params to validate parameters

* DOC update whats new

* TST tests both grid-search and randomize-search

* PEP8

* DOC revert unecessary change

* TST add test for _check_fit_params

* olivier comments

* TST fixes

* DOC whats new

* DOC whats new

* TST revert type of error

* add olivier suggestions

* address olivier comments

* address thomas comments

* PEP8

* comments olivier

* TST fix test by passing X

* avoid to call twice tocsr

* add case column/row sparse in check_fit_param

* provide optional indices

* TST check content when indexing params

* PEP8

* TST update tests to check identity

* stupid fix

* use a distribution in RandomizedSearchCV

* MNT add lightgbm to one of the CI build

* move to another build

* do not install dependencies lightgbm

* MNT comments on the CI setup

* address some comments

* Test fit_params compat without dependency on lightgbm

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>

* Remove abstractmethod that silently brake downstream packages (scikit-learn#15996)

* FIX restore BaseNB._check_X without abstractmethod decoration (scikit-learn#15997)

* Update v0.22 changelog for 0.22.1 (scikit-learn#16002)

- set the date
- move entry for quantile transformer to the 0.22.1 section
- fix alphabetical ordering of modules

* STY Removes hidden scroll bar (scikit-learn#15999)

* Flake8 fixes

* Fix: remove left-over lines that should have been deleted during conflict resolution when rebasing

* Fix missing imports

* Update version

* Fix test_check_is_fitted

* Make test_sag_regressor_computed_correctly deterministic (scikit-learn#16003)

Fix scikit-learn#15818.

Co-authored-by: cgsavard <claire.savard@colorado.edu>
Co-authored-by: Joel Nothman <joel.nothman@gmail.com>
Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com>
Co-authored-by: Matt Hall <matt@agilegeoscience.com>
Co-authored-by: Kathryn Poole <kathryn.poole2@gmail.com>
Co-authored-by: lucyleeow <jliu176@gmail.com>
Co-authored-by: JJmistry <jayminm22@gmail.com>
Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>
Co-authored-by: SylvainLan <sylvain.s.lannuzel@gmail.com>
Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>
Co-authored-by: Hanmin Qin <qinhanmin2005@sina.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Vachan D A <vachanda@users.noreply.github.com>
Co-authored-by: Sambhav Kothari <sambhavs.email@gmail.com>
Co-authored-by: wenliwyan <12013376+wenliwyan@users.noreply.github.com>
Co-authored-by: shivamgargsya <shivam.gargshya@gmail.com>
Co-authored-by: Reshama Shaikh <rs2715@stern.nyu.edu>
Co-authored-by: Oliver Urs Lenz <oulenz@users.noreply.github.com>
Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
Co-authored-by: Brian Wignall <BrianWignall@gmail.com>
Co-authored-by: Ritchie Ng <ritchieng@u.nus.edu>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: inderjeet <43402782+inder128@users.noreply.github.com>
Co-authored-by: scibol <scibol@users.noreply.github.com>
Co-authored-by: Tirth Patel <tirthasheshpatel@gmail.com>
Co-authored-by: Bibhash Chandra Mitra <bibhashm220896@gmail.com>
Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>
Co-authored-by: Tiffany R. Williams <Tiffany8@users.noreply.github.com>
Co-authored-by: Jigna Panchal <40188288+jigna-panchal@users.noreply.github.com>
Co-authored-by: @nkish <19225359+ankishb@users.noreply.github.com>
Co-authored-by: Pulkit Mehta <pulkit_mehta_work@yahoo.com>
Co-authored-by: David Breuer <DavidBreuer@users.noreply.github.com>
Co-authored-by: Niklas <niklas.sm+github@gmail.com>
Co-authored-by: Windber <guolipengyeah@126.com>
Co-authored-by: Stephen Blystone <29995339+blynotes@users.noreply.github.com>
Co-authored-by: Brigitta Sipőcz <b.sipocz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment