scikit-lego
scikit-lego copied to clipboard
koaning
[SUGGESTION] Drop the column and remainder parameters from RepeatingBasisFunction
Hi!
Maybe it has historical reasons, but do we need the parameters column and remainder? Because this can be handled easily and more general using a ColumnTransformer.
Best Robert
You're suggesting something sensible here, but part of me is a little anxious about introducing a breaking change without adding a useful feature. I think this feature is in production in a bunch of places so I want to be a bit conservative here. There's certainly a sense of "purity" missing with the two parameters, but practically it doesn't feel like there's that much harm in keeping them as is.
Also, if you're encoding a single column, then technically you can already combine it with a ColumnTransformer, right?
I'd argue the feature in this case would be consistency with the main sklearn API. When we built this transformer either the ColumnTransformer wasn't out yet or was still experimental, but now it's been there long enough that it's clear that that is the way forward.
I'd vote for deprecating the current behaviour with a proper warning and suggestion on how to change it and then making the change a couple of releases down the road
@MBrouns fair enough! We're at version 0.6.6 now, so we might want to change the behavior in 0.9.x?