Skip to content

add MAD outlier rule to find outliers - #499

Merged
solegalli merged 17 commits into
feature-engine:mainfrom
glevv:mad-outlier-rule
Aug 22, 2022
Merged

add MAD outlier rule to find outliers#499
solegalli merged 17 commits into
feature-engine:mainfrom
glevv:mad-outlier-rule

Conversation

@glevv

@glevv glevv commented Aug 14, 2022

Copy link
Copy Markdown
Contributor

PR for #496
Added MAD outlier rule
Refactored base code and tests

Extend outlier detection functionality of OutlierTrimmer and Winsorizer.

glevv added 6 commits August 12, 2022 09:42
Small refactoring to get rid of repetitive calculations, addition of MAD (median absolute deviation) rule
@glevv
glevv marked this pull request as ready for review August 15, 2022 12:52
@solegalli
solegalli self-requested a review August 17, 2022 11:32
@solegalli solegalli closed this Aug 17, 2022
@solegalli solegalli reopened this Aug 17, 2022

@solegalli solegalli left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @glevv

Really, really good implementation of the outlier handlers. Thank you very much for taking the time to re-write almost the entire logic. Very elegant. I appreciate it a lot!

I also learned a lot.

There are a couple of tests failing.

Stylechecks is complaining about numpy imported but being unused, see below:

stylechecks run-test: commands[0] | flake8 feature_engine tests
feature_engine/outliers/base_outlier.py:3:1: F401 'numpy as np' imported but unused
feature_engine/outliers/base_outlier.py:179:89: E501 line too long (89 > 88 characters)
feature_engine/outliers/base_outlier.py:195:89: E501 line too long (91 > 88 characters)
feature_engine/outliers/base_outlier.py:218:89: E501 line too long (90 > 88 characters)
feature_engine/outliers/trimmer.py:4:1: F401 'numpy as np' imported but unused
tests/test_outliers/test_outlier_trimmer.py:4:1: F401 'numpy as np' imported but unused

The Python tests are failing because of the error to catch when scale

This test is also failing:

def test_error_if_capping_method_quantiles_and_fold_value_not_permitted():

I believe they should all be small fixes. Would you be able to have a look at it?

I think once the tests pass this is ready to go!

Thank you very much!

Comment thread feature_engine/outliers/base_outlier.py Outdated
Comment thread feature_engine/outliers/base_outlier.py Outdated
Comment thread feature_engine/outliers/base_outlier.py
Comment thread tests/test_outliers/test_winsorizer.py Outdated
@glevv glevv changed the title Mad outlier rule [WIP] Mad outlier rule Aug 17, 2022
@glevv

glevv commented Aug 19, 2022

Copy link
Copy Markdown
Contributor Author

Fixes implemented

@solegalli

Copy link
Copy Markdown
Collaborator

Hey @glevv

This implementation is great. Thank you so much!

Code-wise, this is good to go.

If I may ask a little bit more from you, could you please add the Mad rule to the user guide? In this 2 files:

https://github.com/feature-engine/feature_engine/blob/main/docs/user_guide/outliers/Winsorizer.rst

https://github.com/feature-engine/feature_engine/blob/main/docs/user_guide/outliers/OutlierTrimmer.rst

Thank you!

@solegalli solegalli changed the title [WIP] Mad outlier rule [MRG] Mad outlier rule Aug 22, 2022
@solegalli solegalli changed the title [MRG] Mad outlier rule add MAD outlier rule to find outliers Aug 22, 2022
@solegalli solegalli linked an issue Aug 22, 2022 that may be closed by this pull request
@solegalli
solegalli merged commit c409cb0 into feature-engine:main Aug 22, 2022
@glevv
glevv deleted the mad-outlier-rule branch September 25, 2022 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MAD-Median rule for outlier removal

2 participants