.. versionadded:: 0.21, and if it doesn't get merged in time, then 0.22.

Also needs an entry in the related whats_new file, like the others.

I don't understand why the two test pipelines fail. Does it have something with my commits?

Try to pull and merge master? I think I had the same CI fail a few days ago

@NicolasHug Sorry, I'm new to this and don't want to mess things up. Where shall I try to pull and merge master, pls?

add the original repo as a separate remote:

git remote add upstream https://github.com/scikit-learn/scikit-learn.git

fetch the remote

git fetch upstream

merge upstream's branch into your branch (while being in your branch)

git merge upstream/master

Many thanks, I understand now. There is nothing to be merged with though. For now.

@petibear You're not responsible for the CI fails, I opened #13506

@petibear Since #13506 is merged now you can try committing an empty commit to trigger the CI again (or just address the review above of course)

git commit --allow-empty -m "Trigger CI"

Checks should go green

IMO it's pretty clear from the docstring that warm_start affects n_estimators, and the link to the docstring is also helpful

It's more like if you know what it's supposed to do, and you read the docstring, then it's very clear that it conveys the message. But it's cryptic enough that if you don't know, then you may not understand how it works. Maybe add it to doc/modules/ensemble.rst where we only have gradient_boosting_warm_start would help?

As for the tests, I'm okay with what the PR has, since for the rest tests are done in the parent class. No strong feelings there.

I could then draft an addition to doc/modules/ensemble.rst (i.e. to its Bagging meta-estimator section) to say that classes derived from BaseBagging (BaggingClassifier, BaggingRegressor as well as IsolationForest and I guess that's all) support incremental addition of estimators via warm_start. It could have helped me when I encountered the issue. I would also add a short snippet for iForest (?).

Furthermore, since the text would then specifically refer to IsolationForest, I think the sentence "As they provide a way to reduce overfitting, bagging methods work best with strong and complex models (e.g., fully developed decision trees), in contrast with boosting methods which usually work best with weak models (e.g., shallow decision trees)." should be also changed or a further clarification should be added. Isolation forest deliberately uses shallower trees.

I don't think it's relevant to add anything to ensemble.rst since this section of the doc belongs to supervised learning while IForest belongs to unsupervised learning. There is currently no mention of IForest in ensemble.rst, rightfully so.

(For the same reason the sentence about shallow trees doesn't need an update IMO).

I would suggest to update instead the isolation forest section of outlier_detection.rst and write a small example here using warm_start, pretty much like what ensemble.rst/gradient_boosting is already doing.

Fair enough, I've added an entry to outlier_detection.rst. I struggled a bit where to place it (and whether to create a subsection). I decided for a subsection under the reference to the paper as not to make it too prominent, but you might want it different.

Is there anything else to be done here?

could you please merge master again and fix the merge conflicts? You're getting unlucky on this one.

That's not a problem, at least I learn the process.
I'm now in the office, will do it later this night. Will also move comments in the example in outlier_detection.rst.

ok, merged, pushed, hopefully all good now

Thanks @petibear

Many thanks too for all the help, @adrinjalali & the rest of you guys. Happy to have contributed and to have learnt something here