Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New features #49

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

Conversation

fayeab
Copy link

@fayeab fayeab commented Nov 22, 2020

closes #42

Hello,

I've added two features (filtering and deduplication criteria) for skope rules.

This work is an improvement of #42
We have 3 new parameters for SkopeRule class :

  • "filtering_criteria" in place of recall_min et precision_min
filtering_criteria: dict, optional
                    default={'precision': 0.5, 'recall': 0.01}
    The criteria to be used for filtering the rules.
    In the form {criterion: min_value}.
    The keys can be among ('precision', 'recall', 'f1', 'mcc', 'custom_func').
  • "deduplication_criterion" in place of the default F1 score deduplication criterion which was used in deduplicate function
deduplication_criterion: str, optional (default='f1')
    The criterion to be used for deduplicating the rules.
    Either 'f1', 'mcc' or 'custom_func'.
  • "custom_func" parameter
custom_func: FunctionType, optional (default=None)
    A personalised function that can be used as either/both a filtering
    or/and deduplication criterion.
    Has to take 1 parameter (tuple of size 4) which is supposed to be the confusion matrix
    elements (tn, fp, fn, tp) (in that order).

@fayeab fayeab closed this Dec 7, 2020
@fayeab fayeab reopened this Dec 7, 2020
@fayeab fayeab closed this Dec 7, 2020
Signed-off-by: fayeab <fayablaye@gmail.com>
Signed-off-by: fayeab <fayablaye@gmail.com>
@fayeab fayeab reopened this Dec 9, 2020
….base and sklearn.utils.testing modules are deprecated in version 0.22 and will be removed in version 0.24

Signed-off-by: fayeab <fayablaye@gmail.com>
@fayeab fayeab closed this Dec 10, 2020
…datasets.base and sklearn.utils.testing modules are deprecated in version 0.22 and will be removed in version 0.24"

This reverts commit 38e7ce4.
…removed in version 0.24.

Signed-off-by: fayeab <fayablaye@gmail.com>
…utils.testing modules are deprecated in version 0.22 and will be removed in version 0.24

Signed-off-by: fayeab <fayablaye@gmail.com>
@fayeab fayeab reopened this Dec 10, 2020
…e_weight. This parameter is not use in same way by skrules.

Signed-off-by: fayeab <fayablaye@gmail.com>
@ngoix
Copy link
Member

ngoix commented Dec 11, 2020

Thanks @fayeab for the good work
It is gonna be hard to get reviews on this as a whole though, I would split this PR into at least 3 new ones:

  1. requirement updates and test updates / sample_weight renaming / data getters update
  2. addition of filtering_criteria parameter
  3. addition of deduplication_criterion and custom_func params

These PRs do not need to be independent and are probably to be based on each others 1. <- 2. <- 3.- shouldn't take much time to split the code and it will make the review process a lot smoother

…not allowed by scikit-learn checks in version 0.24. All init parameters have to be immutable to make cloning possible.

Signed-off-by: fayeab <fayablaye@gmail.com>
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.

None yet

2 participants