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).
Joris
>
> I'd argue that we've often already broken such code, and that at least now
> it will break with a TypeError rather than silent misbehaviour.
>
> And yet Sebastian's comment implies that there may be a whole raft of
> former MATLAB users writing code without kwargs. Is that a problem if now
> they get a TypeError?
>
> On Fri, 16 Nov 2018 at 16:23, Sebastian Raschka <m...@sebastianraschka.com>
> wrote:
>
>> Also want to say that I really welcome this decision/change. Personally,
>> as far as I am aware, I've trying been using keyword arguments consistently
>> for years, except for cases where it is really obvious, like .fit(X_train,
>> y_train), and I believe that it really helped me regarding writing less
>> error-prone code/analyses.
>>
>> Thinking back of the times where I was using MATLAB, it was really clunky
>> and error-prone to import functions and being careful about the argument
>> order.
>>
>> Besides, keynote arguments definitely make code and documentation much
>> more readable (within and esp. across different package versions) despite
>> (or maybe because) being more verbose.
>>
>> Best,
>> Sebastian
>>
>>
>>
>> > On Nov 15, 2018, at 10:18 PM, Brown J.B. via scikit-learn <
>> scikit-learn@python.org> wrote:
>> >
>> > As an end-user, I would strongly support the idea of future enforcement
>> of keyword arguments for new parameters.
>> > In my group, we hold a standard that we develop APIs where _all_
>> arguments must be given by keyword (slightly pedantic style, but has shown
>> to have benefits).
>> > Initialization/call-time state checks are done by a class' internal
>> methods.
>> >
>> > As Andy said, one could consider leaving prototypical X,y as
>> positional, but one benefit my group has seen with full keyword
>> parameterization is the ability to write code for small investigations
>> where we are more concerned with effects from parameters rather than the
>> data (e.g., a fixed problem to model, and one wants to first see on the
>> code line what the estimators and their parameterizations were).
>> > If one could shift the sklearn X,y to the back of a function call, it
>> would enable all participants in a face-to-face code review session to
>> quickly see the emphasis/context of the discussion and move to the
>> conclusion faster.
>> >
>> > To satisfy keyword X,y as well, I would presume that the BaseEstimator
>> would need to have a sanity check for error-raising default X,y values --
>> though does it not have many checks on X,y already?
>> >
>> > Not sure if everyone else agrees about keyword X and y, but just a
>> thought for consideration.
>> >
>> > Kind regards,
>> > J.B.
>> >
>> > 2018年11月15日(木) 18:34 Gael Varoquaux <gael.varoqu...@normalesup.org>:
>> > I am really in favor of the general idea: it is much better to use named
>> > arguments for everybody (for readability, and to be less depend on
>> > parameter ordering).
>> >
>> > However, I would maintain that we need to move slowly with backward
>> > compatibility: changing in a backward-incompatible way a library brings
>> > much more loss than benefit to our users.
>> >
>> > So +1 for enforcing the change on all new arguments, but -1 for changing
>> > orders in the existing arguments any time soon.
>> >
>> > I agree that it would be good to push this change in existing models. We
>> > should probably announce it strongly well in advance, make sure that all
>> > our examples are changed (people copy-paste), wait a lot, and find a
>> > moment to squeeze this in.
>> >
>> > Gaël
>> >
>> > On Thu, Nov 15, 2018 at 06:12:35PM +1100, Joel Nothman wrote:
>> > > We could just announce that we will be making this a syntactic
>> constraint from
>> > > version X and make the change wholesale then. It would be less formal
>> backwards
>> > > compatibility than we usually hold by, but we already are loose with
>> parameter
>> > > ordering when adding new ones.
>> >
>> > > It would be great if after this change we could then reorder
>> parameters to make
>> > > some sense!
>> >
>> > > _______________________________________________
>> > > scikit-learn mailing list
>> > > scikit-learn@python.org
>> > > https://mail.python.org/mailman/listinfo/scikit-learn
>> >
>> >
>> > --
>> > Gael Varoquaux
>> > Senior Researcher, INRIA Parietal
>> > NeuroSpin/CEA Saclay , Bat 145, 91191 Gif-sur-Yvette France
>> > Phone: ++ 33-1-69-08-79-68
>> > http://gael-varoquaux.info
>> http://twitter.com/GaelVaroquaux
>> > _______________________________________________
>> > scikit-learn mailing list
>> > scikit-learn@python.org
>> > https://mail.python.org/mailman/listinfo/scikit-learn
>> > _______________________________________________
>> > scikit-learn mailing list
>> > scikit-learn@python.org
>> > https://mail.python.org/mailman/listinfo/scikit-learn
>>
>> _______________________________________________
>> scikit-learn mailing list
>> scikit-learn@python.org
>> https://mail.python.org/mailman/listinfo/scikit-learn
>>
> _______________________________________________
> scikit-learn mailing list
> scikit-learn@python.org
> https://mail.python.org/mailman/listinfo/scikit-learn
>
_______________________________________________
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn
