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.

Sign up for GitHub

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

Jump to bottom

Issue 265 privileged class bank dataset #449

Merged
merged 12 commits into from
Jul 27, 2023
Merged

Issue 265 privileged class bank dataset #449

merged 12 commits into from
Jul 27, 2023

Conversation

joosjegoedhart
Copy link
Contributor

@joosjegoedhart joosjegoedhart commented Mar 27, 2023 •
edited
Loading

Closes #265

Changes:

  • Updated readme for bank dataset
  • Added age >60 to unpriviliged group in bank_dataset.py
  • Added tests for bank dataset
  • Fixed linting errors for all tests/test_standard_datasets.py

@hoffmansc or @nrkarthikeyan

@joosjegoedhart joosjegoedhart force-pushed the issue_265_privileged_class_bank_dataset branch from 9d392d2 to 8f3d181 Compare June 14, 2023 08:21
@joosjegoedhart
Copy link
Contributor Author

@hoffmansc please review :)

hoffmansc
hoffmansc requested changes Jul 21, 2023
View reviewed changes
Copy link
Collaborator

@hoffmansc hoffmansc left a comment

Choose a reason for hiding this comment

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

Hi! Thanks for contributing (and for reminding me to review 😉).

One extra request: can you do the same thing for fetch_bank() in aif360.sklearn.datasets? You can add an argument binary_age like fetch_german

tests/test_standard_datasets.py Outdated Show resolved Hide resolved
@joosjegoedhart joosjegoedhart force-pushed the issue_265_privileged_class_bank_dataset branch from 1466316 to 120da68 Compare July 24, 2023 12:51
hoffmansc
hoffmansc requested changes Jul 24, 2023
View reviewed changes
Copy link
Collaborator

@hoffmansc hoffmansc left a comment

Choose a reason for hiding this comment

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

the tests are not passing right now. It looks like the data is not being downloaded properly and there's an issue with the pd.cut line

aif360/sklearn/datasets/openml_datasets.py Outdated Show resolved Hide resolved
aif360/sklearn/datasets/openml_datasets.py Outdated Show resolved Hide resolved
aif360/sklearn/datasets/openml_datasets.py Outdated Show resolved Hide resolved
@joosjegoedhart joosjegoedhart force-pushed the issue_265_privileged_class_bank_dataset branch from 93b3626 to 8157ca7 Compare July 25, 2023 15:19
@joosjegoedhart
Copy link
Contributor Author

joosjegoedhart commented Jul 25, 2023 •
edited
Loading

For test_standard_datasets.py::test_bank_priviliged_attributes

E FileNotFoundError: [Errno 2] No such file or directory: 'aif360/data/raw/bank/bank-additional-full.csv’. I keep on getting errors here. It seems because os.chdir() in notebook_runner.py() sets the working directory of the entire test process process to …AIF360/examples.

I fixed this issue by resetting the path in tests/notebook_runner.py

@joosjegoedhart
Update bank readme with right file
60ff35c 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Add age >=60 to unpriviliged group
0a2eaaf 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Initial test for bank dataset
bb1ac09 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Unittest for bank and fixed linting errors
a304fc4 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Update ci.yml s.t. it downloads bank data and change bank dataset tes…
8810ba2 
…t without mock

Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Add binary_age argument to fetch_bank
8fe306b 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Debug CI
86f3aab 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Debug fetch_bank s.t. it passes test
0323230 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Add reorder categories
bc4e85d 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Reset cwd in notebook runner and ubuntu latest
785c3ef 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart
Update filepath bank dataset
9910a36 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@joosjegoedhart joosjegoedhart force-pushed the issue_265_privileged_class_bank_dataset branch from 651ca23 to 9910a36 Compare July 27, 2023 13:53
@joosjegoedhart
Restore gitlab ci
01e5997 
Signed-off-by: joosjegoedhart <joosje.goedhart@ibm.com>
@hoffmansc hoffmansc merged commit 6f9972e into Trusted-AI:master Jul 27, 2023
9 checks passed
@hoffmansc hoffmansc mentioned this pull request Jul 27, 2023
Closed
@joosjegoedhart joosjegoedhart deleted the issue_265_privileged_class_bank_dataset branch July 31, 2023 12:27
divyagaddipati pushed a commit to divyagaddipati/AIF360 that referenced this pull request Sep 22, 2023
@joosjegoedhart @divyagaddipati
Issue 265 privileged class bank dataset (Trusted-AI#449)
28804bf 
* Updated readme for bank dataset
* Added age >60 to unprivileged group in bank_dataset.py
* Added tests for bank dataset
* Fixed linting errors for all tests/test_standard_datasets.py
* Added binary_age to fetch_bank
* Download bank dataset in ci.yml

Signed-off-by: Divya <divyajyothig96@gmail.com>
meghana009 pushed a commit to meghana009/AIF360 that referenced this pull request Sep 22, 2023
@joosjegoedhart @meghana009
Issue 265 privileged class bank dataset (Trusted-AI#449)
bbe86ea 
* Updated readme for bank dataset
* Added age >60 to unprivileged group in bank_dataset.py
* Added tests for bank dataset
* Fixed linting errors for all tests/test_standard_datasets.py
* Added binary_age to fetch_bank
* Download bank dataset in ci.yml

Signed-off-by: Venkata Meghana Achanta <vachanta@usc.edu>
Signed-off-by: meghana009 <meghanaachanta09@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoffmansc hoffmansc hoffmansc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Privileged class in BankDataset
2 participants
@joosjegoedhart @hoffmansc