Mean Normalisation Scaling#806
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #806 +/- ##
==========================================
+ Coverage 97.98% 98.00% +0.01%
==========================================
Files 107 109 +2
Lines 4320 4350 +30
Branches 857 709 -148
==========================================
+ Hits 4233 4263 +30
Misses 54 54
Partials 33 33 ☔ View full report in Codecov by Sentry. |
solegalli
left a comment
solegalli
left a comment
There was a problem hiding this comment.
Hey @VascoSch92
This is looking really good. Thank you for the first draft.
I think we could tidy it a bit so that we don't loop neither in fit nor in transform.
Could you take a look?
Thank you!
|
Hey @solegalli I addressed your comments. Please let me know what you think :-) |
solegalli
left a comment
solegalli
left a comment
There was a problem hiding this comment.
Hi @VascoSch92
This is looking really good. The tests are great.
Regarding the logic, I think we can speed this up if we store the range instead of min max,, and if we use dictionaries instead of dataframes. Could you give that a go?
Feel free to start working on the dosctrings and on adding a user guide :)
|
Hey @solegalli I changed what requested.
|
solegalli
left a comment
solegalli
left a comment
There was a problem hiding this comment.
Hey @VascoSch92 really good work here. Thank you so much!
We need to add a few files to create the docs now. Would you be able to do that as well?
Thanks a lot for the hard work.
|
Last but not least: we need to add the new module on the readme and on the frontpage of the documentation, which lives here: https://github.com/feature-engine/feature_engine/blob/main/docs/index.rst Thank you!! |
|
Hey @solegalli
|
solegalli
left a comment
solegalli
left a comment
There was a problem hiding this comment.
Hey @VascoSch92 thanks for the quick turnaround. The api docs look great.
Co-authored-by: Soledad Galli <[email protected]>
Co-authored-by: Soledad Galli <[email protected]>
Co-authored-by: Soledad Galli <[email protected]>
Co-authored-by: Soledad Galli <[email protected]>
Co-authored-by: Soledad Galli <[email protected]>
|
Hey @solegalli I changed to dictionaries instead of |
|
Amazing! Thanks a lot! We just need to add a description / demo to the user guide folder in the docs and we are good to go then :) |
|
let me give a look ;-) |
|
@solegalli quick question: in general feature engine use scaling transformers from sklearn. Should we also include examples of these transformers? |
|
Hey @solegalli I updated the docs with a demo. Let me know what do you think. It is just a first version :-) |
|
Hey @solegalli :-) did you have time to look at the latest changes? |
|
Pending acceptance of suggested changes: VascoSch92#1 |
…ormalization rewords the documentation and adds missing links
2 participants
First version of the
MeanNormalizationScalingas discussed in #763I create a new module scaling as discussed.
Probably, you will also add new scaling in this module.