+1 on the ideal in general (and to enforce this on new classes / params). +1 to be conservative and not break existing code.
Le mar. 20 nov. 2018 à 21:09, Joris Van den Bossche < jorisvandenboss...@gmail.com> a écrit : > Op zo 18 nov. 2018 om 11:14 schreef Joel Nothman <joel.noth...@gmail.com>: > >> I think we're all agreed that this change would be a good thing. >> >> What we're not agreed on is how much risk we take by breaking legacy code >> that relied on argument order. >> > > I think that, in principle, it could be possible to do this with a > deprecation warning. If we would do a signature like the following: > > class Model(BaseEstimator): > def __init__(self, *args, param1=1, param2=2): > > then we could in principle catch all positional args, raise a warning if > there are any, and by inspecting the signature (as we now also do in > _get_param_names), we could set the appropriate parameters on self. > I think the main problem is that this would temporarily "allow" that > people also pass a keyword argument that conflicts with a positional > argument without that it raises an error (as Python normally would do for > you), but you still get the warning. > And of course, it would violate the clean __init__ functions in > scikit-learn that do no validation. > > I personally don't know how big the impact would be of simply doing it as > breaking change, but if we think it might be potentially quite big, the > above might be worth considering (otherwise I wouldn't go through the > hassle). > That would render sphinx API doc and IDE prototype tooltips confusing though. But maybe we if use an explicit name such as: *deprecated_positional_args that would be fine enough. -- Olivier http://twitter.com/ogrisel - http://github.com/ogrisel
_______________________________________________ scikit-learn mailing list scikit-learn@python.org https://mail.python.org/mailman/listinfo/scikit-learn
