Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-05-18 Thread Manuel CASTEJÓN LIMAS via scikit-learn
Dear Joel,

I've changed the code of PipeGraph in order to change the old wrappers to
new Mixin Classes. The changes are reflected in this MixinClasses branch:

https://github.com/mcasl/PipeGraph/blob/feature/MixinClasses/pipegraph/adapters.py

My conclusions are that although both approaches are feasible and provide
similar functionality, Mixin Classes provide a simpler solution. Following
the 'flat is better than nested' principle, the mixin classes should be
favoured.
This approach seems as well to be more in line with general
sklearn development practice, so I'll make the necessary changes to the
docs and then the master branch will be replaced with this new Mixin
classes version.

Thanks for pointing out this issue!
Best
Manuel

2018-04-16 14:21 GMT+02:00 Manuel CASTEJÓN LIMAS :

> Nope! Mostly because of lack of experience with mixins.
> I've done some reading and I think I can come up with a few mixins doing
> the trick by dynamically adding their methods to an already instantiated
> object. I'll play with that and I hope to show you something soon! Or at
> least I will have better grounds to make an educated decision.
> Best
> Manuel
>
>
>
>
> Manuel Castejón Limas
> *Escuela de Ingeniería Industrial e Informática*
> Universidad de León
> Campus de Vegazana sn.
> 24071. León. Spain.
> *e-mail: *manuel.caste...@unileon.es
> *Tel.*: +34 987 291 946
>
> Digital Business Card: Click Here 
>
>
>
> 2018-04-15 15:18 GMT+02:00 Joel Nothman :
>
>> Have you considered whether a mixin is a better model than a wrapper?​
>>
>
>
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn


Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-16 Thread Javier López
Hi Manolo!

Your code looks nice, but my use case is a bit different. I have a mixed
set of parameters, some come from my wrapper,
and some from the wrapped estimator. The logic I am going for is something
like
"If you know about this parameter, then deal with it, if not, then pass it
along to the wrapped estimator and hope for the best!"
which is why I was asking Andreas about the use of `super`.

Joel is right that a mixin would be the natural way of adding
functionality, but unless I am getting something wrong that would
require me modifying the base classes from sklearn by either forking the
code (which sounds like a lot of trouble) or by
monkey-patching the import (also not an ideal solution).

There are several wrappers in scikit-learn that are similar in spirit to
what I am trying to do, for instalce `CalibratedClassifierCV`,
but neither of them deal with the `set_params` and `get_params` in a way
that delegates to the base estimator, which makes
them unsuitable for use in some third party tools such as BorutaPy [1]
which expects an estimator where
`estimator.set_params("n_estimators")` makes sense.

Cheers,
Javier


[1] https://github.com/scikit-learn-contrib/boruta_py

On Sat, Apr 14, 2018 at 5:10 PM Manuel CASTEJÓN LIMAS via scikit-learn <
scikit-learn@python.org> wrote:

> Hi Javier!
> Yo can have a look at:
>
> https://github.com/mcasl/PipeGraph/blob/master/pipegraph/adapters.py
>
> There are a few adapters there and I had tool deal with that situation. I
> solved it by using __getattr__ and __setattr__.
> Best
> Manolo
>
>
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn


Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-16 Thread Javier López
How could I make mixins work in this case?

If I define the class `FancyEstimatorMixin`, in order to get a drop-in
replacement for a sklearn
object wouldn't I need to monkey-patch the scikit-learn `BaseEstimator`
class to inherit from my mixin?
Or am I misunderstanding something?

(BTW monkey-patching is one of the things I am trying to avoid in a "clean"
solution)

On Mon, Apr 16, 2018 at 1:24 PM Manuel CASTEJÓN LIMAS via scikit-learn <
scikit-learn@python.org> wrote:

> Nope! Mostly because of lack of experience with mixins.
> I've done some reading and I think I can come up with a few mixins doing
> the trick by dynamically adding their methods to an already instantiated
> object. I'll play with that and I hope to show you something soon! Or at
> least I will have better grounds to make an educated decision.
> Best
> Manuel
>
> 2018-04-15 15:18 GMT+02:00 Joel Nothman :
>
>> Have you considered whether a mixin is a better model than a wrapper?​
>>
>
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn


Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-16 Thread Manuel CASTEJÓN LIMAS via scikit-learn
Nope! Mostly because of lack of experience with mixins.
I've done some reading and I think I can come up with a few mixins doing
the trick by dynamically adding their methods to an already instantiated
object. I'll play with that and I hope to show you something soon! Or at
least I will have better grounds to make an educated decision.
Best
Manuel




Manuel Castejón Limas
*Escuela de Ingeniería Industrial e Informática*
Universidad de León
Campus de Vegazana sn.
24071. León. Spain.
*e-mail: *manuel.caste...@unileon.es
*Tel.*: +34 987 291 946

Digital Business Card: Click Here 



2018-04-15 15:18 GMT+02:00 Joel Nothman :

