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

Include min and max values in the extremeintervals on quantile biner. #149

Closed
wants to merge 1 commit into from

Conversation

gabrieldi95
Copy link

Status

READY

Background context

quantile_biner would result in n+1 quantiles when the min or max values were equal to either the first or last quantile.

Description of the changes proposed in the pull request

Change values categorized as the '0' quantile to 1 and values categorizes as 'n+1' (n being the wanted number of quantiles)
to n.

@gabrieldi95 gabrieldi95 requested a review from a team as a code owner August 15, 2020 23:34
@gabrieldi95
Copy link
Author

Tests failed probably because of already existing errors... File tested is ok.
image

def p(new_df: pd.DataFrame) -> pd.DataFrame:
col_biner = lambda col: np.where(new_df[col].isnull(), nan, np.digitize(new_df[col], bins[col], right=right))
bined_columns = {col: col_biner(col) for col in columns_to_bin}
bined_columns = {col: _fix_values(col_biner(col), len(bins[col])) for col in columns_to_bin}
Copy link
Contributor

Choose a reason for hiding this comment

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

i think np.clip(col_biner(col), 0, len(bins[col])) could be more idiomatic here

@vitorsrg
Copy link
Contributor

the mock test seams to fail because this PR changes the function behaviour...

maybe this change could be triggered by some parameter in a backwardly compatible way

@caique-lima
Copy link
Contributor

@gabrieldi95 will you keep working on this? Seems that the tests are broken. Let me know, otherwise we will close this PR for now, later you can feel free to reopen it.

@caique-lima
Copy link
Contributor

No updates in the last few months

@caique-lima caique-lima closed this Feb 9, 2021
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.

quantile_biner
3 participants