Suggestion: no more return self #935
MaxHalford
started this conversation in
Ideas
Replies: 1 comment
-
I agree with that. It always looked weird to me, since the skmultiflow times. Chaining method calls although useful adds unnecessary clutter. It's not that much used, anyhow. You have my vote, @MaxHalford :) |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
I think it would make sense to remove all the
return self
from the update methods of every class we have. I added this initially because that's what scikit-learn does, and scikit-learn had a big influence on how I did things. But the more I think, the more I see this as noise. Some arguments:model = model.learn_one(x, y)
, eithermodel.learn_one(x, y)
. Both do the same thing. I rather have one way of doing things.model = model.learn_one(x, y)
in docstrings, else the model is expected to be part of the docstring return.return self
allows chaining method calls, suchmodel.learn_one(x, y).learn_one(x, y).predict_one(x)
. But I've never seen that usage pattern.model = model.learn_one(x, y)
gives a false impression thatmodel.learn_one(x, y)
is a pure function call with no side-effects. But it isn't.Of course, I'd be happy to implement this change if we agree on it. I suggest we take a vote between @smastelini, @jacobmontiel, and I. @jacobmontiel is busy at the moment, so let's agree on the change if @smastelini votes in favor.
Beta Was this translation helpful? Give feedback.
All reactions