> Have you considered whether a mixin is a better model than a wrapper?​
>
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn


Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-15 Thread Joel Nothman
Have you considered whether a mixin is a better model than a wrapper?​
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn


Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-14 Thread Manuel CASTEJÓN LIMAS via scikit-learn
Hi Javier!
Yo can have a look at:

https://github.com/mcasl/PipeGraph/blob/master/pipegraph/adapters.py

There are a few adapters there and I had tool deal with that situation. I
solved it by using __getattr__ and __setattr__.
Best
Manolo

El vie., 13 abr. 2018 17:53, Javier López  escribió:

> I have a class `FancyEstimator(BaseEstimator, MetaEstimatorMixin): ...`
> that wraps
> around an arbitrary sklearn estimator to add some functionality I am
> interested about.
> This class contains an attribute `self.estimator` that contains the
> wrapped estimator.
> Delegation of the main methods, such as `fit`, `transform` works just
> fine, but I am
> having some issues with `get_params` and `set_params`.
>
> The main idea is, I would like to use my wrapped class as a drop-in
> replacement for
> the original estimator, but this raises some issues with some functions
> that try using the `get_params` and `set_params` straight in my class, as
> the original
> parameters now have prefixed names (for instance `estimator__verbose`
> instead of `verbose`)
> I would like to delegate calls of set_params and get_params in a smart way
> so that if a
> parameter is unknown for my wrapper class, then it automatically goes
> looking for it in
> the wrapped estimator.
>
>  I am not concerned about my class parameter names as there are only a
> couple of very
> specific names on it, so it is safe to assume that any unknown parameter
> name should
> refer to the base estimator. Is there an easy way of doing that?
>
> Cheers,
> J
> ___
> 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


Re: [scikit-learn] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-13 Thread Andreas Mueller

Please stay on the mailing list :)

I'm not sure if ValueError is the right error that would be raised (I 
think it's not).
And it's hard to say if this will break something in some edge cases. I 
would
probably rather explicitly encode the parameters of FancyEstimator 
instead of the try,

or get them using super.get_params. You also should rewrite get_params.

In principle something like that should work, but I wouldn't go as far 
as saying it's

"safe" and you should test it extensively.

On 04/13/2018 12:41 PM, Javier López wrote:
Is something like this safe, or might I be breaking some important 
functionality?


```
    def set_params(self, **params):
        try:
            super(FancyEstimator, self).set_params(**params)
        except ValueError:
            self.wrapped_estimator_.set_params(**params)
```


On Fri, Apr 13, 2018 at 5:05 PM Andreas Mueller > wrote:


You just need to implement get_params and set_params yourself to
delegate in this way, right?


On 04/13/2018 11:51 AM, Javier López wrote:

I have a class
`FancyEstimator(BaseEstimator, MetaEstimatorMixin): ...` that wraps
around an arbitrary sklearn estimator to add some functionality I
am interested about.
This class contains an attribute `self.estimator` that contains
the wrapped estimator.
Delegation of the main methods, such as `fit`, `transform` works
just fine, but I am
having some issues with `get_params` and `set_params`.

The main idea is, I would like to use my wrapped class as a
drop-in replacement for
the original estimator, but this raises some issues with some
functions
that try using the `get_params` and `set_params` straight in my
class, as the original
parameters now have prefixed names (for instance
`estimator__verbose` instead of `verbose`)
I would like to delegate calls of set_params and get_params in a
smart way so that if a
parameter is unknown for my wrapper class, then it automatically
goes looking for it in
the wrapped estimator.

 I am not concerned about my class parameter names as there are
only a couple of very
specific names on it, so it is safe to assume that any unknown
parameter name should
refer to the base estimator. Is there an easy way of doing that?

Cheers,
J


___
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] Delegating "get_params" and "set_params" to a wrapped estimator when parameter is not defined.

2018-04-13 Thread Javier López
I have a class `FancyEstimator(BaseEstimator, MetaEstimatorMixin): ...`
that wraps
around an arbitrary sklearn estimator to add some functionality I am
interested about.
This class contains an attribute `self.estimator` that contains the wrapped
estimator.
Delegation of the main methods, such as `fit`, `transform` works just fine,
but I am
having some issues with `get_params` and `set_params`.

The main idea is, I would like to use my wrapped class as a drop-in
replacement for
the original estimator, but this raises some issues with some functions
that try using the `get_params` and `set_params` straight in my class, as
the original
parameters now have prefixed names (for instance `estimator__verbose`
instead of `verbose`)
I would like to delegate calls of set_params and get_params in a smart way
so that if a
parameter is unknown for my wrapper class, then it automatically goes
looking for it in
the wrapped estimator.

 I am not concerned about my class parameter names as there are only a
couple of very
specific names on it, so it is safe to assume that any unknown parameter
name should
refer to the base estimator. Is there an easy way of doing that?

Cheers,
J
___
scikit-learn mailing list
scikit-learn@python.org
https://mail.python.org/mailman/listinfo/scikit-learn