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

Fix agg_numerical_column function #14

Merged
merged 3 commits into from
Jan 24, 2021
Merged

Fix agg_numerical_column function #14

merged 3 commits into from
Jan 24, 2021

Conversation

ThaminduR
Copy link
Owner

Logical error in the agg_numerical_column has been fixed.

Related Issue - #13

@agessner
Copy link

Hey @ThaminduR thanks for fixing it! I don't know much about k-anonymization, but here are the tests @Marcaoandradenogueira and I have made. They are giving us the following results:

input

column1 column2 column3 column4 column5
600000 1 test1 x 20
600000 1 test1 y 30
800000 2 test2 x 50
800000 2 test3 x 45
800000 1 test2 y 35
400000 2 test3 y 20

params

categorical = set(('column2', 'column3', 'column4'))
sensitive_column = 'column4'
feature_columns = ['column1', 'column2', 'column3']
k = 2

k_anonymize

column1 column2 column3 column4 count
400000-600005 1, 2 test3, test1 x 1
400000-600005 1, 2 test3, test1 y 2
800000 1, 2 test3, test2 x 2
800000 1, 2 test3, test2 y 1

k_anonymize_w_user

column1 column2 column3 column4 column5
05-10 1, 2 test1, test3 x 20
05-10 1, 2 test1, test3 y 30
05-10 1, 2 test1, test3 x 20
800000 2, 1 test2, test3 x 50
800000 2, 1 test2, test3 y 45
800000 2, 1 test2, test3 y 35

The k_anonymize appears to be fixed, but the k_anonymize_w_user is having the same problem as before. Maybe it's some parameter we are passing wrong or the data is not a good fit. It just felt weird having this 05-10 result.

Thanks!

@ThaminduR
Copy link
Owner Author

97c2211 should fix the issue with k_anonymize_w_user function.
Also ,now the numerical aggregating function will handle 0 or 5 as the last digit case, gracefully.

@ThaminduR
Copy link
Owner Author

if the test cases are giving the correct output, then I'll merge this.

@ThaminduR ThaminduR merged commit a41b63f into master Jan 24, 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.

None yet

2 participants