Add Repeating Basis Functions

[RensDimmendaal]

I worked on this a while ago in PR #147, but I started from a fresh branch because I decided to limit the scope only to repeating basis functions (#20), excluding the spanning basis functions (#29).

Feedback adapted so far:

• Write a transformer when X contains only one column
• Write a wrapper which selects one column, when X has more than one column
• Added basic tests
• Added docstring

To Do:

• Write documentation

Could someone review the code?
I'll work on the documentation in the meantime as well.

Tagging people involved in PR #147: @kayhoogland @koaning @MBrouns

[koaning]

I've taken a few minutes to have a glance, will try to find some more time to look at the code later.

[RensDimmendaal]

• Adopted the docstring suggestions
• Rename the floor/ceil parameters to input_range to clarify what it does
• Added documentation as a notebook

Not sure why travis fails, seems to me like the issue is with other notebooks which i have not touched link.

[koaning]

So there's a bug in a part of the code that has to do with the grouped-estimator. If that part of the library is causing this then I'll still gladly merge the work here.

[koaning]

but this seems to be formatting on your side?

[koaning]

But this you may safely ignore, this is something that I am picking up with @pim-hoeven.

[RensDimmendaal]

Thanks. My bad on the formatting issue. Fixed it now.

If that other issue can be ignored then I think it is ready.

[koaning]

Great. I'll have a look at the end of the day.

[koaning]

Great that you took the effort of moving the preprocessing documentation into a notebook but is there a reason why only your code is evaluated?

[koaning]

I am playing with the notebook and i see something odd.

When remainder should be dropping ... it isn't?

[koaning]

I am having worries about the remainder behaviour. To me it feels like the base behaviour should be that it always drops the column. In my mind it would be used similar to this:

You would have a feature union split the steps and pass the correct column to the model. I think having passthrough behaviour can still be valid but I would argue that it goes against a base behaviour.

@MBrouns thoughts?

[RensDimmendaal]

Docs:
Had to do a slight rewrite of the code snippet you screenshotted above, but I've now converted all snippets in the preprocessing docs to working code.

Regarding faulty "drop" behavior:
Shame on me, that's a bug. I've fixed it and added a test for it now as well.

Default behavior "drop"/"passthrough":
That's fair. It makes more sense to have "drop" as the default. That's also the default of the ColumnTransformer which I use to accomplish the behavior.

[MBrouns]

regarding the drop / passthrough I think we should go for the principal of least surprise and to me it feels like the transformer is most like the one hot encoder. The OHE keeps all the non-categorical columns and simply adds the encoded columns so for me it would make most sense to have the same behaviour here.

On the other hand, the categorical_features attribute of the OHE is getting deprecated and users are told to use a ColumnTransformer instead, so I don't know..

[koaning]

Oh yeah. I forgot to click the merge button. @RensDimmendaal well done!

[RensDimmendaal]

Whoo! :-) Thanks for the help.